Skip to content

Adoption UX: honest first-adoption verdict, custom-manifest protection, agent detection - #297

Open
pengfei-threemoonslab wants to merge 12 commits into
mainfrom
claude/adoption-ux-iter3
Open

Adoption UX: honest first-adoption verdict, custom-manifest protection, agent detection#297
pengfei-threemoonslab wants to merge 12 commits into
mainfrom
claude/adoption-ux-iter3

Conversation

@pengfei-threemoonslab

@pengfei-threemoonslab pengfei-threemoonslab commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Three debts from the coding-agent adoption reviews, in one branch, one commit each.

1. A first adoption is not a policy weakening

Adding the manifest to a repository that had none is the first verdict every new adopter sees. It said:

This PR weakens the release policy that evaluates it; a coding agent cannot self-approve that change — a human must review it.

carried a finding titled "Policy change cannot be proven safe (no base snapshot)", and — because a missing-manifest base counted as a safe_recovery — shipped no fix_task at all. The run said stop and named nothing to do.

Adoption is now proved from git rather than inferred from the diff, and proved the hard way: the comparison base must carry no manifest under any name. A PR that moves the manifest while loosening it also finds nothing at the configured path and would otherwise get to call itself a first adoption — reproduced, and now closed (test_moved_manifest_cannot_pass_itself_off_as_an_adoption, plus a live check against a rename-and-loosen diff, which still gets the "cannot be proven safe" copy).

Unchanged: check id, medium severity, human_review_required, can_merge_without_human=false. Only the claim about what happened changed, plus a fix_task that now names the one act that clears it. check gets the same correction locally, keyed on the diff carrying exactly one manifest record and that record being a plain addition — a composite diff (new manifest plus an edit to a live one) keeps the generic wording, per #282's rule that a block-level "safe" signal must never soften a path-wide fail-closed guard.

2. Agent detection and the agent-mode error promise

check --agent defaulted to codex and never read the harness variables Shipgate already consults to switch agent mode on, so every Claude Code and Cursor run labelled its result and audit id codex. One table now defines both the hints and the actor they identify.

The skills and slash command promise that with AGENTS_SHIPGATE_AGENT_MODE=1 a failing command emits a structured next_action line on stderr. check, audit, and preflight printed prose only. Each error path now emits the line with its exit_code, and preflight no longer lets an unexpected failure escape as a bare traceback.

3. Withdrawn: one verify per turn (#295)

Attempted, reviewed twice, and removed from this PR. Its soundness rests on input_set_id being the complete input set, and it is not — adapter inputs such as openai_api.prompt_files never enter it, so an ignored one can change with neither the plan identity nor a worktree digest moving. The Stop hook is a standalone script that cannot compute the real identity, so every cheap approximation leaks somewhere. It was a latency optimization with no correctness benefit and it produced six review findings.

#295 stays open with every constraint recorded, and the incomplete input set is filed as #299 because it also underpins receipts and attestations.

3. Correctness fixes from review

  • The manifest a run actually loaded is a trust root. The table only knew **/shipgate.yaml, so --config new-gate.yml had no manifest trust root at all — its gate could be introduced or rewritten with an empty release substrate, and a clean scan then produced passed/mergeable/complete beneath an adoption headline saying a human was required. One shared predicate now serves the trust-root check, the policy fail-safe, and the adoption wording.
  • policy_weakened is derived from the evidence kind, so an adoption stops being recorded as a weakening by the registry, attestations, feedback's gate-bypass alarm, and reviewer routing.
  • Rerun commands carry the whole evaluated request — config, base (or an explicit --no-base), policy packs, baseline, ci-mode, fail-on, plugin and heuristic flags — because a rerun that drops them evaluates a different question.
  • Preflight recovery reproduces the invocation that failed instead of a bare preflight --json, which discarded the target and answered a question nobody asked.
  • Agent-mode errors follow docs/errors.json: both next_action and next_actions[], only published kinds, and filesystem failures as other_error/4.
  • Adoption wording requires every touched policy surface to be the manifest; ls-tree failure is "cannot prove"; git rename/delete detection catches old-gate.yml → new-gate.yml.

Verification

  • Full suite green (ruff and pytest run unpiped); fixture run ai_generated_refund_pr still blocked.
  • End to end on scratch repos: first adoption via init --writeverify (both the local worktree path and the CI --base/--head path) produces the introduction headline, manifest_introduced evidence kind, and a populated fix_task; after committing, the ordinary wording returns.
  • CLAUDECODE=1 / CURSOR_TRACE_ID / bare shell each produce the right actor; explicit --agent still wins.
  • A custom-named manifest (--config new-gate.yml) is now protected in verify, check, and preflight, and stays out of the graded agent route; a base that keeps its own manifest under another name no longer reads as a first adoption.
  • Recovery commands were replayed by hand: the check correction preserves actor/workspace/config/policy, the verify rerun runs clean from another directory, and stdin-backed or conflicting requests offer a review action instead of an unreplayable command.

Found in passing, not fixed here

ci_mode_weakened cannot fire through the real --base path. _prepare_base_report scans the base with ci_mode="advisory", prepare.py applies that by mutating the manifest, and final_report.py builds effective_policy from the mutated manifest — so every cached base snapshot records advisory and a strict→advisory downgrade compares equal. Existing coverage misses it because the tests inject the base EffectivePolicy directly. Filed as #298; such a PR still routes to human review via SHIP-VERIFY-TRUST-ROOT-TOUCHED, so it is a missed specific finding, not a silent merge.

🤖 Generated with Claude Code

pengfei-threemoonslab and others added 3 commits July 28, 2026 16:20
Adding the manifest to a repository that had none is the first verdict
every new adopter sees, and every surface got it wrong: the headline said
the PR weakens the policy that evaluates it, the fail-safe finding was
titled "Policy change cannot be proven safe", and a missing-manifest base
counted as a safe recovery, which nulled the fix_task — so the run said
stop and named nothing to do.

Adoption is now proved from git rather than inferred from the diff, and
deliberately proved the hard way: the comparison base must carry no
manifest under any name. A PR that *moves* the manifest while loosening
it also finds nothing at the configured path, and would otherwise get to
call itself a first adoption.

What did not change: the check id, the medium severity, the
human_review_required state, and can_merge_without_human. Adopting a
release policy is still a human decision. Only the claim about what
happened changed — plus a fix_task that now names the one act that
clears it.

`check` gets the same correction locally, keyed on the diff carrying
exactly one manifest record and that record being a plain addition. A
composite diff (new manifest plus an edit to a live one) keeps the
generic wording, per PR #282: a block-level "safe" signal must never
soften a path-wide fail-closed guard.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`check --agent` defaulted to codex and never read the harness variables
Shipgate already consults to switch agent mode on, so every Claude Code
and Cursor run labelled its result and audit id `codex`. One table now
defines both the hints and the actor they identify, so the two cannot
drift apart. An explicit --agent still wins; a plain shell still gets
codex.

The skills and slash command promise that with
AGENTS_SHIPGATE_AGENT_MODE=1 a failing command emits a structured
next_action line on stderr. check, audit, and preflight printed prose
only — an agent that mis-invoked them had to parse English or guess.
Each error path now emits the line with its exit_code, matching what
scan and verify already do, and preflight no longer lets an unexpected
failure escape as a bare traceback.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A governed turn verified twice. The coding agent ran verify because the
previous result's next_action told it to, and the Stop hook then ran an
identical verify because it had no way to know that happened — a second
or more of every turn spent recomputing a result nobody had questioned.

verify now records finished worktree runs in the git directory, next to
the last_verified_signature cache the hook already keeps, and the hook
reports that run when it can prove the repository has not moved.

The first attempt at this was reverted for two fail-opens, and the
design is shaped by both. It reads nothing from the workspace: the
reports directory is writable by the agent under evaluation, so a
verdict read from it is a verdict the subject can author. And identity
is a content digest — HEAD's tree plus the blob hash of every changed
and untracked file — not diff text. Diff text depends on how it was
asked for, and an untracked file's content never appears in `git diff`
at all, so a digest over diff text calls two different worktrees the
same. The commit-after-verify stale pass that sank the first attempt is
now impossible by construction, and is a regression test.

Config, effective CI mode, base ref and the commit it resolved to must
all match too; a forced --ci-mode rewrites the manifest's mode for the
run, and the policy-weakening check compares that value against the
base. Any mismatch, an unreadable record, or an identity that cannot be
established re-verifies.

The hook cannot import Shipgate — it is a standalone rendered script —
so the digest exists twice. A test runs both implementations against a
live repository and compares them; if they ever drift the digests stop
matching and the hook simply verifies for itself.

Forging the record can only make an advisory hook echo a forged state,
which forging last_verified_signature already allowed. PR-time verify
and CI read none of it.

Closes #295

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pengfei-threemoonslab and others added 3 commits July 28, 2026 16:59
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every finding reproduced. Fixes, in the reviewer's order:

1. The fix_task rerun command omitted --config, invented origin/main,
   and always appended --head HEAD. Verified: a nested-manifest
   uncommitted adoption emitted a command that exits 2. It now emits the
   config always, a base only when one was used, and no --head for a
   working-tree run.

2/3/4. Reuse now has to identify the actual verification request. A run
   is recorded only when it is shaped like the hook's own invocation —
   no baseline, diff reference, policy pack, authorization, fail-on set,
   or non-default plugin/heuristic mode — and any other run clears the
   record instead of leaving a stale one. The identity hashes the HEAD
   commit, not only its tree, because an empty commit leaves the tree
   identical while moving the clock verify evaluates expiries against. A
   git-ignored scan input is read by the scan and invisible to the
   digest, so its presence vetoes the record outright rather than being
   silently uncovered. The identity is captured before the scan and
   re-checked at write time, closing the window where an edit lands
   mid-scan and binds the old verdict to the new state; the base commit
   gets the same treatment. The hook compares the effective base, head
   and ci-mode it is about to use, not the raw installed arguments.

5. The new error emitters carried only the legacy next_action string.
   docs/errors.json states the line always carries next_actions[] too,
   and that the kind is one of its published ids — `shipgate_error` was
   not. Both now derive from one NextAction so they cannot disagree, and
   a test asserts the documented envelope against docs/errors.json
   itself rather than against what we happen to emit.

6. policy_weakened stayed true through an adoption, so the registry,
   attestations, feedback's gate-bypass alarm and reviewer routing all
   inherited a contradiction with a headline saying the opposite. It is
   now derived from the evidence kind, not the check id alone. The
   fail-closed property does not move: an adoption must contain the
   manifest in its own diff (which is also the literal claim being
   made), so trust_root_touched is structurally set, and
   can_merge_without_human stays exactly the pure projection its schema
   validates it to be.

Plus the three P2s: the adoption wording now requires every touched
policy surface to be the manifest, so a diff that also edits an existing
policies/** file keeps the strict copy; ls-tree failure is "cannot
prove" rather than proven absence, and git's own rename/delete detection
catches old-gate.yml -> new-gate.yml, which no name check could; audit
reports an unwritable --out as a config error instead of a traceback and
exit 1; and preflight still prints the traceback under --verbose.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Withdrew the Stop-hook verify reuse (#295) rather than patch it again.
Findings 2 and 3 were both in that mechanism, and finding 3 explains
why they keep appearing: its soundness rests on input_set_id being the
complete input set, and it is not — adapter inputs such as
openai_api.prompt_files never enter it, so an ignored one can change
with neither the plan identity nor a worktree digest moving. The hook is
a standalone script that cannot compute the real thing, so every cheap
approximation leaks somewhere. It was a latency optimization with no
correctness benefit, and two review rounds have now found real
fail-opens in it. #295 stays open with these constraints recorded; the
incomplete input set is filed separately because it also underpins
receipts and attestations.

1. The trust-root table only knew **/shipgate.yaml, so a run pointed at
   --config new-gate.yml had no manifest trust root at all: its gate
   could be introduced or rewritten with an empty release substrate,
   and a clean scan then produced passed/mergeable/complete beneath an
   adoption headline claiming a human was required. Whatever a run loads
   as its gate is now classified as one, through a single shared
   predicate used by the trust-root check, the policy fail-safe, and the
   adoption wording — three sites that previously would have disagreed
   about path spelling.

4. Rerun commands now carry the whole evaluated request: policy packs,
   baseline and mode, diff reference, ci-mode, fail-on, plugin and
   heuristic flags, authorization, and an explicit --no-base. The last
   is the subtle one — without it a rerun auto-detects a base the
   evaluated run never compared against.

5. Preflight recovery reproduces the invocation that failed instead of
   recommending a bare `preflight --json`, which discarded the
   workspace, config, plan and diff and answered a question nobody
   asked. When the request came from stdin it offers a review action
   rather than a command it cannot reproduce.

6. Host-audit filesystem failures are other_error/4 per docs/errors.json
   on both --baseline-file and --out; a directory passed as
   --baseline-file no longer escapes as a traceback and exit 1.

7. The adoption repair names the resolved config path, not a hardcoded
   shipgate.yaml.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@pengfei-threemoonslab pengfei-threemoonslab left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of 187757267903174cafce8d2950c9a23fa256cdbd after the latest update.

The reuse/TOCTOU findings from the prior round are resolved: the optimization was fully removed. The rerun command also now preserves config, base/--no-base, policy packs, baseline, CI/fail policy, plugin flags, patch/heuristic flags, and authorization, and the structured adoption repair targets the configured manifest.

I found 9 remaining actionable issues: 4 P1 and 5 P2. I recommend addressing these before merge. Validation performed: Ruff passed; source verification and receipt reproduction passed; live GitHub checks are green; the full local suite had only sandboxed isolated-build dependency errors, and tests/test_packaging.py passed 8/8 when rerun with network access.

PR metadata cleanup: the title and the final Verification bullet still advertise the removed “one verify per turn” behavior.


config = getattr(context, "config_path", None)
if not is_configured_manifest(config, path):
return None

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Keep custom-manifest evidence deterministic.

For a committed-head run, context.config_path points inside a freshly named agents-shipgate-verify-head-* archive. Returning it as matched_glob persists that random path in finding evidence, and finding identity hashes this evidence. Two identical runs had the same request ID but different fingerprints and report run IDs. Please emit a stable logical/workspace-relative manifest path instead; the normalized changed path is sufficient.

item
for item in diff_files
for candidate in (item.new_path, item.old_path)
if candidate and trust_root_class_for(candidate.replace("\\", "/")) == "manifest"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Protect the configured manifest in check and preflight, not only in verify checks.

The configured-manifest predicate is not threaded into these earlier control surfaces. A diff for new-gate.yml loaded via --config new-gate.yml produced check decision allow with no violations, while preflight reported no protected touch and requires_human_review=false; nested configured manifests also miss the local boundary filter. Both routes then emit verify commands for the default shipgate.yaml. Please pass the resolved configured-manifest identity through boundary/preflight classification and exact command construction.

contain shell metacharacters, so every interpolated value is quoted.
"""

parts = ["agents-shipgate", "verify"]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve the evaluated workspace in the exact rerun command.

This builder still has no workspace or output-directory input. When verification is invoked from outside its target repository, the emitted command runs in the caller directory, where it can fail or evaluate another checkout; with custom --out, it also leaves the requested artifact directory stale. Please include and quote the resolved --workspace and selected --out (and consider other output-shaping request options such as --pr-comment-style).

Comment thread src/agents_shipgate/cli/check.py Outdated
"--format agent-json was removed in the 0.14.0 contract cleanup. "
"Use --format agent-boundary-json.",
err=True,
command="agents-shipgate check --format agent-boundary-json",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Keep structured recovery on the same check request.

These fixed commands discard all valid arguments surrounding the bad field. For example, an invalid format on a cursor run against another workspace emits only agents-shipgate check --format agent-boundary-json, switching actor, workspace, config, policy, and diff/range context. Following the documented rank-1 action can therefore answer a different boundary question. Rebuild it from the original request, changing only the invalid value and shell-quoting arguments; stdin-backed input should not claim a replayable command.

ref = head
else:
return False
names = _MANIFEST_FILE_NAMES | {config_relative.name}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Do not prove first adoption by checking only the default and new basenames.

If the base retains an operational custom manifest under another name, no YAML is removed, so this can incorrectly prove adoption. Repro: base retains valid strict old-gate.yml; head adds configured advisory new-gate.yml without deleting the old file. The result was manifest_introduced, policy_weakened=false, and a headline claiming the base carries no manifest. Please fail closed unless absence of an existing manifest can actually be established.

as "a trust root is in play" — ``_can_merge_without_human`` above all —
fail-closed regardless of the other two flags.
"""
if manifest_introduced:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Do not let adoption wording override mixed existing-policy changes.

manifest_introduced unconditionally selects this headline even when capability_review.policy_weakened is true. Adding a manifest while editing an existing policy produced a base_snapshot_unavailable policy finding, yet the headline said there was no prior gate to weaken, the fix task said nothing existing was weakened, and review_policy_weakening was omitted. Pure-adoption wording should require no other policy/trust-root change needing review, or the result should surface both actions.

Comment thread src/agents_shipgate/cli/check.py Outdated

# The actor lands in the result and in the audit id, so an undetected
# harness mislabels every row it writes. An explicit flag always wins.
agent = agent or detect_actor()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Bind the detected actor into the audit identity.

The result’s actor field now changes, but the central audit digest omits actor and the legacy digest still hardcodes codex. Identical evaluations for codex, claude-code, and cursor therefore return the same audit_id, leaving the audit-attribution issue described by this PR unresolved. Please include the effective actor in the audit identity, or revise the claimed contract if caller-independent IDs are intentional.

Comment thread src/agents_shipgate/cli/preflight.py Outdated
) -> None:
"""Run the proactive static preflight contract for coding agents."""

rerun = _rerun_command(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Do not emit a command that repeats an invalid preflight request shape.

The rerun is built before mutually exclusive plan flags are rejected and is reused for every ConfigError. With both --plan CHANGELOG.md and --changed-files CHANGELOG.md, the rank-1 kind=command action contains both flags again, so it can never satisfy its own expects without manual modification. Emit a review action for ambiguous request-shape conflicts or construct an explicitly normalized valid command.

Comment thread src/agents_shipgate/cli/host_audit.py Outdated
if drift:
try:
baseline = load_host_grants_baseline(resolved_baseline)
except OSError as exc:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] This filesystem-error handler is unreachable for baseline reads.

load_host_grants_baseline() converts every read OSError into ValueError, so using a directory as --baseline-file returns config_error/2 and recommends bare agents-shipgate audit --host, dropping drift, workspace, and baseline path and performing an unrelated inventory. Preserve/inspect the underlying I/O cause and route it through the documented filesystem-error path with a real path-repair action.

1. Finding evidence carried the resolved config path, which for a
   committed-head run is a freshly named verify-head archive. That value
   is hashed into the fingerprint, so two identical runs produced
   different fingerprints and report run ids. It now carries the
   normalized changed path.

2. The configured manifest was protected only in the verify checks.
   `check` returned allow with no violations and preflight reported no
   protected touch for a diff that rewrote a repository's own gate under
   a non-default name — and the untracked-content filter dropped it
   before the evaluator ever saw it. The identity is now threaded through
   diff collection, boundary classification, and preflight, and the
   resulting class is recorded on the boundary row so the graded agent
   route excludes it without every band call site needing the config.

3. Rerun commands carry the resolved --workspace and a non-default --out,
   so a rerun from elsewhere evaluates the same checkout and does not
   leave the requested artifact directory stale. `check` recovery is
   rebuilt from the failing request with only the invalid field
   corrected; a fixed command discarded actor, workspace, config, policy
   and diff context, answering a different boundary question. Preflight's
   verify command echoes the request as the caller spelled it rather than
   a resolved absolute path, which would embed one machine's checkout
   location in a portable artifact.

4. Adoption is no longer proved by two basenames. A base that keeps an
   operational manifest under another name deletes nothing and matches no
   name check; absence is now established by asking git for any tracked
   YAML carrying the manifest's required top-level keys. It over-matches
   by design — the cost of a false match is the plainer wording.

5. Adoption wording stands down when policy_weakened is set: introducing
   a manifest while editing an existing policy file was describing away
   the very finding that needed review, and dropping the
   review_policy_weakening repair with it.

6. The audit id now distinguishes the actor. Detection changed the label
   but not the digest — the central one omitted it, the legacy one
   hardcoded codex — so all three agents shared an audit_id, which is the
   attribution problem detection exists to solve. Codex ids are
   unchanged; the other goldens are regenerated, audit_id only.

7. Preflight offers a review action rather than replaying a request-shape
   conflict that can never satisfy its own expects, and the host-audit
   baseline reader recovers the I/O cause from __cause__ (the loader
   converts every read OSError to ValueError) while a genuinely missing
   baseline keeps its documented config_error/2 contract.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pengfei-threemoonslab pengfei-threemoonslab changed the title Adoption UX: honest first-adoption verdict, agent detection, one verify per turn Adoption UX: honest first-adoption verdict, custom-manifest protection, agent detection Jul 29, 2026

@pengfei-threemoonslab pengfei-threemoonslab left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed exact head 71047f6. The common-path fixes for deterministic custom-manifest evidence, configured-manifest classification, mixed adoption/policy wording, full verify reruns, and actor-distinct attribution are real. I still found nine reproducible issues: seven P1 and two P2, detailed inline.

Validation: GitHub CI is green; Ruff and diff checks pass; all non-packaging tests passed, and the packaging module passed 8/8 with isolated-build network access. Shipgate produced a valid reproducible receipt with mergeable / passed and no reported blockers. Its local boundary result nevertheless remains agent_action_required after the exact successful verify rerun, consistent with the wrong-target verification-action finding below.

Comment thread src/agents_shipgate/cli/host_audit.py Outdated
"--save-baseline`, or point --baseline-file at a valid one."
),
command=(
"agents-shipgate audit --host --workspace "

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Never overwrite a failed baseline. All non-I/O baseline load failures emit a rank-1 --save-baseline command targeting the same path. In a disposable repo, following this recovery for an existing malformed file replaced it with baseline JSON and acknowledged the current grants without human review. It also drops --scope local-static. Missing, malformed, or integrity-failed baselines must route to review or a non-destructive correction.

Comment thread src/agents_shipgate/cli/verify/git.py Outdated
"--all-match",
"-l",
"-e",
"^project:",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Parse retained manifests structurally. This proof recognizes only exact unquoted project: and agent: lines. A valid retained manifest using quoted YAML keys loads successfully, but the probe returns false and _manifest_introduced reports a first adoption with policy_weakened=false. Enumerate and parse candidate YAML blobs, failing closed on unreadable results.

),
input_mode=input_mode, # type: ignore[arg-type]
input_issues=input_issues,
config_path=config_path,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Bind check verification to its target. The configured manifest now reaches classification, but an ordinary force-run checked with --config shipgate-self.yaml still authorizes only agents-shipgate verify --json, dropping both workspace and config and evaluating the default manifest. Thread an invocation-specific verify command through the boundary-control projection.

Comment thread src/agents_shipgate/core/preflight.py Outdated
"agents-shipgate",
"verify",
"--workspace",
shlex.quote(workspace.as_posix()),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Nested preflight can verify the wrong gate. Preflight interprets config relative to its requested workspace, while verify first replaces that workspace with the Git root and then resolves config. With distinct root and services/api manifests, this emitted command succeeded but generated a report for the root gate. Render config relative to the Git root or make verify honor the requested workspace.

Comment thread src/agents_shipgate/core/trust_roots.py Outdated
candidate = path.replace("\\", "/").strip()
if not configured or not candidate:
return False
return (

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Canonicalize configured-manifest identity. Raw suffix comparison is bypassable by equivalent path spellings. Configuring docs/engineering/../manifest-v0.1.example.full.yaml and changing the normalized path returned allow with zero violations; the same logic can false-positive unrelated suffixes. Compare normalized, containment-checked workspace-relative identities.

Comment thread src/agents_shipgate/core/preflight.py Outdated
]
touches = classify_protected_touches(changed)
verify_command = _verify_command(workspace, config)
touches = classify_protected_touches(changed, config_path)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Include rename source paths. The diff-derived changed set keeps only each destination path. Renaming configured shipgate-self.yaml to an unrecognized name produced no protected touch, requires_human_review=false, and an agent-action route. Include both old_path and new_path, deduplicated, before classification.

Comment thread src/agents_shipgate/cli/check.py Outdated
parts.extend(["--config", shlex.quote(str(config))])
if policy is not None:
parts.extend(["--policy", shlex.quote(str(policy))])
if diff:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Recovery is still injectable and incomplete. This safe serializer is used only for flag errors. Diff-input failures still call the legacy builder, which joins unquoted user-controlled paths/refs, hardcodes workspace to ., and drops config/policy. A semicolon in --diff becomes a shell command boundary in the allowed action; a one-sided range plus another bad flag is silently dropped. Use one complete quoted serializer and return review when repair is ambiguous.


payload = {
"schema": AGENT_BOUNDARY_RESULT_SCHEMA_VERSION,
"actor": actor,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Existing Codex audit IDs rotate. Adding actor unconditionally changes established Codex IDs despite the changelog saying Codex IDs remain unchanged. The docs-only golden rotates from agent_boundary_bcae9dc247f5dae731a41e8c to agent_boundary_6992d494be4b9b5ad459b3e3. Preserve the legacy Codex digest payload or explicitly version the identity contract.

input_mode: Literal["worktree", "git_range", "provided_diff"] = "provided_diff",
input_issues: list[BoundaryInputIssue] | None = None,
host_snapshot: HostBoundarySnapshot | None = None,
config_path: Path | None = None,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Pass the configured manifest into the cached assessment. The new parameter is supplied by local check but not by assessment_for_scan_context. Consequently, a custom-manifest diff gets the protected-surface boundary finding locally but not in full verify. Tier-A trust-root detection still routes review, but the public surfaces publish inconsistent evidence.

pengfei-threemoonslab and others added 5 commits July 29, 2026 12:45
1. A malformed, unknown-schema, or integrity-failed host-grants baseline
   recommended --save-baseline against the same path. Following that
   recovery overwrote the failed artifact with the current grants,
   acknowledging them unreviewed and destroying the evidence a human
   needed to see. Those route to review now; only a genuinely absent
   baseline gets a record command, and it carries the requested --scope.

2. The retained-manifest probe grepped for unquoted `project:`/`agent:`.
   A manifest with quoted keys loads fine and was invisible to it, so the
   base looked empty and the run claimed a first adoption. Candidates are
   parsed now, bounded by count and size, failing closed on anything
   unreadable. (The git pathspec I first used silently matched nothing,
   which would have made the probe answer "no manifest" for every tree —
   filtering happens in Python.)

3. `check` authorized a bare `agents-shipgate verify --json`, dropping
   the workspace and manifest it had just evaluated. The command is
   threaded through the boundary control projection, and the config is
   rendered the way verify resolves it — relative to the repository root
   — which is the same defect preflight had for nested manifests. Both
   now share one builder.

4. Configured-manifest identity is compared on normalized,
   containment-checked paths. `docs/x/../manifest.yaml` returned allow
   with zero violations against the normalized changed path. Callers that
   know their workspace pass it, making the comparison exact; the
   fallback over-matches, which adds a finding rather than dropping one.

5. Preflight kept only the destination of a rename, so renaming the
   configured manifest to an unrecognized name produced no protected
   touch and an agent-action route. Both sides of every record are
   classified.

6. `check` recovery is one quoted serializer for every path. The
   diff-input failure still used the legacy builder, which joined
   user-controlled paths and refs unquoted into a published *authorized*
   command and hardcoded the workspace. A one-sided ref range keeps its
   documented "omit both and check the working tree" repair on that path
   and only that path; elsewhere dropping a requested range silently
   answers a different question, so it returns review.

7. Existing codex audit ids no longer rotate: the actor is folded into
   the digest only when it is not the default, so every id issued before
   detection existed keeps its value. My previous changelog claimed this
   while the diff did the opposite.

8. `assessment_for_scan_context` passes the configured manifest, so the
   local check and full verify stop publishing different evidence for the
   same diff.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant