feat(#6966): add fullsend agent new to generate a complete custom agent - #6972
Open
waynesun09 wants to merge 17 commits into
Open
feat(#6966): add fullsend agent new to generate a complete custom agent#6972waynesun09 wants to merge 17 commits into
fullsend agent new to generate a complete custom agent#6972waynesun09 wants to merge 17 commits into
Conversation
PR Summary by QodoGenerate complete custom agents with
AI Description
Diagram
High-Level Assessment
Files changed (44)
|
|
🤖 Review · Commit: |
Site previewPreview: https://fe7fa397-site.fullsend-ai.workers.dev Commit: |
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
🤖 Review · Commit: |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
🤖 Review · Commit: |
Member
Author
|
/fs-review |
|
🤖 Review · Commit: |
Member
Author
|
/fs-review |
|
🤖 Finished Review · ❌ Failure (validation failed after 2 iteration(s)) · Started 5:32 PM UTC · Completed 6:15 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high |
Member
|
This introduces a lot of machinery for something that could be a |
Member
Author
|
Fair question. Two things: It's an interface, not a copy. The loudest user feedback we have is that creating an agent is hard to figure out (#4839, #5804, #6830, #6834). What users want is: name + role in, working agent out. Working means: commit it, type There's nothing to
About half of the 1.5k lines of Go handles exactly that: the role→provider table, trigger compile, pre-write checks, registration, and the symlink/rollback safety the bot review asked for. The templates themselves are 226 lines. On the 6k: 2.2k is tests and 1.7k is golden fixtures. I'll cut the goldens from 7 trees to 2 (about 1,200 lines fewer, same coverage) and can trim CLI-level tests that duplicate package tests if you'd like. |
Member
Author
|
Numbers after the trim, head
The golden trees went 7 → 2 and the duplicated CLI tests are gone: −1,200 lines, patch coverage 87.4% (plain |
|
🤖 Review · Commit: |
|
🤖 Review · Commit: |
|
🤖 Review · Commit: |
|
🤖 Review · Commit: |
…nt new Groundwork for `fullsend agent new`: the three pure, network-free pieces the generator is built on. The role table is hardcoded rather than derived from mintcore.BuiltInRoles(). Derivation would re-admit `scribe`, which config.ValidRoles() deliberately excludes as a mint-only dogfood role that "must not silently pass config validation", and it would fail open for any future canonical role with no provider pairing. Drift is caught by a test asserting each role is in both BuiltInRoles() and ValidRoles() and that its permissions equal mintcore.RolePermissionsFor(role) — BuiltInRoles rather than HasRole, because HasRole also returns true for standalone-mint custom roles registered at runtime. Trigger presets are pinned by string equality, not just compiled. An expression can compile and still touch an absent optional field, which fails only at first dispatch — MatchHarnesses turns that into a red ::error:: annotation on every matching event. The `command` preset guards fork pull requests with has() rather than the reference doc's `!= null`: state.change_proposal is absent, not null, on a non-PR comment, so `!= null` raises a missing-key error on every issue comment. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
Render turns resolved options into the full file set: harness, agent definition, result schema, post-script, and the shared scaffold assets a generated agent depends on but does not own. The harness is built as a harness.Harness value and marshalled, never formatted as text, so the generator cannot emit a field the validator does not know about. The agent definition's frontmatter is marshalled for the same reason: a --description containing a colon or a leading ">" would otherwise produce a broken document. The only user value that reaches the generated shell script is the agent name, substituted after it has passed harness.ValidAgentBasename. Providers and profiles are referenced by path, not bare name. A bare name with no definition on disk does not fail loudly — the embedded fallback fills in only the OpenAI provider, so any other name degrades to a warning and then a sandbox that cannot reach Vertex. The generator therefore also writes policies/, providers/ and profiles/ when absent: a per-repo install vendors none of them, CI's workspace layering skips policies/ because the embedded scaffold has no such directory, and profiles/ was never in LAYERED_DIRS at all. ValidAgentBasename and ValidSlug are exported alongside the existing ValidPluginBasename so a caller that constructs a harness, rather than loading one, can reject an unsafe name before writing it anywhere. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
CheckGenerated runs the checks a freshly constructed harness needs:
Lint diagnostics, ResolveRelativeTo, ValidateFilesExist, and a stat of
every provider and profile named by path.
That fourth step is the point of the helper. ValidateFilesExist skips
providers and profiles deliberately — ResolveHarness reads them later,
during the run's resolve step, and reports its own error. A generator
has no resolve step, so without it the failure the generator most needs
to catch is invisible: a harness naming providers/vertex-ai.yaml with no
such file validates cleanly and then fails at run time, or degrades to a
warning and a sandbox that cannot reach Vertex.
CheckGenerated is deliberately not wired into run or lock. Those two
interleave minting, runner-env validation and ${VAR} expansion between
the same steps, in different orders — run expands paths so they resolve
before ValidateFilesExist stat-checks them, and lock never calls
ValidateFilesExist at all. Collapsing them would change behaviour on the
two hottest execution paths. ValidateRunnerEnvWith is likewise excluded,
and says so: it requires every ${VAR} to be set in the calling process,
which is true in CI and false on a developer's machine.
The trigger presets are now evaluated against the normative
NormalizedEvent v1 fixtures. The command preset is the default, so every
generated agent carries it, including role coder — it must fire for a
non-fork pull request comment and an issue comment, and must not fire
for a comment on a fork. A companion test evaluates it against all
thirteen fixtures and asserts it errors on none: that is the property
the reference doc's `!= null` does not have.
Assisted-by: Claude
Signed-off-by: Wayne Sun <gsun@redhat.com>
Generates a complete, valid, runnable custom agent from a name and a role, registers it, and validates the result with the same loader dispatch uses — so the errors that used to appear at the first dispatch after merge appear at generation time instead. Ordering is the substance of the command. The name is checked against the shell-safety pattern before anything touches disk; the trigger is expanded and compiled before any file is written; everything is rendered and validated in a scratch directory first, so a harness that would fail validation never leaves a half-written .fullsend behind. Where a shared asset already exists in the target, the target's copy is what gets validated, since a hand-edited policies/base.yaml is common and its content matters. --force overwrites the four files an agent owns but never a shared scaffold asset, and never papers over a config name collision. A trigger is mandatory. ListTriggeredHarnesses skips a trigger-less harness with a bare `continue` and no annotation, so such an agent registers, validates, appears in `agent list`, and is then silently never dispatched — the single failure this command is best placed to prevent. The printed CI instruction is derived from the same trigger that went into the harness, so the two cannot disagree. The existing subcommand-count assertion is updated for the sixth subcommand and gains positive assertions for `new` and `set`. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
Every command shown was run against a real temporary --fullsend-dir and the output pasted is the output it produced. The CEL reference's slash-command pattern is corrected to use !has(state.change_proposal) rather than `!= null`. change_proposal is ABSENT on a comment posted to a plain issue, not present-and-null — the NormalizedEvent schema requires only `labels` under `state` — so comparing it against null raises a missing-key error, and dispatch reports that as `trigger eval failed` on every issue comment in the repository. The corrected block is byte-identical to what `agent new --on command:` emits, and a test asserts the two stay that way. The Bring Your Own Agent guide claimed twice that `github setup` creates policies/, providers/ and profiles/. It creates none of them, and that sentence is what walks authors into #6834; it now says so and points at the generator. ADR 0102 records the decision. It is a new ADR, not a superseding one: #4839's July "prefer guides over a CLI" call was taken in Slack and never written up. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
In a linked worktree, .git is a FILE containing "gitdir: <path>" rather than a directory, and the repository config lives in the main repository's common directory. The owner lookup read dir/.git/config directly, so inside a worktree it found nothing and every generated agent silently fell back to the "fullsend-<name>" slug. Worktrees are the normal way to work in this project, so that was the common path rather than an edge case. readGitConfig now follows the gitdir pointer and the commondir file; a malformed pointer still degrades to the fallback rather than failing. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
Six correctness findings, all on paths that could leave a repository changed by a failed run or accept input that should be refused. A name already registered in config.yaml was only detected during registration, which happens after every file has been written — so a duplicate name left the directory modified by a run that then failed. The check now happens before anything is written, and the fullsend-dir check happens before that, so a missing directory is still reported as "run github setup first" rather than as a failure to read config.yaml from inside it. Destinations are now resolved with Lstat rather than Stat, and every directory between the fullsend dir and each destination is checked. A repository shipping a `.fullsend/harness` symlink, or a dangling symlink at a destination, could otherwise redirect a generated file outside the fullsend directory entirely. Anything that is not a regular file is refused rather than overwritten. A failure part-way through the write loop now removes the files that run created, rather than leaving a partial agent: some files present, the harness perhaps missing, nothing registered. An existing shared asset that cannot be read — a directory in its place, or bad permissions — now fails naming the path. It previously fell back to validating the embedded copy, which is not the file the agent would have run against. --runtime is validated while options are resolved rather than during registration, and only a real io.EOF ends a spec document: any other error from the second decode is a malformed second document and is now reported instead of being read as absence. The opt-in validation_loop gains a preflight_check. The validation script hard-fails when python3 or jsonschema is missing, and it does so only after the agent has run; preflight_check is evaluated before sandbox creation, so the same missing dependency costs nothing. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
codecov/patch failed at 72.23%. The cause was mine: internal/harness's CheckGenerated and the exported name validators were exercised only from internal/agentnew's tests, and Codecov credits a file from tests in its own package. My local measurement had used -coverpkg, which does credit cross-package calls, so it reported 88% for a patch CI scored at 72%. CheckGenerated now has direct tests in package harness: the happy path, each missing-file branch including the provider and profile stat that ValidateFilesExist deliberately skips, a lint diagnostic returned non-fatally, a path escaping the fullsend directory, and a bare provider name correctly not stat-ed. ValidAgentBasename and ValidSlug get table tests; the rejected half of ValidAgentBasename is the security-relevant one, since the agent name reaches shell interpolation. The generator's write paths are covered in their own package too: force-overwrite versus shared-asset preservation, collision refusal, dry-run, the injected-failure rollback, both symlink refusals, and the unreadable shared asset. Measured without -coverpkg, as CI measures: 87.9%. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
Adds the ADR 0102 entry to the agent-registration Decided list in architecture.md, which the repo's convention expects for a new ADR, and converts the ADR's Consequences section to bullets per the template. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
Step 0 opened with a runnable command while the prerequisites it needs — the CLI on PATH, a scaffolded repository, inference and the GitHub Apps — were two sections further down. It now points at them first, and the overview leads with the numbered sequence rather than prose so step 0 reads as part of the flow instead of an aside. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
The generated post-script cut the comment at MAX_COMMENT_CHARS and then appended the truncation marker, so a truncated comment came out longer than the 16384-character limit the generated result schema declares. It now reserves the marker's length before cutting. Verified against the generated script: a 17,000-character comment produces exactly 16384. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
Seven golden trees for a five-role table was mostly repetition: the roles differ only in table data — image, providers, profiles — which TestRoleTableMatchesMint and its siblings already assert directly, and more precisely than a byte diff does. Kept triage as the default and validation-loop as the second. Not retro or coder, despite those having the larger diffs: their differences are data, while validation-loop is the only variant that exercises an actual conditional in the generator — `if opts.ValidationLoop` in both buildHarness and sharedAssets, which is also the only path that writes a conditional shared asset. Two branches the dropped trees did cover are now asserted directly rather than incidentally: that each role's image and provider set reach the generated harness, including that both image constants are reachable so neither becomes dead configuration; and that each --on preset survives marshalling into the harness. Those are checks a table test cannot make, which is why they are worth keeping when the trees go. On the CLI side, TestAgentNewForceNeverOverwritesSharedAssets and TestAgentNewMissingDir duplicated package-level tests in internal/agentnew outright, and the dry-run test's "wrote nothing" half did too; it now asserts only what this layer can see, that the command reports its plan. treeSnapshot lost its last caller and is removed. Net 1,200 lines lighter. Patch coverage 87.4%, measured with plain `go test -cover` per package as CI measures it. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
A reader who has not used fullsend cannot follow the new pages. Three reviewer questions on the sibling PR were all the same defect — a term of art used as if it were plain English — so this fixes the class rather than the instances. `mint` was used four times in docs/cli/agent.md before anything said what it is; it now gets a sentence saying agents hold no long-lived credentials and ask a service for a short-lived token, which is what makes the role table matter. `--slug` said "harness slug", which tells a reader nothing they did not already have; it now says what the value is for. "A per-repo install does not vendor them" is now "`fullsend github setup` does not copy these into your repository". "You edit prose, not plumbing" is now the actual claim: every file is written for you except the instructions the agent follows. Triggers are described before the presets table rather than assumed. The Bring Your Own Agent step 0 listed harness, agent definition, schema, post-script, trigger and registration — six terms, none defined until later in the page, in the first thing a reader sees. It now says what those files do. The generated harness header called the image "the fleet's current pin". Every generated agent carries that comment, so the jargon shipped to users; it now explains that the digest pins the image so every run uses the same one. Goldens regenerated. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
Four reviewers ran over this branch; three independently found the same
thing, and it is the failure this whole command exists to prevent.
`postCmd.Dir = runDir` (internal/cli/run.go:1645) — the post-script runs
from the RUN directory, and the agent's output is one level down, in
iteration-<N>/output/. The template hardcoded `output/agent-result.json`,
a path that never exists, so every agent this command generated would
have failed its first real run. The fleet's own post-script gets this
right, and I had quoted its pattern in the research for this change.
The tests could not have caught it. They matched substrings, and
`output/` appears in both the right and the wrong form; the shell test
built `<tmp>/output/` and ran from there, encoding the same wrong
assumption, so seventeen green assertions were confirming the bug. The
post-script is now executed by a Go test against the real layout —
iteration directories, the validated-iteration override, and the
no-output case — which is the only shape of test that distinguishes
them.
Selecting the last iteration by glob order was also wrong past nine:
iteration-10 sorts before iteration-9. It is compared numerically now.
Also from this round, each verified before fixing:
- A symlinked `.fullsend` root wrote straight through it. The earlier
symlink guard only walked segments BENEATH the root, and
ValidateFullsendDir used Stat, which follows. Reproduced by writing
three files outside the tree, then fixed with Lstat.
- Rollback deleted `--force`-overwritten files instead of restoring
them, so an interrupted regeneration destroyed a working agent rather
than leaving it alone. Originals are now kept and restored.
- os.WriteFile leaves an existing file's mode alone, so a re-generated
post-script could keep a non-executable bit. Chmod is now explicit.
- `_lead` passed generation and then failed registration, leaving ten
files behind: harness and config disagreed on the first character.
config.ValidConfigAgentName is exported and both rules now apply.
- The command trigger fired on GitHub Discussions, which the docs say it
does not. Verified against the normative fixture — entity.kind is
"conversation" — and the doc's own pattern had the work_item guard I
had dropped. Restored in the preset and the reference together.
- The body told every agent to run fullsend-check-output, which exits
non-zero unless FULLSEND_OUTPUT_SCHEMA is set — and the runner only
sets it when a validation loop is configured, which is off by default.
The instruction is now rendered only when it will work, and the tools
list matches what the body actually invokes rather than claiming to.
- ${#var} counts bytes under a C locale while the schema's maxLength
counts code points, so a valid multibyte summary was rejected and
truncation could slice mid-character. The script pins LC_ALL.
- `jq -e .` called a bare `null` document invalid JSON and let a
top-level array reach a raw jq error; it now checks for an object.
A model-supplied summary containing a newline broke the heading it is
interpolated into, and is refused.
- ownerFromGitConfig kept its section state across headers, so an origin
with no url could return a submodule's owner.
- The slug lookup was handed a relative path it could not walk above.
- ADR 0102 documented `--template-dir`, which does not exist.
- --runtime offered the dummy runtimes, which do no inference.
- `timeout_minutes: 0` was indistinguishable from omitted.
- A comment described an implementation the code does not use, two named
a function that does not exist, and a dead branch returned the same
value twice.
Patch coverage 87.8%, plain `go test -cover` per package.
Assisted-by: Claude
Signed-off-by: Wayne Sun <gsun@redhat.com>
The generated prompt says the summary is one line and the generated post-script refuses one that is not, but the generated schema only constrained type and length. So a multi-line summary passed the validation loop, no retry was triggered, and the run died in the post-script with nothing posted — the three files that are supposed to describe one contract described two. Found reviewing the reference agent in fullsend-ai/agents#1167, which is generated from this template. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
The result-path bug this branch fixed could only ever surface at run time, and nothing in the PR had actually run a generated agent. So I ran one: `--runtime dummy` exercises the real sandbox and the real post-script with a scripted result in place of the model, which proves the whole pipeline without spending inference or touching a forge. It works — agent exit code 0, and the post-script finds iteration-1/output/agent-result.json and renders the comment. That output is now in the docs as what success looks like, because it is the one command whose failure this branch exists to prevent. Running it also found a gap in the command's own next-steps text. The generated harness copies GOOGLE_APPLICATION_CREDENTIALS into the sandbox, so a run stops at validation without it — even under the dummy runtime, which does no inference — and the printed list did not mention it. GH_TOKEN also has to be a real token, because a connectivity check runs before the agent does; a placeholder fails with `Bad credentials`. Both are now in the printed next steps and in the troubleshooting prose, alongside the `--forge github` requirement. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
waynesun09
added a commit
to fullsend-ai/agents
that referenced
this pull request
Sep 4, 2026
The generator template had the post-script looking for its input in the wrong directory (fullsend-ai/fullsend#6972), so the example carried the same defect and this repository's test confirmed it: run_post built <tmp>/output/agent-result.json and ran from there, which is not the layout fullsend provides. Seventeen assertions passed against a script that could not have worked. The test now builds iteration-<N>/output/ and runs from the run directory, as internal/cli/run.go does, and adds the two cases the old shape could not express: that the highest-numbered iteration wins, and that FULLSEND_VALIDATED_ITERATION_DIR overrides it. The example is regenerated from the fixed template, which also brings the locale pin, the JSON-object shape check in place of `jq -e .`, the single-line summary check, and the work_item guard that keeps the trigger off GitHub Discussions. Three more reject cases cover shapes the old `jq -e .` gate got wrong: a bare null, a bare false, and a top-level array. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
waynesun09
force-pushed
the
agent-6966-agent-new
branch
from
September 4, 2026 16:10
ce5b57d to
366e64a
Compare
|
🤖 Review · Commit: |
The dummy run proves the plumbing; it cannot prove that a real model can follow the generated prompt with the tools and network profile the harness grants. This run does: the reference agent against a real pull request, exit 0, schema validation passed, post-script printing rather than posting. Getting there took isolating an unrelated failure, which is worth recording because the symptom is misleading. A local `--runtime claude` run died with `Could not refresh access token: policy_denied`, which reads as a credentials problem. It was not: the composed sandbox policy contained no Vertex network policy at all, because this host's registered `vertex-ai` provider had drifted to zero credential keys. The provider file and the profile were both byte-correct. Deleting the provider and letting the run re-ensure it produced a healthy registration and the run worked. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
|
🤖 Finished Review · ❌ Failure (validation failed after 2 iteration(s)) · Started 4:40 PM UTC · Completed 5:22 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high |
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
2 participants
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.
What
Adds
fullsend agent new <name>— generates a complete, valid, runnable custom agent from a name and a role, registers it, and validates the result before returning.fullsend agent new lint-docs --fullsend-dir .fullsend \ --role triage --description "Check docs changes for broken links"Writes
harness/,agents/,schemas/andscripts/post-*.shfor the agent, plus thepolicies/base.yaml,providers/andprofiles/a per-repo install does not vendor (written only when absent, never overwritten — not even with--force).Why
Building a custom agent by hand is the most common piece of negative user feedback. Three of the failure modes are silent:
trigger:is silently undispatchable. It registers, validates, and shows up inagent list, thenListTriggeredHarnessesskips it with a barecontinueand no annotation — while resolve and load failures both emit::error::. None of the seven fleet harnesses has atrigger:to copy from.agent newrefuses to write one without a trigger.role:the mint does not serve surfaces as an opaque403at first dispatch (Mint service returns opaque 403 for unregistered BYO agent roles #6563).--roleis a closed five-row table with a drift test againstmintcore.RolePermissionsFor.policies/base.yamlor provider file fails at run time, or degrades to a warning and a sandbox that cannot reach Vertex (v0.37 scaffold should generate a default policies/base.yaml for sandbox-enabled agents #6834). The generator writes them and stats them.gh aw newis the precedent. #4839 recorded a July "prefer guides over a CLI" decision — taken in Slack, never written up as an ADR — which is why ADR 0102 is a new ADR rather than a superseding one.Also fixes a live documentation bug
docs/guides/user/cel-triggers-reference.mdgave this as the canonical slash-command trigger:state.change_proposalis absent on a comment posted to a plain issue, not present-and-null — the NormalizedEvent schema requires onlylabelsunderstate, and thejira-fs-triage-comment/discussion-fs-vouch-commentfixtures confirm it. Comparing an absent key againstnullraises a missing-key error, andMatchHarnessesreports that as::error:: harness dispatch: skipping agent <name>: trigger eval failedon every issue comment in the repository. Anyone who copied that pattern has an agent that looks permanently broken.Corrected to
!has(event.state.change_proposal) || !event.state.change_proposal.is_fork. The doc block is now byte-identical to what--on command:emits, andTestCommandPresetMatchesTheReferenceDocasserts they stay that way.The BYO guide also claimed twice that
github setupcreatespolicies/,providers/andprofiles/. It creates none of them — that sentence is what walks authors into #6834.How to test
Validated commands
Every command in the documentation was executed against a real temporary
--fullsend-dirand the pasted output is what it produced.agent new lint-docs --role triage --description ...find .fullsend -type f | sortagent new -f link-check.agent.yamlagent new report --dry-runagent new report --role scribelock lint-docs --offlineagent listagent set lint-docs --model sonnetSet agent ...lineagent remove link-checkagent new 'a;rm -rf /'/ bad--trigger/ bad--onDocumented but not executed here: a full
fullsend runof the generated agent, and CI dispatch. Both need GCP credentials, a sandbox image pull and a live forge, and the post-script's comment path mutates a real work item. The BYO guide points atrunning-agents-locally.mdfor prerequisites and documentsPOST_<NAME>_DRY_RUN=1so the post-script prints instead of posting; no success block is fabricated for either.Gates
go test ./internal/agentnew/... ./internal/cli/... ./internal/harness/... ./internal/config/...— greengo test -coverper package — threshold is 80%, and CI is the authority. An earlier revision of this body claimed 88.2%; that figure came fromgo test -coverpkg, which credits cross-package calls in a way Codecov does not, and the first CI run scored the patch at 72.23%. The tests forharness.CheckGeneratednow live in packageharnesswhere Codecov can see them.make lintclean, including the ADR and markdown-link checksmake e2e-testnot run — it needs live GitHub pool orgsKnown gap
Generaterolls back the files it wrote if the write loop itself fails, and aname already in
config.yamlis now refused before anything is written. But ifrunAgentAddfails for some other reason after the files have landed — anunwritable
config.yaml, or acfg.Validate()failure from unrelated existingcontent — the generated files stay on disk and the command exits with
agent files were written but registration failed: ....That message is accurate rather than misleading, and re-running with
--forcerecovers, so this is a follow-up rather than a blocker. Closing it properly
means either extending the rollback across the registration step or writing the
config first and the files second; both are a larger change to the ordering than
belongs in this PR. Flagged by Qodo #1's request for transaction semantics and
by the review agent's partial transcript.
Notes for review
## Notesentry recording thatagent newsits in front of the registration model 0058 describes, which is the kind of forward cross-reference CONTRIBUTING.md explicitly allows.docs/architecture.mdgains the matching entry in the agent-registrationDecided:list./renumber-adrbefore merge.lockandrunare deliberately not rewired onto the newharness.CheckGeneratedhelper. They interleave minting, runner-env validation and${VAR}expansion between the same steps in different orders —runexpands paths so they resolve beforeValidateFilesExiststats them, andlocknever callsValidateFilesExistat all. Whetherlockshould gain it is a real question, but a behaviour change on those paths does not belong in a generator PR.ValidateRunnerEnvWithis intentionally not part of the generation-time check, and says so in the helper's doc comment: it requires every${VAR}to be set in the calling process, which is true in CI and false on a developer's machine. The run-time error is in the troubleshooting table.Closes #6966