diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 8f5dec4fe..b8316c506 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -721,8 +721,20 @@ jobs: // issue at all is separately owned by `Canonical issue and // evidence`, which states a requirement an author can actually // meet. + // + // "Exists to verify against" means the issue declares the + // contract, not merely that an issue is linked. Testing only for + // presence left the same defect one level down: a `claude/...` + // branch closing an ordinary issue armed the gate, which then + // demanded `policy.agent_login` and `policy.run_id` that no + // intent snapshot had ever written, so the verdict was + // permanently `invalid_payload` (#1401). `declaresAgentContract` + // is the same predicate `issueDispatch` already relies on, so + // both arms of this disjunction now require the thing the gate + // goes on to measure. return login !== 'dependabot[bot]' && - (issueDispatch || (pullProvenance && Boolean(selectedIssue))); + (issueDispatch || + (pullProvenance && declaresAgentContract(selectedIssue))); } function intentContractErrors(issue, comments, pullCreatedAt) { const errors = []; @@ -2047,8 +2059,20 @@ jobs: // issue at all is separately owned by `Canonical issue and // evidence`, which states a requirement an author can actually // meet. + // + // "Exists to verify against" means the issue declares the + // contract, not merely that an issue is linked. Testing only for + // presence left the same defect one level down: a `claude/...` + // branch closing an ordinary issue armed the gate, which then + // demanded `policy.agent_login` and `policy.run_id` that no + // intent snapshot had ever written, so the verdict was + // permanently `invalid_payload` (#1401). `declaresAgentContract` + // is the same predicate `issueDispatch` already relies on, so + // both arms of this disjunction now require the thing the gate + // goes on to measure. return login !== 'dependabot[bot]' && - (issueDispatch || (pullProvenance && Boolean(selectedIssue))); + (issueDispatch || + (pullProvenance && declaresAgentContract(selectedIssue))); } const prNumber = Number(process.env.INPUT_PR_NUMBER || 0); diff --git a/tests/unit/test_agent_completion_gate.py b/tests/unit/test_agent_completion_gate.py index c58715293..a34307155 100644 --- a/tests/unit/test_agent_completion_gate.py +++ b/tests/unit/test_agent_completion_gate.py @@ -2894,22 +2894,43 @@ def test_scheduled_scanner_detects_frozen_intent_changes(self): ['issue label without contract', base, issue(1, ['mcp/agent']), false], ['issue label with contract', base, issue(1, ['mcp/agent'], CONTRACT), true], // Pull-side provenance identifies the producer; it does not supply a - // contract to score against. With no linked issue there is no intent - // snapshot, so `policy.agent_login`, `policy.run_id` and `issue.number` - // can never be populated and the verdict is permanently `invalid_payload`. - // Each of these arms the gate only once an issue exists to verify against. + // contract to score against. That contract lives only in the intent + // snapshot `snapshot-agent-task-intent` writes for a dispatched issue, so + // without one `policy.agent_login`, `policy.run_id` and `issue.number` can + // never be populated and the verdict is permanently `invalid_payload`. + // + // So each of these arms the gate only once a linked issue *declares the + // contract*. A merely-present issue is not enough: an ordinary issue has no + // snapshot either, and arming on it reproduced the same unsatisfiable gate + // one level down (#1401). Both directions are pinned for every provenance + // signal — plain issue must not arm, contract issue must. ['PR label, no issue', {...base, labels: [{name: 'agent-task'}]}, null, false], - ['PR label + issue', {...base, labels: [{name: 'agent-task'}]}, PLAIN, true], + ['PR label + plain issue', + {...base, labels: [{name: 'agent-task'}]}, PLAIN, false], + ['PR label + contract issue', + {...base, labels: [{name: 'agent-task'}]}, issue(1, [], CONTRACT), true], ['branch, no issue', {...base, head: {ref: 'codex/fix'}}, null, false], - ['branch + issue', {...base, head: {ref: 'codex/fix'}}, PLAIN, true], + ['branch + plain issue', {...base, head: {ref: 'codex/fix'}}, PLAIN, false], + ['branch + contract issue', + {...base, head: {ref: 'codex/fix'}}, issue(1, [], CONTRACT), true], ['manifest, no issue', {...base, body: ''}, null, false], - ['manifest + issue', - {...base, body: ''}, PLAIN, true], + ['manifest + plain issue', + {...base, body: ''}, PLAIN, false], + ['manifest + contract issue', + {...base, body: ''}, + issue(1, [], CONTRACT), true], ['known agent, no issue', {...base, user: {login: 'google-labs-jules[bot]'}}, null, false], - ['known agent + issue', - {...base, user: {login: 'google-labs-jules[bot]'}}, PLAIN, true], + ['known agent + plain issue', + {...base, user: {login: 'google-labs-jules[bot]'}}, PLAIN, false], + ['known agent + contract issue', + {...base, user: {login: 'google-labs-jules[bot]'}}, + issue(1, [], CONTRACT), true], + // The exact shape that was red on #1400: an agent-prefixed branch closing an + // ordinary issue. This must resolve `not_applicable`, not `invalid_payload`. + ['claude branch + ordinary issue', + {...base, head: {ref: 'claude/clever-heisenberg-8k227t'}}, PLAIN, false], ['dependabot excluded', { ...base, user: {login: 'dependabot[bot]'}, @@ -3464,25 +3485,34 @@ def test_agent_applicability_requires_provenance_or_declared_contract(self): ['unlabelled issue with contract', human, { number: 7, labels: [{name: 'bug'}], body: CONTRACT }, false], - // Pull request provenance applies against a linked issue -- an agent - // producing work against a contract-less issue is still applicable, and - // therefore still blocked. - ['known agent author', { + // Pull request provenance applies only against an issue that declares the + // contract. A known agent closing a contract-less issue has nothing to be + // measured against -- the snapshot job never wrote one -- so arming here + // produced a permanent invalid_payload rather than an enforcement (#1401). + ['known agent author, contract-less issue', { user: {login: 'google-labs-jules[bot]'}, head: {ref: 'feature/thing'}, labels: [], body: '' - }, {number: 7, labels: [{name: 'agent-task'}], body: '## Summary\n'}, true], - // ...but with no linked issue there is no intent snapshot and no declared - // run id or login, so the required payload fields are unsatisfiable and the - // gate would block permanently rather than ever reaching a verdict. These - // are `not_applicable`, not violations. + }, {number: 7, labels: [{name: 'agent-task'}], body: '## Summary\n'}, false], + ['known agent author, declared contract', { + user: {login: 'google-labs-jules[bot]'}, + head: {ref: 'feature/thing'}, labels: [], body: '' + }, {number: 7, labels: [{name: 'agent-task'}], body: CONTRACT}, true], + // Likewise with no linked issue at all: no intent snapshot, no declared run + // id or login, so the required payload fields are unsatisfiable and the gate + // would block permanently rather than ever reaching a verdict. These are + // `not_applicable`, not violations. ['agent branch prefix, no issue', { user: {login: 'groupthinking'}, head: {ref: 'jules/thing'}, labels: [], body: '' }, null, false], - ['agent branch prefix with issue', { + ['agent branch prefix, contract-less issue', { + user: {login: 'groupthinking'}, + head: {ref: 'jules/thing'}, labels: [], body: '' + }, {number: 7, labels: [], body: ''}, false], + ['agent branch prefix, declared contract', { user: {login: 'groupthinking'}, head: {ref: 'jules/thing'}, labels: [], body: '' - }, {number: 7, labels: [], body: ''}, true], + }, {number: 7, labels: [], body: CONTRACT}, true], ['agent label on the pull request, no issue', { user: {login: 'groupthinking'}, head: {ref: 'feature/thing'},