Software factory change - #515
Merged
Merged
Conversation
The worker's artifact scan skipped every entry whose name began with a dot, so nothing an agent wrote under `.workflow-artifacts/` — the conventional artifact directory — ever reached `output.artifacts`. An `artifact_exists` gate naming such a path could therefore never pass: it failed as a bare `retries_exhausted exit=1` on a file sitting on disk, with nothing in the completion to say why. Replace the blanket dot-prefix skip with an exact-name set (`.git`, `.relayflowd`, `node_modules`) applied at any depth before the entry's type is consulted, so a linked worktree's regular `.git` file is skipped too while `.github` and `.relayflowd-notes` stay eligible. The gate still reads the journal and never the disk; the fix is that the journal now describes what the agent wrote. Co-Authored-By: Claude <noreply@anthropic.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix: journal agent artifacts written under dot-directories (#512)
Summary
packages/sdk/src/agent-artifacts.ts) skippedevery directory entry whose name began with a dot. Nothing an agent wrote
under
.workflow-artifacts/— the conventional artifact directory — everreached the journaled
output.artifacts, so anartifact_existsgate namingsuch a path could never pass. It failed as a bare
retries_exhausted exit=1on a file sitting on disk, with nothing in the completion to say why.
.git,.relayflowd,node_modules— matched at any depth and applied before theentry's type is consulted, so a linked worktree's regular
.gitfile isskipped as surely as a
.gitdirectory, while.github,.relayflowd-notesand every other author-chosen name that merely starts the same way stays
eligible.
is that the journal now describes what the agent actually wrote.
Commit:
6990e55, onrelayflow/flows-software-garden-f1a04253.Scope
Followed
reviewed-plan.md, which selected the inclusion route of theticket's "either / or" acceptance. Deliberately not done, per that review:
flows check/gate_path_excludedrefusal.namedGateErrorssees noworker transport or output provenance, an author-supplied JSON output can
legally contain any path, and
checkTypeScriptFlowdoes not execute anauthored body — a global path prohibition would be a new producer-aware
contract, not one cheap branch.
artifacts.includeopt-in, no.gitignorehandling, no kernel change,no change to the
subprocess_gatestdio loss (A failing subprocess_gate journals empty stdout/stderr: the lowering uses stdio:'inherit' and the daemon's stdio is captured nowhere #511).Known limitations left standing and still documented: a content diff proves a
change during the interval, not authorship; a JSON-shaped final message still
journals no artifacts; the relay transport journals none.
Files
packages/sdk/src/agent-artifacts.tsSKIPPED_DIR_NAMES→SKIPPED_ENTRY_NAMES(.git,.relayflowd,node_modules); blanket dot-prefix skip removed; scan semantics documented beside the codepackages/sdk/tests/agent-artifacts.test.ts.gitfile) replacing the old blanket-exclusion testpackages/sdk/tests/artifact-gates.test.ts.workflow-artifacts/x/y.mdand excludes.git; new lowered-gate test pinning journal-only judgement on a dot path;namedGateErrorsaccepts the dot pathpackages/sdk/tests/agent-artifacts-live.test.tsagent-2writing and gating.workflow-artifacts/x/y.md, asserted from the realstep.completeddocs/SURFACE.mdownEvidencePathsprose corrected to the subtree exclusionworker-cli.tsactually implementsTest plan / evidence
All commands run from the repo root unless noted. Captures are in
/tmp/flows-512/.Full SDK suite
npm run test --prefix packages/sdkThe three affected suites pass:
The 18 failures are pre-existing and environmental
Baseline:
git stash push -u, rebuild, run only the six failing files ate21caad(HEAD before this change):The failing set is byte-identical to the one in the changed tree:
Their causes are all sandbox environment, not code:
spawnSync …/kernel/target/debug/relayflowd ENOENT—ops/cargo.shputs thebuild output outside the repo on purpose; these tests hard-code the in-repo
path (
provider-trigger-executor,live-kernel,webhook-live,mcp).expected '1.3.6' to be '1.4.0'— the sandbox'sbunis older thanauthored-node-runtime.test.tsrequires.bad_request: unknown field 'required_streams'— the prebuiltrelayflowdonPATH predates the current protocol (
communication-mixed-resume).I did not attempt to fix these; they are unrelated to this change.
Mutation verification
Meaning per
AGENTS.md: the specific change reverted, the specific tests runand failing captured, the change restored byte-for-byte, re-run and passing
captured. Both executions and the exact restoration actually happened.
1. Reverted — line 58 of
agent-artifacts.tsput back toif (entry.name.startsWith('.') || SKIPPED_ENTRY_NAMES.has(entry.name)) continue;,rebuilt, then:
The live failure reproduces the reported shape exactly — a gate that cannot
pass, with nothing to inspect:
and the unit failure names the dropped paths:
2. Restored byte-for-byte — the file was copied back from a pre-mutation
snapshot and the digest checked:
(identical to the digest taken before the mutation)
3. Re-run, same three files:
Acceptance mapping
.workflow-artifacts/x/y.mdhas that path in its journaledartifactsagent-artifacts-live.test.ts: real built CLI + real daemon + local agent;step.completed.output.artifactsforagent-2equals['.workflow-artifacts/x/y.md']andagent-2.gatecompletessuccessagent-artifacts.test.ts), worker output (artifact-gates.test.ts), journal + gate (agent-artifacts-live.test.ts)docs/SURFACE.md, pinned by tests at root and nested depthNotes
npm ciforpackages/surfaceandpackages/sdk,npm run buildinpackages/surface,then
npm install ./packages/surface --prefix packages/sdk --no-save --ignore-scriptsand a rootnode_modules/@relayflows/surfacelink — thesame sequence
.github/workflows/cloud-runtime-artifact.ymluses. Nocommitted file was changed for this.
plan.mdandreviewed-plan.mdare left untracked and uncommitted.AGENTS.md.Note
Medium Risk
Changes which files are journaled as agent artifacts and can affect
artifact_existsgates and any flow asserting onAgentResult.artifacts; behavior is intentional but broadens inclusion of dot-path writes.Overview
Fixes artifact journaling so paths under dot-directories (especially
.workflow-artifacts/) appear inoutput.artifactsandartifact_existsgates can pass.The workspace scanner in
agent-artifacts.tsno longer skips every name starting with.. It only skips exact entry names at any depth:.git,.relayflowd, andnode_modules(including a worktree.gitfile). Dotfiles and paths like.github/or.relayflowd-notes/are scanned and diffed like any other regular file.docs/SURFACE.mdis updated to match: content-based diff rules, dot-directories as artifacts, and that the worker excludes the whole configured data-dir subtree (not a per-fileownEvidencePathslist). Tests cover unit scan/diff, mocked worker output, live CLI + daemon flows, and journal-only gate behavior for dot paths.Reviewed by Cursor Bugbot for commit 6990e55. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes the worker artifact scanner so files agents write under dot-directories like
.workflow-artifacts/are journaled, which letsartifact_existsgates on those paths pass instead of failing with a bareretries_exhaustedon a file sitting on disk..git,.relayflowd, andnode_modules, matched exactly at any depth, so a worktree's regular.gitfile is skipped while.githuband.relayflowd-notesstay journaled.Written for commit 6990e55. Summary will update on new commits.