feat(cli): name failing assets and drift cause on actor_guidance_unverified refusals - #383

Fleet review (deepseek)

Lens: evidence + test design. PR #383 (part 2 of the #373 chain) adds a detached ActorGuidanceRefusal record (stage, per-asset cause, observed/canonical modes, bounded shadow role/location) and renders additive asset=... path=... cause=... lines before the unchanged status=conflict reason=actor_guidance_unverified line. The evidence-only contract is well guarded: the refusal record cannot become report/proof, Refusal() returns copies, and tests compare trees before/after to prove nothing is written. Three gaps: the evidence-stage path lets mode drift win the cause switch so a content+mode-drifted asset loses its content-drift fact (the conflict stage reports both facts); docs/operating.md describes the opposite output order and omits the mode_drift=true token the lines actually carry; and no test covers the combined-content+mode case at the evidence stage or pins that a conflict-stage refusal always names at least one asset.

Suggestion (2)

• Docs describe the status line followed by asset lines; the writer emits asset lines first (docs/operating.md:101): docs/operating.md:101 says the plan "prints status=conflict reason=actor_guidance_unverified (exit 3) followed by one asset= line per failing asset", but writeActorGuidanceRefusal (internal/cli/update.go) writes header, then the asset lines, then the status line — matching the PR body ("before the unchanged status line") and not the docs. The documented samples at docs/operating.md:104-105 also omit the mode_drift=true token that both the mode-drift and the combined content+mode lines actually emit, so a reader comparing sample to output sees a field they cannot explain. Suggest either flipping the docs sentence or moving the status line, and adding the token to the samples.
• Two refusal conditions that would reproduce the issue are untested (internal/cli/update_guidance_test.go:255): The new CLI tests cover content drift, mode drift, combined drift (on a skill, through the conflict stage), and shadow, but two conditions that the issue's complaint depends on are unpinned. (a) Combined content+mode drift at the evidence stage: edit an actor file's bytes AND chmod 0640 so the preflight refuses at ActorGuidanceRefusedEvidence, then assert the emitted line reports the content-drift fact as well as the modes — this is exactly where the two stages currently disagree. (b) Empty asset list: a conflict-stage refusal where no classified decision has Action=ownership.Conflict produces zero asset lines and degrades back to the bare reason token; a test should pin either that this cannot happen or the documented behaviour when it does. Both are condition-level statements only; no test code is suggested here.

Question (1)

• Evidence-stage cause switch lets mode drift hide content drift (internal/installcoord/actor_guidance.go:212): recordedDriftAssets puts case asset.ModeDrift: before case !owned || ownerHash != slot.SHA256:, so an asset whose bytes AND mode both drifted is recorded with Cause=ActorGuidanceModeDrift and ModeDrift=true, and actorGuidanceRefusalAssets renders it as cause=mode_drift mode_drift=true observed_mode=... canonical_mode=... with no content-drift fact at all. conflictAssets, by contrast, keeps Cause=ContentDrift and reports the mode facts alongside it. The PR body claims "both facts are reported" for content-drifting assets whose mode also drifted, and the docs describe cause=content_drift as meaning the bytes differ. The result for the operator: they are told the file failed on mode, fix the mode, rerun, and get a second refusal. Is the asymmetric precedence intentional, or should the evidence stage mirror the conflict stage and carry the content fact too?

Verdict: ready to merge

Fleet review (glm)

Lens: requirements + adversarial. The diff satisfies the #373 update criterion: each refused actor-guidance update now prints per-asset diagnostics naming the failing file and its cause (content drift, mode drift with observed/canonical modes, or shadow role with bounded location), while keeping the stable reason token, exit 3, opaque stderr, and no-write behavior, all pinned by new coord and CLI tests including the combined content+mode case. Two minor docs/implementation mismatches and one evidence-stage cause-merging question remain; none blocks merge.

Suggestion (2)

• Docs say the status line comes first; the code prints asset lines before it (docs/operating.md:101): The new doc sentence says the plan prints 'status=conflict reason=actor_guidance_unverified' followed by one asset= line per failing asset, but writeActorGuidanceRefusal (internal/cli/update.go) emits the asset= lines first and the status line last. One of the two is wrong; pick one order and align.
• Mode-drift doc example omits the mode_drift=true field the code always emits (docs/operating.md:106): actorGuidanceRefusalAssets appends 'mode_drift=true' to every cause=mode_drift line (and to content-drift lines that also drifted in mode), but the documented example line shows only 'cause=mode_drift observed_mode=0640 canonical_mode=0600'. Consumers parsing the documented shape will not expect the extra field.

Question (1)

• Evidence-stage asset with content+mode drift reports only the mode cause (internal/installcoord/actor_guidance.go:213): recordedDriftAssets prioritizes ModeDrift over content drift in its switch, so a captured asset that drifted in both bytes and mode is reported with cause=mode_drift and no content-drift fact, while the conflict stage (conflictAssets) reports the content cause plus the mode facts. The file is still named and the modes are shown, so this may be intentional, but the two stages report the same physical state differently. Can you confirm the asymmetry is deliberate (or record the content-drift fact in the evidence stage too)?

Verdict: ready to merge

Fleet review (qwen38)

Lens: evidence. Second-line review of part 2 of the #373 chain (actor_guidance_unverified per-asset diagnostics). The evidence design is strong overall: installcoord tests pin the refusing stage, per-asset cause and mode facts, the detached/evidence-only invariant (report and proof stay unusable, refusal cannot be mutated back), and tree-comparison proof that a refusal never writes; CLI tests pin exit 3, empty stderr, the stable reason token, relative-only paths, and no writes. Two gaps where the PR's stated claims outrun its evidence: (1) on the evidence-stage refusal path, an asset that drifted in BOTH content and mode is reported as cause=mode_drift only — the content-drift fact is dropped, which contradicts the PR body's 'both facts are reported' claim and the conflict-path behavior the PR itself pins in a test, and it can re-create the trial-and-error loop issue #373 exists to end; (2) the body claims the apply re-check gate now carries refusal evidence, but no hunk touches the re-check code and no end-to-end test exercises it (every new CLI refusal fails at preflight, before the re-check).

Blocking (1)

• Evidence-stage refusal hides content drift when an asset also drifted in mode (internal/installcoord/actor_guidance.go:215): In recordedDriftAssets the switch tests case asset.ModeDrift: first, so an asset whose bytes differ from the recorded digest AND whose mode differs gets Cause=ModeDrift and the content-drift check is never evaluated. The CLI renderer (internal/cli/update.go, actorGuidanceRefusalAssets) then prints only cause=mode_drift observed_mode=... canonical_mode=... for that asset — no indication the bytes also drifted. This contradicts the PR body's claim that mode_drift 'also fires for content-drifting assets whose mode also drifted: both facts are reported', and it contradicts the invariant the same PR pins for the conflict path (TestUpdateGuidanceRefusalNamesContentAndModeDrift keeps cause=content_drift plus mode facts). The real-world install/uninstall/update ownership conflicts fail closed without naming the conflicting asset paths or drift cause #373 scenario (externally authored agents/cortex-.md under unadopted/registered guidance, typically not 0600) runs through exactly this evidence stage. No test covers the combined content+mode case on the evidence stage.
  • Failure scenario: A Pi actor guidance file has bytes appended by an external editor and its mode is 0640 (canonical 0600). cortex update --runtime pi refuses with only asset=actors/<role> path=... cause=mode_drift observed_mode=0640 canonical_mode=0600; the content drift is invisible. The operator chmods 0600 as instructed, re-runs, and is refused again — now with cause=content_drift — reintroducing the multi-round guessing that issue install/uninstall/update ownership conflicts fail closed without naming the conflicting asset paths or drift cause #373's expected outcome ('the output names which file failed verification and why: content drift vs mode drift') exists to eliminate.

Suggestion (1)

• Verification claims rest on local runs with named pre-existing failures (internal/cli/update_guidance_test.go:143): 'Suites green; the two pre-existing environment-dependent CLI failures behave identically to main' is not reproducible from the PR text alone (no CI artifact referenced). The claims are testable — same two named failures are also cited in feat(cli): name conflicting asset paths on install and uninstall conflict refusals #382 — but a reviewer or CI cannot confirm the identical-to-main comparison without the author's transcripts. Linking the CI run or a main-vs-branch failure comparison would make the green claim evidence instead of assertion.

Question (1)

• Re-check-gate claim is not visible in the diff and not exercised by any new test (internal/cli/update.go:506): The PR body says 'PreflightActorGuidance and its re-check gate now carry detached refusal evidence', but the diff contains no hunk modifying the apply re-check path, and every new CLI refusal test fails at the preflight stage (the --apply variant of the content-drift test is refused before the re-check ever runs; TestUpdateGuidanceProductionApplyRechecks is unchanged and asserts no asset= lines). Where does the re-check refusal render its asset= lines, and is there an observable check that a mid-apply re-check refusal also emits them rather than the old bare status line?

Verdict: not ready

Fleet review (glmflash)

Lens: adversarial. The additive asset= diagnostics are well-built at the CLI layer: writeActorGuidanceRefusal keeps the header, stable status token, empty stderr, and exit 3 intact; paths stay runtime-relative and are omitted rather than invented when the candidate has no file for the identity; the detached refusal record is tested against mutation and never grants authority; and the combined content+mode drift case is correctly reported at the conflict stage with both facts. The adversarial pass found one real inconsistency between the two refusal-capture paths: at the conflict stage a both-drifted asset keeps cause=content_drift and adds the mode facts, but at the evidence stage recordedDriftAssets makes mode drift take precedence in its switch and silently drops the content-drift fact entirely — the renderer for cause=mode_drift prints only the two modes, so tampered bytes in an actor guidance file are hidden behind a chmod-level diagnosis. The docs added by this PR also describe the output in the wrong order.

Blocking (1)

• Evidence-stage refusal hides content drift when the mode also drifted (internal/installcoord/actor_guidance.go:208): recordedDriftAssets switches with case asset.ModeDrift: first, so an asset whose bytes AND mode both drifted gets Cause=ActorGuidanceModeDrift and no content-drift fact is captured anywhere. The struct contract says 'Cause is the single reason the asset refused verification; ModeDrift records the additional, independent fact', and the conflict-stage path (conflictAssets + actorGuidanceRefusalAssets) deliberately reports content_drift with mode_drift=true and both modes — the evidence-stage path breaks that contract in the opposite direction, silently dropping the content evidence. Actor guidance files are the prompt-injection surface; the primary fact (bytes changed) is exactly what issue install/uninstall/update ownership conflicts fail closed without naming the conflicting asset paths or drift cause #373 asked to be named.
  • Failure scenario: An actor file was edited externally (content drift) and also chmod'ed to 0640 (mode drift); adoption is not passed, so the preflight refuses at the evidence stage. The refusal prints cause=mode_drift observed_mode=0640 canonical_mode=0600 with no content-drift indication. The operator chmods the file back to 0600 — the exact fix the output named — reruns, and the update is still refused, now as cause=content_drift. The first diagnostic actively hid the tampering that caused the refusal, contradicting the PR body's own claim that for content-drifting assets whose mode also drifted 'both facts are reported'.

Suggestion (1)

• Docs describe the refusal output in the wrong order (docs/operating.md:101): The added docs say the plan prints status=conflict reason=actor_guidance_unverified 'followed by one asset= line per failing asset', but writeActorGuidanceRefusal emits header, then the asset= lines, then the status line last (as before). Anyone scripting against the documented order will misparse. Reword the docs to 'preceded by' or show the actual block order in the example.

Verdict: not ready

Fleet review (mimo)

Lens: requirements. Part 2 wires asset-level refusal evidence into the guidance preflight gate with RED/GREEN tests at both layers, satisfying issue #373 criterion 2 for every actor_guidance_unverified refusal the preflight produces - but the PR's own docs promise the same diagnostics for "apply re-check" refusals, which still exit through an opaque stderr token with no asset lines, leaving criterion 2's "(and any drift-based refusal)" clause unmet. The docs also state the print order backwards and describe only one of the two cases that emit cause=content_drift.

Blocking (1)

• Apply re-check refusals still name no asset and no cause (criterion 2, "any drift-based refusal") (internal/cli/update.go:518): Issue install/uninstall/update ownership conflicts fail closed without naming the conflicting asset paths or drift cause #373 criterion 2: "For actor_guidance_unverified (and any drift-based refusal), the output names which file failed verification and why: content drift vs mode drift." This PR routes only the preflight refusal (update.go:507-510) through writeActorGuidanceRefusal. When the preflight passes and the fresh re-check inside installtxn.ApplyActorGuidance refuses (mode-drift decision check at installtxn.go:38, fresh observation mismatch at :57, or shadow re-scan at :60), update.go:516-519 writes only error=update_transaction_failed to stderr and returns exit 4 - stdout carries no status line, no asset= line, and no content-vs-mode cause. The PR's own docs (docs/operating.md:101) state: "When a guidance preflight or apply re-check refuses, the plan prints status=conflict reason=actor_guidance_unverified (exit 3) followed by one asset= line per failing asset naming the failing file and the cause" - no code path produces that output for an apply re-check refusal. The refusal itself is reachable: this PR's own TestUpdateGuidanceProductionApplyRechecks pins that ApplyActorGuidance refuses after a passing preflight.
  • Failure scenario: cortex update --runtime pi --catalog ./catalog --apply: the preflight at update.go:507 passes; an actor file is modified (or a shadow appears) before ApplyActorGuidance's fresh re-check; ApplyActorGuidance returns a conflict; the CLI prints nothing on stdout, writes error=update_transaction_failed to stderr and exits 4 - no file named, no content-vs-mode cause - the opaque drift refusal criterion 2 forbids, and not the exit-3 status line plus asset= lines that docs/operating.md:101 promises.

Suggestion (3)

• Docs state the refusal print order backwards (docs/operating.md:101): docs/operating.md:101 says the plan prints status=conflict reason=actor_guidance_unverified (exit 3) "followed by one asset= line per failing asset", but writeActorGuidanceRefusal emits header, asset lines, then the status line (update.go:545-549); the PR body agrees with the code (asset= lines "before the unchanged status line"). An operator or parser following the docs looks for asset lines after status=conflict and finds none.
• content_drift docs describe only one of the two cases the code reports (docs/operating.md:108): docs/operating.md:108 says "cause=content_drift means the bytes differ from the recorded state", but the cause is also emitted when the captured asset "carries no recorded ownership at all" (ActorGuidanceContentDrift doc comment, installcoord/actor_guidance.go) - a byte-identical but unrecorded file is labeled content_drift while its bytes do not differ, so an operator who diffs against the candidate sees no difference and cannot tell why verification failed.
• Evidence-stage mode-drift cause silently drops the content-drift fact (internal/installcoord/actor_guidance.go:213): recordedDriftAssets checks mode drift before the recorded digest (actor_guidance.go:213-217): an asset that drifted in BOTH content and mode reports cause=mode_drift with no content marker, while the conflict stage reports cause=content_drift plus mode_drift=true. The PR body's claim that for content-drifting assets whose mode also drifted "both facts are reported" holds only for the conflict stage; in the evidence stage the content fact is never evaluated or rendered, and the docs paragraph lists no way to see it.

Verdict: not ready

Superseded by #435 (installcoord core) and #436 (CLI and docs, stacked on #435). This PR grew to 661 lines against main, over the 400-line review budget. The split keeps exactly the same final tree, verified with git diff, plus the #384 fix: content drift is reported before mode drift.