Repository navigation
bug(verify-preview): distinguish unreadable diff from trigger no_match #308
Description
Summary
When a committed-ref diff cannot be read, agents-shipgate verify --preview still evaluates the Trigger against empty inputs and reports skip_reason: "no_match".
"No rule matched" is not a valid explanation when the verifier never obtained a readable diff.
Reproduction
A shallow/blobless local PR workflow exposed two input failures:
- the base and head had no reachable merge base;
- after history was deepened, the partial clone still lacked source blobs needed for the textual diff.
In both shapes, Preview reported fields equivalent to:
{
"changed_files": [],
"diff_text_available": false,
"trigger": {
"should_run": false,
"skip": true,
"skip_reason": "no_match",
"rationale": "No rules matched; nothing in this PR signals a tool-surface change."
}
}The underlying Git errors were respectively:
fatal: <base>...<head>: no merge base
fatal: could not fetch <blob-sha> from promisor remote
The latter occurs because verification intentionally sets GIT_NO_LAZY_FETCH=1, which is the right static/no-implicit-network boundary.
Why this matters
The top-level control result can remain fail-closed (merge_verdict: "unknown", can_merge_without_human: false), but the Trigger explanation is misleading. A user or automation may incorrectly interpret no_match as evidence that the PR is unrelated to agent capabilities.
Expected result
When diff acquisition fails or is incomplete:
- do not emit
skip_reason: "no_match"; - mark trigger evaluation as not evaluated/incomplete;
- expose a machine-readable diff-input reason, for example:
refs_missingmerge_base_missingobjects_missinggit_failed;
- preserve the relevant Git failure safely in diagnostics;
- return the precise next action, such as fetching/deepening history or hydrating required partial-clone blobs.
Illustrative shape:
{
"diff_status": "objects_missing",
"trigger": {
"evaluation_status": "not_evaluated",
"should_run": null,
"skip_reason": null
},
"merge_verdict": "unknown",
"can_merge_without_human": false
}Regression coverage
Please add committed-ref Preview tests for:
- no merge base in a shallow clone;
- promisor/partial-clone blob unavailable with lazy fetching disabled;
- a real agent-related diff that must not be classified as
no_matchmerely because its input is unreadable.
Activity
pengfei-threemoonslab commentedon Aug 5, 2026
Re-verified at
527a0349(v0.16.0b7). Confirmed, and the blast radius is larger than filed in two ways.1. On an unconfigured workspace, the diff failure is never reported at all.
Both
diff_unavailablebranches inrun_previeware gated onmanifest_present(cli/verify/orchestrator.py:3149,:3156):With no
shipgate.yaml, an unreadable diff falls past both,trigger.should_runisFalse(because the trigger just evaluated empty inputs), and control reaches the finalelse:The Git failure is in
base_notesbut nothing in the headline, the verdict, or the next action mentions it.merge_verdict: "unknown"andcan_merge_without_human: falsestill hold, so the control result is fail-closed — but the route is wrong, and it is wrong precisely in the cold-start case that this class of failure comes from. Shallow and blobless clones of unadopted repositories are the normal shape for first contact.2. Draft PR #305 replicates the defect into two new code paths.
#305 ("Fail closed when verify cannot resolve a comparison base") adds
fetch_required/selection_requiredstates torun_verify, and at that branch does:which emits
skip_reason: "no_match"with the rationale "nothing in this PR signals a tool-surface change" inside a verifier artifact that has just declared it could not bind a trustworthy comparison base. The PR touchesrun_verifyonly;run_previewis unchanged, so the originally filed path survives as well.The two are otherwise complementary — #305 covers repro case 1 (no merge base) for
verifyand usefully distinguishes shallow from unrelated histories, which this issue does not ask for. Suggest sequencing this issue first, or folding the trigger-side fix into #305, so the new fail-closed states do not ship with a contradictory trigger block.3. Root cause is in the trigger evaluator, not only its callers.
triggers.py:452-458has no representation for "inputs were not obtained" — theelsearm is the only landing place, and it hardcodesno_match. Every caller that evaluates against empty inputs will reproduce this. Fixing it at the three call sites will not hold; the evaluator needs theevaluation_statusthe issue proposes, and adding a state to the catalog's reported output is aschema_versionquestion worth settling as part of the fix.pengfei-threemoonslab commentedon Aug 6, 2026
Additional reproduction shape from a real supported-framework PR:
While evaluating google/adk-samples#1745 with Agents Shipgate
0.16.0b7/ contract19,verify --preview --base base-pr-1745 --head HEAD --jsonreturned:{ "changed_files": [], "diff_text_available": false, "trigger": { "skip": true, "skip_reason": "no_match", "rationale": "No rules matched; nothing in this PR signals a tool-surface change." }, "base_notes": [ "Preview diff unavailable: Git diff exceeded static output bounds or could not be read." ] }This PR adds a Google ADK
LlmAgentgraph and multipleFunctionToolbindings, but also includes large PNG/GIF demo assets. The current implementation collects--name-statusand the full diff body separately, then raises one generic error and discards both results if either bounded read returnsNone. The output therefore cannot distinguish a metadata failure from a body-limit failure, and a body-heavy PR can lose otherwise usable changed-path evidence.Suggested extension to this issue's acceptance criteria:
--name-statuspaths when only the diff body exceeds_DIFF_BODY_LIMIT.diff_statusthat distinguishes metadata failure, body truncation, missing objects, and other Git errors.no_matchwhen diff acquisition is incomplete.agent.pyis still recognized and routed even though unrelated demo assets make the aggregate PR large.