Commit eae59ed
authored
fix(verify): distinguish an unreadable diff from trigger no_match (#340)
* fix(verify): distinguish an unreadable diff from trigger no_match
`verify --preview` collapsed every diff-acquisition failure into one
message, then evaluated the trigger catalog against the empty inputs that
failure left behind — publishing `skip_reason: "no_match"` with the
rationale "nothing in this PR signals a tool-surface change" about a PR it
had never read. The control result stayed fail-closed, but the explanation
invited exactly the wrong conclusion. On a workspace with no manifest the
failure was not reported at all: both diff-failure branches were gated on
`manifest_present`, so a shallow or blobless clone of an un-adopted
repository — the normal shape of first contact — fell through to "Shipgate
is not configured in this workspace", with the Git error visible nowhere
but `base_notes`.
Three changes, at the three layers the defect actually spans.
Diff acquisition is classified rather than flattened. `not_attempted`,
`refs_missing`, `merge_base_missing`, `objects_missing`,
`metadata_limit_exceeded`, `body_limit_exceeded`, `git_timeout`, and
`git_failed` are read off Git's own diagnostic (stderr is now piped and
drained under a small cap instead of discarded) and travel on a new
`verifier.json` `diff_status` block with a bounded, path-redacted excerpt,
the precise repair, and whether fetching can perform it. Metadata and body
are collected separately, so a body that cannot be read no longer discards
the changed paths that were read successfully — a blobless clone answers
`--name-status` in full, and those paths are exactly what says a PR touches
an agent surface.
The trigger evaluator gained the state it was missing. `input_status` and
`evaluation_status`, with `should_run`, `run_shipgate`, `skip`, and
`skip_reason` all `null` when the inputs were not fully read. The asymmetry
is deliberate: rule matching is monotone in the evidence, so a run verdict
reached from partial evidence stays sound and is still published, while any
skip verdict is withheld. The stop block, which reasons over the very path
evidence that is missing, is reported as not evaluable.
And an unreadable diff now outranks every adoption route in preview,
manifest or not, routing `merge_base_missing`/`objects_missing` to
deepening history or hydrating partial-clone objects rather than to review.
Trigger catalog schema 0.2 -> 0.3; verifier schema 0.6 -> 0.7 (v0.6 stays a
frozen, readable reference). `contract_version` and every other schema
counter are unchanged.
Closes #308
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(verify): address review — split unrelated histories, keep partial evidence
Four contract inconsistencies from the engineering review on #340.
**Unrelated histories were routed into an unrecoverable fetch loop.** Git
reports two different failures identically as "no merge base": a shallow
checkout that truncated a merge base which does exist, and two roots that
share no ancestor at all. Only the first is repairable by fetching, and the
second was being sent to `fetch_base` forever. `git rev-parse
--is-shallow-repository` discriminates them exactly, so the reason splits:
`merge_base_missing` (shallow — deepen, `fetch_repairable: true`) and the new
`unrelated_histories` (no fetch can create an ancestor — confirm the base ref,
routed to a human). A failed shallow probe is neither and stays `git_failed`
rather than asserting a cause it cannot establish. A regression test deepens
a real shallow clone and proves the diff then reads clean, so the remediation
is the one that actually works.
**Partial worktree evidence was collected and then dropped.** When
`working_tree_context` raised, the handler recorded the classified reason but
never merged the paths the failed collector had already read, so `base_notes`
said the paths were collected while `changed_files` was empty and a changed
`tools/new_mcp.json` lost its path-rule match. The paths and any text now
merge into the accumulators before the artifact is built. The untracked-path
inventory also moves ahead of the body read: it is cheap metadata independent
of the body, and a brand-new capability file appears in no `git diff` at all,
so collecting it afterwards meant a body failure dropped it entirely.
**Preview could contradict its own published verdict.** Partial evidence can
still carry a sound run verdict — a matched path rule needs no diff body — and
the evaluator publishes it deliberately. The failure branch nevertheless
printed "no relevance verdict was reached" as both `headline` and
`control.reason` next to `should_run: true`. It now branches on the trigger
result and says relevance is established while the full diff still needs
recovering.
**The agent-facing surfaces omitted that exception.** SKILL.md (all three
copies), the relevance prompt (all four), and AGENTS.md said every incomplete
diff implies `not_evaluated`/`null`, which would have consumers override a
valid run verdict. They now state the monotonicity rule that STABILITY.md
already documented, and carry the new reason token.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(verify): make diff_status enforced, and consistent with its own routing
Four contract inconsistencies from the second engineering review on #340.
**`diff_status` was documented as always emitted and enforced nowhere.** Both
the model and `_build_verifier` defaulted it to `None`, so a current
`verifier_schema_version: "0.7"` payload could drop the input-health block
entirely and still validate — indistinguishable from one that read its diff
cleanly, which is the single claim this field exists to prevent. It is now a
required, non-null field on the model and in the generated v0.7 schema, and
`_build_verifier` requires its caller to supply one. The nullable
representation is confined to the compatibility path: a pre-v0.7 artifact
normalizes to `VerifierDiffStatus.unknown()`, a new fourth completeness value
that names exactly what such an artifact recorded — nothing. Like every value
other than `complete` it withholds permission to read a negative trigger
verdict. `fetch_repairable` is now also structurally checked against the
reason, so no artifact can advertise a fetch as the repair for a failure a
fetch cannot touch.
**The repair action was derived incrementally, not from the reported status.**
When the committed diff failed fetch-repairably and the worktree then failed
deterministically, `_least_complete` selected the worktree failure for
`diff_status` while an `or` preserved the earlier `fetch_base` action — an
artifact carrying `fetch_repairable: false` beside an authorized fetch, which
is the loop the classification exists to prevent. Failures now accumulate with
their repair targets and `_worst_diff_failure` selects one, breaking ties
toward the failure a fetch cannot repair; the action and the headline are both
derived from that single selection.
**The failed-verification headline contradicted the control route.** The
branch supplied no headline override, so `_verifier_headline` mapped every
failed/unknown scan to "human review required" while control said
`agent_action_required` with `next_action: fetch_base` and
`human_review.required: false`. Both now come from the same classified
failure.
**Not every run verdict is attributable to diff paths.** In an adopted
repository `TRIGGER-EXISTING-MANIFEST-PRESENT` force-runs on the manifest
alone, with `changed_files: []`, and the preview headline nevertheless claimed
the paths it read showed a capability surface. It now separates rules matched
on the change set from the force-run rule, and the skill mirrors, prompts,
AGENTS.md, STABILITY.md, and the agent contract all point readers at
`matched_rules` before attributing the verdict.1 parent 23c4931 commit eae59ed
53 files changed
Lines changed: 5029 additions & 213 deletions
File tree
- .well-known
- adoption-kits/claude-code-skill
- prompts
- docs
- plugins/claude-code/skills/agents-shipgate
- prompts
- prompts
- skills/agents-shipgate
- prompts
- src/agents_shipgate
- cli
- discovery/agent_instructions/renderers
- verify
- schemas
- tests
- golden/codex_boundary_result
- integration/github_action
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
180 | 180 | | |
181 | 181 | | |
182 | 182 | | |
183 | | - | |
| 183 | + | |
184 | 184 | | |
185 | 185 | | |
186 | 186 | | |
| |||
279 | 279 | | |
280 | 280 | | |
281 | 281 | | |
282 | | - | |
| 282 | + | |
283 | 283 | | |
284 | 284 | | |
285 | 285 | | |
| |||
441 | 441 | | |
442 | 442 | | |
443 | 443 | | |
444 | | - | |
| 444 | + | |
445 | 445 | | |
446 | 446 | | |
447 | 447 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
315 | 315 | | |
316 | 316 | | |
317 | 317 | | |
318 | | - | |
| 318 | + | |
319 | 319 | | |
320 | 320 | | |
321 | 321 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
5 | 43 | | |
6 | 44 | | |
7 | 45 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
652 | 652 | | |
653 | 653 | | |
654 | 654 | | |
655 | | - | |
| 655 | + | |
656 | 656 | | |
657 | 657 | | |
658 | 658 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
16 | 52 | | |
17 | 53 | | |
18 | 54 | | |
| |||
595 | 631 | | |
596 | 632 | | |
597 | 633 | | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
598 | 637 | | |
599 | 638 | | |
600 | 639 | | |
| |||
1300 | 1339 | | |
1301 | 1340 | | |
1302 | 1341 | | |
1303 | | - | |
1304 | | - | |
| 1342 | + | |
| 1343 | + | |
1305 | 1344 | | |
1306 | 1345 | | |
1307 | | - | |
| 1346 | + | |
| 1347 | + | |
1308 | 1348 | | |
1309 | 1349 | | |
1310 | 1350 | | |
1311 | 1351 | | |
1312 | 1352 | | |
1313 | 1353 | | |
| 1354 | + | |
| 1355 | + | |
| 1356 | + | |
| 1357 | + | |
| 1358 | + | |
| 1359 | + | |
| 1360 | + | |
| 1361 | + | |
| 1362 | + | |
| 1363 | + | |
| 1364 | + | |
| 1365 | + | |
| 1366 | + | |
| 1367 | + | |
| 1368 | + | |
| 1369 | + | |
| 1370 | + | |
| 1371 | + | |
| 1372 | + | |
| 1373 | + | |
1314 | 1374 | | |
1315 | 1375 | | |
1316 | 1376 | | |
| |||
1357 | 1417 | | |
1358 | 1418 | | |
1359 | 1419 | | |
1360 | | - | |
| 1420 | + | |
| 1421 | + | |
| 1422 | + | |
| 1423 | + | |
| 1424 | + | |
| 1425 | + | |
| 1426 | + | |
| 1427 | + | |
| 1428 | + | |
| 1429 | + | |
| 1430 | + | |
1361 | 1431 | | |
1362 | 1432 | | |
1363 | 1433 | | |
| |||
Lines changed: 4 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
39 | | - | |
| 39 | + | |
| 40 | + | |
40 | 41 | | |
41 | 42 | | |
42 | 43 | | |
| |||
83 | 84 | | |
84 | 85 | | |
85 | 86 | | |
86 | | - | |
| 87 | + | |
| 88 | + | |
87 | 89 | | |
88 | 90 | | |
89 | 91 | | |
| |||
0 commit comments