From c8106c70b34317f632295da030f99598ff84c2d1 Mon Sep 17 00:00:00 2001 From: Hayden <154503486+groupthinking@users.noreply.github.com> Date: Sat, 1 Aug 2026 14:39:10 -0500 Subject: [PATCH] fix: scope agent gate applicability to real dispatch evidence `agentTaskApplicable` unioned the pull request's labels with the linked issue's labels, so a linked issue carrying `agent-task` made any pull request an agent completion. Label automation applies `agent-task` as a topic tag to issues that were never created from the agent task template and therefore never declared an Agent Run ID or Agent Login. Those pull requests were then measured against a contract that did not exist, producing `blocked` / `invalid_payload` with `policy.agent_login` and `policy.run_id` invalid -- a state no author could reach, because the missing fields live on an issue that was never dispatched. Applicability is now a function of dispatch evidence: - Pull request provenance (known agent author, agent branch prefix, an agent label on the pull request, or a lock manifest in its body) is sufficient on its own, so fail-closed behaviour is preserved for real agent work regardless of how the issue is labelled. - The linked issue's agent label counts only when the issue body actually declares the Agent Run ID and Agent Login the gate goes on to require, ignoring the `_No response_` placeholder left by unfilled issue forms. The contract check is nested inside `agentTaskApplicable` so the function stays self-contained for the workflow's `node -e` extraction harness, and both copies remain byte-identical. A mislabelled issue is now surfaced with `core.notice` as `mislabelled_agent_task`; it cannot be reported as a collection error because the gate short-circuits to `not_applicable` before reading them. Two pre-existing assertions encoded the old behaviour and are updated to the corrected contract while preserving their original intent: the row proving a linked issue's labels are consulted now covers both the contract-less and contract-bearing cases, and the textual-link fallback check keeps its contrast by declaring a contract on the selected issue. Fixes #1130 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/pr-checks.yml | 178 ++++++++++++++++++----- tests/unit/test_agent_completion_gate.py | 173 +++++++++++++++++++++- 2 files changed, 307 insertions(+), 44 deletions(-) diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 4131c473b..725fc8074 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -614,10 +614,51 @@ jobs: }; } function agentTaskApplicable(pull, selectedIssue) { + function declaresAgentContract(issue) { + function normaliseHeading(value) { + return String(value || '').toLowerCase() + .replace(/[^a-z0-9 ]/g, '').trim(); + } + function section(body, headings) { + const wanted = new Set(headings.map(normaliseHeading)); + const lines = String(body || '').split(/\r?\n/); + const output = []; + let collecting = false; + for (const line of lines) { + const heading = line.match(/^#{2,6}\s+(.+?)\s*$/); + if (heading) { + if (collecting) { + break; + } + collecting = wanted.has(normaliseHeading(heading[1])); + continue; + } + if (collecting) { + output.push(line); + } + } + return output.join('\n').trim(); + } + function declared(headings) { + const value = section( + String((issue && issue.body) || ''), headings + ).replace(/^\x60|\x60$/g, '').trim(); + return Boolean(value) && value !== '_No response_'; + } + return declared(['agent run id', 'run id']) && + declared(['agent login']); + } function normaliseLabel(value) { return String(value || '').toLowerCase() .replace(/[^a-z0-9 ]/g, '').trim(); } + function carriesAgentLabel(source) { + return (source || []).map(label => + typeof label === 'string' ? label : label.name + ).map(normaliseLabel).some(label => + ['agent', 'agenttask', 'mcpagent'].includes(label) + ); + } const login = String( pull && pull.user && pull.user.login || '' ); @@ -628,21 +669,6 @@ jobs: 'openai-codex[bot]', 'chatgpt-codex-connector[bot]' ]); - const labels = [ - ...((pull && pull.labels) || []).map(label => - typeof label === 'string' ? label : label.name - ), - ...(selectedIssue && selectedIssue.labels - ? Array.isArray(selectedIssue.labels) - ? selectedIssue.labels - : selectedIssue.labels.nodes || [] - : []).map(label => - typeof label === 'string' ? label : label.name - ) - ].map(normaliseLabel); - const agentLabel = labels.some(label => - ['agent', 'agenttask', 'mcpagent'].includes(label) - ); const agentBranch = /^(?:agent|claude|codex|copilot|jules)[/-]/i.test( String(pull && pull.head && pull.head.ref || '') @@ -651,10 +677,28 @@ jobs: //i.test( String(pull && pull.body || '') ); - return login !== 'dependabot[bot]' && ( - knownAgents.has(login) || agentBranch || agentLabel || - manifestPresent - ); + // Provenance asserted by the pull request itself. Each of these is + // a claim made by the producing side that this is agent work. + const pullProvenance = knownAgents.has(login) || agentBranch || + manifestPresent || + carriesAgentLabel((pull && pull.labels) || []); + // Issue-side dispatch. `agent-task` is also applied as a topic tag + // by label automation to issues that never declared a contract, so + // the bare label is not sufficient evidence of a dispatch: it only + // counts when the issue actually declares the run id and login the + // gate goes on to require. Treating the bare label as sufficient + // made the gate permanently unsatisfiable for human pull requests + // closing such issues, because the contract they were measured + // against had never been written (#1130). + const issueLabelSource = selectedIssue && selectedIssue.labels + ? Array.isArray(selectedIssue.labels) + ? selectedIssue.labels + : selectedIssue.labels.nodes || [] + : []; + const issueDispatch = carriesAgentLabel(issueLabelSource) && + declaresAgentContract(selectedIssue); + return login !== 'dependabot[bot]' && + (pullProvenance || issueDispatch); } function intentContractErrors(issue, comments, pullCreatedAt) { const errors = []; @@ -1872,10 +1916,51 @@ jobs: Number.isFinite(pullTime) && snapshotTime < pullTime; } function agentTaskApplicable(pull, selectedIssue) { + function declaresAgentContract(issue) { + function normaliseHeading(value) { + return String(value || '').toLowerCase() + .replace(/[^a-z0-9 ]/g, '').trim(); + } + function section(body, headings) { + const wanted = new Set(headings.map(normaliseHeading)); + const lines = String(body || '').split(/\r?\n/); + const output = []; + let collecting = false; + for (const line of lines) { + const heading = line.match(/^#{2,6}\s+(.+?)\s*$/); + if (heading) { + if (collecting) { + break; + } + collecting = wanted.has(normaliseHeading(heading[1])); + continue; + } + if (collecting) { + output.push(line); + } + } + return output.join('\n').trim(); + } + function declared(headings) { + const value = section( + String((issue && issue.body) || ''), headings + ).replace(/^\x60|\x60$/g, '').trim(); + return Boolean(value) && value !== '_No response_'; + } + return declared(['agent run id', 'run id']) && + declared(['agent login']); + } function normaliseLabel(value) { return String(value || '').toLowerCase() .replace(/[^a-z0-9 ]/g, '').trim(); } + function carriesAgentLabel(source) { + return (source || []).map(label => + typeof label === 'string' ? label : label.name + ).map(normaliseLabel).some(label => + ['agent', 'agenttask', 'mcpagent'].includes(label) + ); + } const login = String( pull && pull.user && pull.user.login || '' ); @@ -1886,21 +1971,6 @@ jobs: 'openai-codex[bot]', 'chatgpt-codex-connector[bot]' ]); - const labels = [ - ...((pull && pull.labels) || []).map(label => - typeof label === 'string' ? label : label.name - ), - ...(selectedIssue && selectedIssue.labels - ? Array.isArray(selectedIssue.labels) - ? selectedIssue.labels - : selectedIssue.labels.nodes || [] - : []).map(label => - typeof label === 'string' ? label : label.name - ) - ].map(normaliseLabel); - const agentLabel = labels.some(label => - ['agent', 'agenttask', 'mcpagent'].includes(label) - ); const agentBranch = /^(?:agent|claude|codex|copilot|jules)[/-]/i.test( String(pull && pull.head && pull.head.ref || '') @@ -1909,10 +1979,28 @@ jobs: //i.test( String(pull && pull.body || '') ); - return login !== 'dependabot[bot]' && ( - knownAgents.has(login) || agentBranch || agentLabel || - manifestPresent - ); + // Provenance asserted by the pull request itself. Each of these is + // a claim made by the producing side that this is agent work. + const pullProvenance = knownAgents.has(login) || agentBranch || + manifestPresent || + carriesAgentLabel((pull && pull.labels) || []); + // Issue-side dispatch. `agent-task` is also applied as a topic tag + // by label automation to issues that never declared a contract, so + // the bare label is not sufficient evidence of a dispatch: it only + // counts when the issue actually declares the run id and login the + // gate goes on to require. Treating the bare label as sufficient + // made the gate permanently unsatisfiable for human pull requests + // closing such issues, because the contract they were measured + // against had never been written (#1130). + const issueLabelSource = selectedIssue && selectedIssue.labels + ? Array.isArray(selectedIssue.labels) + ? selectedIssue.labels + : selectedIssue.labels.nodes || [] + : []; + const issueDispatch = carriesAgentLabel(issueLabelSource) && + declaresAgentContract(selectedIssue); + return login !== 'dependabot[bot]' && + (pullProvenance || issueDispatch); } const prNumber = Number(process.env.INPUT_PR_NUMBER || 0); @@ -2077,6 +2165,20 @@ jobs: normaliseHeading(typeof label === 'string' ? label : label.name) )); const applicable = agentTaskApplicable(pr, issue); + // A linked issue carrying `agent-task` that never declared a run id + // and login was not dispatched to an agent -- the label is topic + // noise. Report it so the mislabel is visible and correctable, + // instead of silently measuring the pull request against a contract + // that does not exist (#1130). + if (!applicable && issue && + ['agenttask', 'mcpagent'].some(label => issueLabels.has(label))) { + core.notice( + 'mislabelled_agent_task: issue #' + issue.number + ' carries an ' + + 'agent task label but declares no Agent Run ID / Agent Login, ' + + 'and this pull request shows no agent provenance. Treating it ' + + 'as human work. Remove the label if it was applied in error.' + ); + } if (applicable && !issue) { collectionErrors.push('missing_linked_issue'); } diff --git a/tests/unit/test_agent_completion_gate.py b/tests/unit/test_agent_completion_gate.py index 301263cd1..13d0faf02 100644 --- a/tests/unit/test_agent_completion_gate.py +++ b/tests/unit/test_agent_completion_gate.py @@ -2754,9 +2754,14 @@ def test_scheduled_scanner_detects_frozen_intent_changes(self): self.assertEqual(len(applicable_functions), 2) self.assertEqual(applicable_functions[0], applicable_functions[1]) applicable_assertions = r""" -const issue = (number, labels) => ({ +const CONTRACT = [ + '### Agent Login', '', '`google-labs-jules[bot]`', '', + '### Agent Run ID', '', '`run-42`' +].join('\n'); +const issue = (number, labels, body) => ({ number, - labels: {nodes: labels.map(name => ({name}))} + labels: {nodes: labels.map(name => ({name}))}, + body: body || '' }); const base = { user: {login: 'maintainer'}, @@ -2767,7 +2772,11 @@ def test_scheduled_scanner_detects_frozen_intent_changes(self): const rows = [ ['human', base, null, false], ['PR label', {...base, labels: [{name: 'agent-task'}]}, null, true], - ['issue label', base, issue(1, ['mcp/agent']), true], + // A bare agent label on the linked issue is a topic tag applied by label + // automation, not a dispatch. It only asserts agent work when the issue + // actually declares the contract the gate goes on to require (#1130). + ['issue label without contract', base, issue(1, ['mcp/agent']), false], + ['issue label with contract', base, issue(1, ['mcp/agent'], CONTRACT), true], ['branch', {...base, head: {ref: 'codex/fix'}}, null, true], ['manifest', {...base, body: ''}, null, true], ['known agent', {...base, user: {login: 'google-labs-jules[bot]'}}, null, true], @@ -2775,7 +2784,7 @@ def test_scheduled_scanner_detects_frozen_intent_changes(self): ...base, user: {login: 'dependabot[bot]'}, labels: [{name: 'agent-task'}] - }, issue(1, ['agent-task']), false] + }, issue(1, ['agent-task'], CONTRACT), false] ]; for (const [name, pull, selected, expected] of rows) { const actual = agentTaskApplicable(pull, selected); @@ -2849,8 +2858,12 @@ def test_scheduled_scanner_detects_frozen_intent_changes(self): self.assertEqual(completed.returncode, 0, completed.stderr) combined_assertions = r""" -const first = {number: 1, labels: {nodes: []}}; -const second = {number: 2, labels: {nodes: [{name: 'agent-task'}]}}; +const first = {number: 1, labels: {nodes: []}, body: ''}; +const second = { + number: 2, + labels: {nodes: [{name: 'agent-task'}]}, + body: '### Agent Login\n\n`google-labs-jules[bot]`\n\n### Agent Run ID\n\n`run-42`' +}; const multi = { user: {login: 'maintainer'}, head: {ref: 'feature/ordinary'}, @@ -3242,6 +3255,154 @@ def test_adapter_recognizes_agent_labels_scripts_and_blocking_threads(self): self.assertIn("!explicitlyNonBehavioral", workflow) self.assertIn("VADE-RECOMMENDATION", workflow) + def test_agent_applicability_requires_provenance_or_declared_contract(self): + """A bare `agent-task` label on a linked issue is not a dispatch. + + Label automation applies `agent-task` as a topic tag to issues that were + never created from the agent task template. Before this guard the gate + unioned pull request labels with linked issue labels, so any pull request + closing such an issue was judged an agent completion and then measured + against a contract the issue had never declared -- producing a permanent + ``blocked``/``invalid_payload`` verdict that no author could satisfy. + """ + + workflow = self._workflow() + applicable = _javascript_functions( + workflow, + "function agentTaskApplicable(", + ) + self.assertEqual(len(applicable), 2) + + assertions = r""" +const CONTRACT = [ + '### Agent Login', + '', + '`google-labs-jules[bot]`', + '', + '### Agent Run ID', + '', + '`run-42`' +].join('\n'); +const human = { + user: {login: 'groupthinking'}, + head: {ref: 'feature/thing'}, + labels: [], + body: 'Fixes #7' +}; +const rows = [ + // The regression this guard exists for: a human pull request closing an + // issue that automation mislabelled `agent-task` without a contract. + ['mislabelled linked issue', { + user: {login: 'groupthinking'}, + head: {ref: 'fix/agentic-workflow-noop-terminal-state-1091'}, + labels: [{name: 'documentation'}, {name: 'ci/cd'}], + body: 'Fixes #1091' + }, { + number: 1091, + labels: [{name: 'agent-task'}, {name: 'bug'}], + body: '## Summary\nSomething broke.\n' + }, false], + // A genuine issue-side dispatch is still gated. + ['declared contract', human, { + number: 7, labels: [{name: 'agent-task'}], body: CONTRACT + }, true], + ['contract via graphql label nodes', human, { + number: 7, labels: {nodes: [{name: 'agent-task'}]}, body: CONTRACT + }, true], + // Unfilled issue form fields render as the placeholder, not a contract. + ['no response placeholder', human, { + number: 7, + labels: [{name: 'agent-task'}], + body: '### Agent Login\n\n_No response_\n\n### Agent Run ID\n\n_No response_\n' + }, false], + ['half declared contract', human, { + number: 7, + labels: [{name: 'agent-task'}], + body: '### Agent Login\n\n`jules`\n' + }, false], + ['unlabelled issue with contract', human, { + number: 7, labels: [{name: 'bug'}], body: CONTRACT + }, false], + // Pull request provenance stands alone: an agent producing work against a + // contract-less issue is still applicable, and therefore still blocked. + ['known agent author', { + user: {login: 'google-labs-jules[bot]'}, + head: {ref: 'feature/thing'}, labels: [], body: '' + }, {number: 7, labels: [{name: 'agent-task'}], body: '## Summary\n'}, true], + ['agent branch prefix', { + user: {login: 'groupthinking'}, + head: {ref: 'jules/thing'}, labels: [], body: '' + }, null, true], + ['agent label on the pull request', { + user: {login: 'groupthinking'}, + head: {ref: 'feature/thing'}, + labels: [{name: 'agent-task'}], body: '' + }, null, true], + ['lock manifest in the pull request body', { + user: {login: 'groupthinking'}, + head: {ref: 'feature/thing'}, + labels: [], + body: '' + }, null, true], + // Dependabot is excluded regardless of every other signal. + ['dependabot', { + user: {login: 'dependabot[bot]'}, + head: {ref: 'jules/bump'}, + labels: [{name: 'agent-task'}], body: '' + }, {number: 7, labels: [{name: 'agent-task'}], body: CONTRACT}, false], + ['plain human pull request', human, { + number: 7, labels: [{name: 'bug'}], body: '' + }, false], + ['no linked issue', human, null, false] +]; +for (const [name, pull, issue, expected] of rows) { + const actual = agentTaskApplicable(pull, issue); + if (actual !== expected) { + throw new Error(`${name}: ${actual} !== ${expected}`); + } +} +""" + completed = subprocess.run( + ["node", "-e", applicable[0] + assertions], + check=False, + capture_output=True, + text=True, + ) + self.assertEqual(completed.returncode, 0, completed.stderr) + + def test_agent_applicability_copies_stay_identical(self): + """The scheduled sweep and the per-pull-request collector must agree. + + Both jobs publish to the same commit status context, so divergent + applicability logic would let one job block what the other skips. The + contract check is nested inside ``agentTaskApplicable`` so the function + stays self-contained for the ``node -e`` extraction harness above. + """ + + workflow = self._workflow() + copies = _javascript_functions(workflow, "function agentTaskApplicable(") + self.assertEqual(len(copies), 2) + self.assertEqual(copies[0], copies[1]) + self.assertIn("function declaresAgentContract(", copies[0]) + + def test_mislabelled_agent_task_is_reported_rather_than_blocked(self): + """The mislabel is surfaced as an annotation, never as a gate reason. + + ``agent_completion_gate.evaluate`` short-circuits to ``not_applicable`` + before it reads ``collection_errors``, so a diagnostic pushed there would + be silently discarded. It is emitted with ``core.notice`` instead. + """ + + workflow = self._workflow() + + self.assertIn("mislabelled_agent_task", workflow) + self.assertNotIn( + "collectionErrors.push('mislabelled_agent_task')", + workflow, + ) + notice = workflow[workflow.index("mislabelled_agent_task") - 400:] + self.assertIn("core.notice(", notice[:600]) + def test_workflow_does_not_approve_or_merge(self): workflow = self._workflow()