Repository navigation
feat(coexistcheck): warn on a default agent the suppressed identity no longer serves - #196
Conversation
…o longer serves adopt without --rewrite-agent leaves settings pointing at the foreign agent while suppressing that identity, and coexist-check reported nothing. Layer 5 now warns, naming both remedies. A preflight that raised warnings no longer ends on 'preflight clean'; it counts them. Refs #95
arcavenai
left a comment
There was a problem hiding this comment.
Approving. After adopt without --rewrite-agent, a repo can be left with a default agent that no longer resolves. coexist-check now warns about that instead of calling the run clean, and a run that raised warnings now says so on its last line. On main, the same 2-WARN repo ended on preflight clean: enable/adopt may proceed; at head it ends on preflight passed with 2 warning(s): enable/adopt may proceed.
What I ran. I built both binaries (main 33fa181, head b4123c4) and used a scratch HOME and CLAUDE_CONFIG_DIR with the network denied. The fixtures are git init repos with no commits. Every go test ran with -count=1.
The new check. I ran 20 settings layouts through Run with a scratch test, since deleted.
| case | head |
|---|---|
foreign agent in settings.json, identity suppressed by settings.local.json |
1 WARN naming the agent, the identity and both remedies |
| foreign agent in the local file over a clean committed one | WARN |
| clean local agent over a foreign committed one | none |
local "agent": "", or no local key, over a foreign committed one |
falls back to the committed file; WARN |
| suppression in the committed file against a user-scope enable, agent there | WARN |
own form vsdd-orch or vsdd:orch, unprefixed orch, another pack's prefix, the lookalike vsdd-factory-x:orch |
none |
| foreign prefix, identity enabled or never enabled | none |
| no default agent | none |
malformed settings.local.json or settings.json |
Run returns parse <file>: unexpected end of JSON input from the census, before this check, the same as on main; no panic and no WARN |
a non-string "agent" in the local file |
the local file is ignored and the committed value is used |
End to end, coexist-check gives these results:
- Dangling agent: 1 WARN, ending
preflight passed with 1 warning(s). - A fresh
wave-state.yamlalone: 2 WARNs, ending... 2 warning(s). - Both together: 3 WARNs, ending
... 3 warning(s). - A clean repo: still ends
preflight clean.
PassVerdict is preflight clean only at zero warnings, and it is reached only when Refuse() is false, so a run with an ERROR never prints it. It prints N warning(s) for any N above 0, which is the form the ruling names.
Red, mutants, and consumers.
- Red at 1c96b21: the 5 positive tests fail and the 5 controls pass. All 10 pass at head, and
./...passes. - Mutants: each of the 8 listed is killed by the tests the body names. So are three I added: an off-by-one count, an empty local
agentoverriding the committed one, and the adopt count increment dropped. - Consumers:
git grep 'preflight clean'finds 3 lines on main, the positive control. At head it finds onlyPassVerdict, the docs line and the new tests' assertions. No script, workflow, justfile or other doc parses the line. - CI: b4123c4 passes.
Non-blocking:
-
False WARN after a re-enable. The WARN text says "no such agent resolves", and that is false when an identity of the pack is listed as suppressed but the pack is still effectively enabled. Two cases do this:
- a user-scope enable, a committed
false, and a localtrue, so the identity is re-enabled locally; - a second marketplace's identity of the same plugin enabled beside the suppressed one.
In both, the run already refuses on
same-repo-dual-enable, so the verdict line is unaffected. Only the extra WARN in the refused output is wrong. Returning early whenview.EffectivelyEnabledholds an identity of the pack would remove it. - a user-scope enable, a committed
-
Uncovered mutant. Counting INFO findings as warnings in
dryRunPreflight(case foreign.Warn, foreign.Info:) survives, because neither dry-run fixture raises an INFO. A control with one INFO finding would pin the count to WARN only. -
"Directly under". The body says the dry run's last line sits directly under the printed WARNs. In the test's output the four-step plan block sits between them, so the line is a few lines below, not adjacent. The reason for counting holds either way: both are on the same screen of one run.
Seat: reviewer / reviewer
After
adoptwithout--rewrite-agent, a repo's default agent can name an identity that adopt has just suppressed, so a session offers no such agent andcoexist-checkstill reports a clean preflight. This adds the missing layer-5 warning, so an operator who skipped the dry run learns about it at the next check instead of in a session. It also stops the preflight from calling a run with warnings "clean".Item 2 of #95; item 3 merged in #194, so this completes #95. Additive except for one changed output line: a run that raised warnings now ends on
preflight passed with N warning(s)where it used to end onpreflight clean. Nothing parses that line in this repo; the docs line is updated.The architect ruled WARN, with the banner fix (valid until 2026-10-22).
What changed:
checkDefaultAgent(internal/coexistcheck/coexistcheck.go) reads the effective default agent,settings.local.jsonoversettings.json. It warns only when the agent carries the foreign<pack>:prefix and an identity of that pack is suppressed in the repo. The warning names the agent and both remedies:sideshow adopt <pack> --rewrite-agentandsideshow activate <pack>.PassVerdict(warns)returnspreflight cleanonly for zero warnings and zero errors, elsepreflight passed with N warning(s).coexist-checkuses it for its last line.adopt.go:215) can follow printed WARNs: the dry run prints the preflight findings, then the four plan steps, then this line, so a WARN is still on screen above it. It uses the same rule:preflight passed with N warning(s) and every plan step resolves: the real run would proceed.dryRunPreflightnow returns the warning count it printed.docs/repo-bindings-enablement.mddescribes both endings.Pins (red in 1c96b21, green in the fix commit):
settings.json, identity suppressed: WARN naming both remedies, and the end-to-endcoexist-checkrun ends onpassed with 1 warning(s).settings.local.jsonover a cleansettings.json: fires.settings.json: does not fire.preflight clean, forcoexist-checkand for the adopt dry run.Mutants, each compiled and killed by a named test: suppression ignored (
..BesideEnabledIdentityDoesNotWarn); committed file read over local (..InLocalSettingsWarns,..CleanLocalAgentOverForeignCommittedDoesNotWarn); any agent fires (..BoundAgentBesideSuppressedIdentityDoesNotWarn); verdict always clean (the three banner tests); verdict never clean (the two no-warning controls); adopt drops the count (TestAdopt_DryRunCountsWarningsInItsVerdict);coexist-checkignores warnings (its banner tests); finding raised as INFO (the three WARN tests).Gates, each run on its own: build, vet,
go test -count=1 ./..., golangci-lint (0 issues), gofumpt -l (empty).Closes #95