diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 4131c473b..0a1a84c98 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -618,6 +618,43 @@ jobs: return String(value || '').toLowerCase() .replace(/[^a-z0-9 ]/g, '').trim(); } + function carriesLabel(source, accepted) { + return (source || []).map(label => + typeof label === 'string' ? label : label.name + ).map(normaliseLabel).some(label => + accepted.includes(label) + ); + } + function declaresAgentContract(issue) { + function section(body, headings) { + const wanted = new Set(headings.map(normaliseLabel)); + 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(normaliseLabel(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']); + } const login = String( pull && pull.user && pull.user.login || '' ); @@ -628,21 +665,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 +673,33 @@ 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 by the producing side that this is agent work. + const pullProvenance = knownAgents.has(login) || agentBranch || + manifestPresent || + carriesLabel((pull && pull.labels) || [], + ['agent', 'agenttask', 'mcpagent']); + // Issue-side dispatch. Label automation also applies agent task + // labels as topic tags to issues that never declared a contract, + // so the bare label is not 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). Only the + // two contract labels count here -- the generic `agent` label is + // never recognised by the snapshot job or the collector, so it + // stays a pull-request-side provenance signal only. + const issueLabelSource = selectedIssue && selectedIssue.labels + ? Array.isArray(selectedIssue.labels) + ? selectedIssue.labels + : selectedIssue.labels.nodes || [] + : []; + const issueDispatch = + carriesLabel(issueLabelSource, ['agenttask', 'mcpagent']) && + declaresAgentContract(selectedIssue); + return login !== 'dependabot[bot]' && + (pullProvenance || issueDispatch); } function intentContractErrors(issue, comments, pullCreatedAt) { const errors = []; @@ -1876,6 +1921,43 @@ jobs: return String(value || '').toLowerCase() .replace(/[^a-z0-9 ]/g, '').trim(); } + function carriesLabel(source, accepted) { + return (source || []).map(label => + typeof label === 'string' ? label : label.name + ).map(normaliseLabel).some(label => + accepted.includes(label) + ); + } + function declaresAgentContract(issue) { + function section(body, headings) { + const wanted = new Set(headings.map(normaliseLabel)); + 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(normaliseLabel(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']); + } const login = String( pull && pull.user && pull.user.login || '' ); @@ -1886,21 +1968,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 +1976,33 @@ 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 by the producing side that this is agent work. + const pullProvenance = knownAgents.has(login) || agentBranch || + manifestPresent || + carriesLabel((pull && pull.labels) || [], + ['agent', 'agenttask', 'mcpagent']); + // Issue-side dispatch. Label automation also applies agent task + // labels as topic tags to issues that never declared a contract, + // so the bare label is not 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). Only the + // two contract labels count here -- the generic `agent` label is + // never recognised by the snapshot job or the collector, so it + // stays a pull-request-side provenance signal only. + const issueLabelSource = selectedIssue && selectedIssue.labels + ? Array.isArray(selectedIssue.labels) + ? selectedIssue.labels + : selectedIssue.labels.nodes || [] + : []; + const issueDispatch = + carriesLabel(issueLabelSource, ['agenttask', 'mcpagent']) && + declaresAgentContract(selectedIssue); + return login !== 'dependabot[bot]' && + (pullProvenance || issueDispatch); } const prNumber = Number(process.env.INPUT_PR_NUMBER || 0); @@ -2163,6 +2253,27 @@ jobs: .replace(/^\x60|\x60$/g, '').trim(); const expectedAgentLogin = section(issueBody, ['agent login']) .replace(/^\x60|\x60$/g, '').trim(); + // A linked issue carrying an agent task label without declaring a + // run id and login was never dispatched to an agent -- the label + // is topic noise. Surface the mislabel so it is visible and + // correctable (#1130). Keyed on the missing contract itself, not + // on inapplicability, so pull requests that are inapplicable for + // other reasons (for example Dependabot's unconditional + // exclusion) never receive a notice falsely claiming a declared + // contract is missing. + const contractDeclared = [expectedRunId, expectedAgentLogin] + .every(value => value && value !== '_No response_'); + if (issue && !contractDeclared && + ['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. The label asserts an agent dispatch' + + ' contract; remove it from the issue if it was applied in' + + ' error.' + ); + } if (applicable && !expectedRunId) { collectionErrors.push('missing_agent_run_id'); } else if (applicable && String(manifest.run_id || '') !== expectedRunId) { diff --git a/tests/unit/test_agent_completion_gate.py b/tests/unit/test_agent_completion_gate.py index 301263cd1..b6d0b2c85 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,13 @@ 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 +3256,172 @@ 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` and `mcp/agent` as topic tags 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 (#1130). + """ + + 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], + // Only the two contract labels arm the issue side. The generic `agent` + // label is never recognised by the snapshot job or the collector, so an + // issue carrying it (even with contract headings) must not arm the gate: + // it would be permanently blocked as linked_issue_not_agent_task with no + // snapshot to satisfy. + ['generic agent label on issue', human, { + number: 7, labels: [{name: 'agent'}], body: CONTRACT + }, false], + // 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'}], 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, and it is keyed on the missing contract + itself rather than on inapplicability, so pull requests that are + inapplicable for unrelated reasons (Dependabot's unconditional + exclusion) never receive a notice falsely claiming a declared + contract is missing. + """ + + workflow = self._workflow() + + self.assertIn("mislabelled_agent_task", workflow) + self.assertNotIn( + "collectionErrors.push('mislabelled_agent_task')", + workflow, + ) + notice = workflow[workflow.index("mislabelled_agent_task") - 600:] + self.assertIn("core.notice(", notice[:800]) + self.assertIn("!contractDeclared", notice[:800]) + self.assertNotIn("!applicable", notice[:800]) + def test_workflow_does_not_approve_or_merge(self): workflow = self._workflow()