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()