diff --git a/src/index.ts b/src/index.ts index 4286682b..627d3435 100644 --- a/src/index.ts +++ b/src/index.ts @@ -43,6 +43,7 @@ export { githubProjectWatchPrimitives, type WatchPrimitives, type WatchTick, + type WatchRunOutcome, type WatchLinearOptions, type WatchGithubOptions, type WatchGithubProjectOptions, diff --git a/src/runners/github.test.ts b/src/runners/github.test.ts new file mode 100644 index 00000000..f8e1909f --- /dev/null +++ b/src/runners/github.test.ts @@ -0,0 +1,60 @@ +import { describe, it, expect } from 'vitest'; +import { githubAdapter } from './github.js'; +import { GITHUB_SECRET_BLOCKED_LABEL } from '../github-labels.js'; +import type { RunGithubIssueDeps } from './github.js'; +import type { GhRunner } from '../tasks/github.js'; +import type { Task } from '../tasks/fetcher.js'; + +function makeDeps(): RunGithubIssueDeps { + return { repoPath: '/repo', repoSlug: 'owner/repo' }; +} + +function makeGh(failOn?: (args: string[]) => boolean): { gh: GhRunner; calls: string[][] } { + const calls: string[][] = []; + const gh: GhRunner = async (args) => { + calls.push(args); + if (failOn?.(args) === true) throw new Error('gh: failed'); + return ''; + }; + return { gh, calls }; +} + +const task: Task = { id: 'owner/repo#7', title: 't', description: '', labels: [], children: [], comments: [] }; + +describe('githubAdapter', () => { + it('signalSecretBlock creates the secret-blocked label before adding it, and posts the masked comment', async () => { + const { gh, calls } = makeGh(); + const adapter = githubAdapter(makeDeps(), gh); + + await adapter.signalSecretBlock('owner/repo#7', task, { + reason: 'findings', + findings: [{ file: '.env', patternName: 'generic-api-key', masked: 'KEY=ab****' }], + }); + + const create = calls.findIndex((c) => c[0] === 'label' && c[1] === 'create' && c[2] === GITHUB_SECRET_BLOCKED_LABEL && c.includes('--force')); + const add = calls.findIndex((c) => c[0] === 'issue' && c[1] === 'edit' && c[2] === '7' && c.includes('--add-label') && c.includes(GITHUB_SECRET_BLOCKED_LABEL)); + expect(create).toBeGreaterThan(-1); + expect(add).toBeGreaterThan(create); // `gh issue edit --add-label` fails on a repo without the label + const comment = calls.find((c) => c[0] === 'issue' && c[1] === 'comment' && c[2] === '7'); + expect(comment?.at(-1)).toContain('blocked publish'); + expect(comment?.at(-1)).toContain('KEY=ab****'); + expect(comment?.at(-1)).not.toContain('trigger label'); // the release step is per watch source, posted by the primitive + }); + + it('signalSecretBlock never throws: a failed label create still attempts the add and the comment', async () => { + const { gh, calls } = makeGh((args) => args[0] === 'label'); + const adapter = githubAdapter(makeDeps(), gh); + + await expect(adapter.signalSecretBlock('owner/repo#7', task, { reason: 'scan-error', message: 'gitleaks missing' })).resolves.toBeUndefined(); + + expect(calls.some((c) => c[0] === 'issue' && c[1] === 'edit' && c.includes(GITHUB_SECRET_BLOCKED_LABEL))).toBe(true); + expect(calls.some((c) => c[0] === 'issue' && c[1] === 'comment')).toBe(true); + }); + + it('signalSecretBlock never throws when every gh call fails', async () => { + const { gh } = makeGh(() => true); + const adapter = githubAdapter(makeDeps(), gh); + + await expect(adapter.signalSecretBlock('owner/repo#7', task, { reason: 'scan-error', message: 'x' })).resolves.toBeUndefined(); + }); +}); diff --git a/src/runners/github.ts b/src/runners/github.ts index 61169614..542b7b46 100644 --- a/src/runners/github.ts +++ b/src/runners/github.ts @@ -1,5 +1,5 @@ import { execa } from 'execa'; -import { GitHubTaskFetcher, linkPullRequest, addPrFailureLabel, editGithubLabels, commentGithubIssue } from '../tasks/github.js'; +import { GitHubTaskFetcher, linkPullRequest, addPrFailureLabel, editGithubLabels, commentGithubIssue, defaultGhRunner } from '../tasks/github.js'; import { GitHubProjectFetcher } from '../tasks/github-project.js'; import { implementReviewSimplifyStages } from '../pipeline/pipeline.js'; import { publishReviewVerdict } from '../pipeline/review-publish.js'; @@ -9,23 +9,25 @@ import { runSourcedIssue } from './source-adapter.js'; import { renderSecretBlockComment } from '../core/secret-scan.js'; import { GITHUB_VERIFY_FAILED_LABEL, GITHUB_VISUAL_PROOF_FAILED_LABEL, GITHUB_SECRET_BLOCKED_LABEL } from '../github-labels.js'; import type { Task } from '../tasks/fetcher.js'; +import type { GhRunner } from '../tasks/github.js'; import type { ProviderChoice } from '../agents/registry.js'; import type { FanOutOutcome } from '../pipeline/fan-out.js'; import type { SecretBlock } from '../core/secret-scan.js'; -import type { RunIssueDeps, SourceAdapter, ProofFailureKind } from './source-adapter.js'; +import type { RunIssueDeps, RunIssueResult, SourceAdapter, ProofFailureKind } from './source-adapter.js'; /** Everything needed to run a single GitHub issue end to end. */ export interface RunGithubIssueDeps extends RunIssueDeps { repoSlug: string; } -export interface RunGithubIssueResult { - task: Task; - /** Absent when the agent produced no changes (no PR opened). */ - prUrl?: string; -} +/** The shared run result, unnarrowed: `secretBlocked` must reach the watch loop so a withheld PR is not read as "no changes". */ +export type RunGithubIssueResult = RunIssueResult; -function githubAdapter(deps: RunGithubIssueDeps): SourceAdapter { +/** + * @internal Exported for unit tests; production callers use runGithubIssue. Only `signalSecretBlock` + * goes through the injected `gh`; `prepare`, `linkPr` and `addFailureLabel` still reach the real CLI. + */ +export function githubAdapter(deps: RunGithubIssueDeps, gh: GhRunner = defaultGhRunner): SourceAdapter { return { async prepare(issueRef: string) { const task = await new GitHubTaskFetcher(deps.repoSlug).fetch(issueRef); @@ -43,10 +45,16 @@ function githubAdapter(deps: RunGithubIssueDeps): SourceAdapter { await linkPullRequest(deps.repoSlug, issueRef, prUrl); }, async signalSecretBlock(issueRef: string, _task: Task, block: SecretBlock) { - await Promise.all([ - editGithubLabels(deps.repoSlug, issueRef, { add: [GITHUB_SECRET_BLOCKED_LABEL] }).catch(() => undefined), - commentGithubIssue(deps.repoSlug, issueRef, renderSecretBlockComment(block)).catch(() => undefined), - ]); + // `gh issue edit --add-label` fails on a repo that never created the label, which would leave + // the held issue with no Vanguard label at all. Ensure it first (the same pre-step as + // addPrFailureLabel and the revise path); every step stays best-effort. + const label = gh(['label', 'create', GITHUB_SECRET_BLOCKED_LABEL, '--repo', deps.repoSlug, '--force']) + .catch(() => undefined) + .then(() => editGithubLabels(deps.repoSlug, issueRef, { add: [GITHUB_SECRET_BLOCKED_LABEL] }, gh)) + .catch(() => undefined); + // No recovery procedure here: this adapter also serves one-shot `vanguard run` and Projects, + // whose release steps differ. The watch primitive that holds the issue posts the resume line. + await Promise.all([label, commentGithubIssue(deps.repoSlug, issueRef, renderSecretBlockComment(block), gh).catch(() => undefined)]); }, }; } diff --git a/src/runners/gitlab.test.ts b/src/runners/gitlab.test.ts index 2acaab45..004d8ac4 100644 --- a/src/runners/gitlab.test.ts +++ b/src/runners/gitlab.test.ts @@ -186,6 +186,42 @@ describe('gitlabAdapter', () => { expect(adapter.closeIssueOnMerge).toBe(true); }); + it('signalSecretBlock creates the secret-blocked label before adding it, and posts the masked note', async () => { + const { calls, glab } = makeGlab(); + const adapter = gitlabAdapter(makeDeps(), glab); + const task = { id: 'group/project#7', title: 't', description: '', labels: [], children: [], comments: [] }; + + await adapter.signalSecretBlock('group/project#7', task, { + reason: 'findings', + findings: [{ file: '.env', patternName: 'generic-api-key', masked: 'KEY=ab****' }], + }); + + const create = calls.findIndex((c) => c[0] === 'label' && c[1] === 'create' && c.includes('vanguard::secret-blocked')); + const add = calls.findIndex((c) => c[0] === 'issue' && c[1] === 'update' && c.includes('vanguard::secret-blocked')); + expect(create).toBeGreaterThan(-1); + expect(add).toBeGreaterThan(create); // the add would be a silent no-op without the label + const note = calls.find((c) => c[0] === 'issue' && c[1] === 'note'); + expect(note?.at(-1)).toContain('blocked publish'); + expect(note?.at(-1)).toContain('KEY=ab****'); + expect(note?.at(-1)).not.toContain('trigger label'); // the release step is per watch source, posted by the primitive + }); + + it('signalSecretBlock still adds the label and notes when label creation fails', async () => { + const calls: string[][] = []; + const glab: GlabRunner = async (args) => { + calls.push(args); + if (args[0] === 'label') throw new Error('403'); + return '{}'; + }; + const adapter = gitlabAdapter(makeDeps(), glab); + const task = { id: 'group/project#7', title: 't', description: '', labels: [], children: [], comments: [] }; + + await expect(adapter.signalSecretBlock('group/project#7', task, { reason: 'scan-error', message: 'gitleaks missing' })).resolves.toBeUndefined(); + + expect(calls.some((c) => c[0] === 'issue' && c[1] === 'update' && c.includes('vanguard::secret-blocked'))).toBe(true); + expect(calls.some((c) => c[0] === 'issue' && c[1] === 'note')).toBe(true); + }); + it('publishVerdict throws when reviewerOutcome is missing', async () => { const adapter = gitlabAdapter(makeDeps()); await expect( diff --git a/src/runners/gitlab.ts b/src/runners/gitlab.ts index 5c23e4d0..8dba9708 100644 --- a/src/runners/gitlab.ts +++ b/src/runners/gitlab.ts @@ -1,6 +1,6 @@ import { execa } from 'execa'; import { agentAuthFromEnv } from '../agents/auth.js'; -import { GitLabTaskFetcher, linkMergeRequest, addMrFailureLabel, editGitlabLabels, commentGitlabIssue } from '../tasks/gitlab.js'; +import { GitLabTaskFetcher, linkMergeRequest, addMrFailureLabel, editGitlabLabels, commentGitlabIssue, defaultGlabRunner } from '../tasks/gitlab.js'; import { implementReviewSimplifyStages } from '../pipeline/pipeline.js'; import { parseMergeRequestRef, postMergeRequestNote, mergeRequestReviewMarker } from './mr-review.js'; import { REVIEW_INCOMPLETE, stripReviewMarkers } from './review-prompt.js'; @@ -13,7 +13,7 @@ import type { Task } from '../tasks/fetcher.js'; import type { ProviderChoice } from '../agents/registry.js'; import type { GlabRunner } from '../tasks/gitlab.js'; import type { SecretBlock } from '../core/secret-scan.js'; -import type { RunIssueDeps, SourceAdapter, PublishVerdictInput, ProofFailureKind } from './source-adapter.js'; +import type { RunIssueDeps, RunIssueResult, SourceAdapter, PublishVerdictInput, ProofFailureKind } from './source-adapter.js'; /** Everything needed to run a single GitLab issue end to end. */ export interface RunGitlabIssueDeps extends RunIssueDeps { @@ -21,10 +21,8 @@ export interface RunGitlabIssueDeps extends RunIssueDeps { project: string; } -export interface RunGitlabIssueResult { - task: Task; - prUrl?: string; -} +/** The shared run result, unnarrowed: `secretBlocked` must reach the watch loop so a withheld MR is not read as "no changes". */ +export type RunGitlabIssueResult = RunIssueResult; /** @internal Exported for unit tests; production callers use runGitlabIssue. */ export function gitlabAdapter(deps: RunGitlabIssueDeps, glab?: GlabRunner): SourceAdapter { @@ -43,10 +41,15 @@ export function gitlabAdapter(deps: RunGitlabIssueDeps, glab?: GlabRunner): Sour await linkMergeRequest(deps.project, issueRef, mrUrl, glab); }, async signalSecretBlock(issueRef: string, _task: Task, block: SecretBlock) { - await Promise.all([ - editGitlabLabels(deps.project, issueRef, { add: [GITLAB_SECRET_BLOCKED_LABEL] }, glab).catch(() => undefined), - commentGitlabIssue(deps.project, issueRef, renderSecretBlockComment(block), glab).catch(() => undefined), - ]); + // `glab issue update --label` is a silent no-op on a project lacking the label (see + // addMrFailureLabel), which would leave the held issue with no Vanguard label at all. Ensure + // it first; every step stays best-effort. + const label = (glab ?? defaultGlabRunner)(['label', 'create', '--repo', deps.project, '--name', GITLAB_SECRET_BLOCKED_LABEL]) + .catch(() => undefined) + .then(() => editGitlabLabels(deps.project, issueRef, { add: [GITLAB_SECRET_BLOCKED_LABEL] }, glab)) + .catch(() => undefined); + // No recovery procedure here (the watch primitive that holds the issue posts the resume line). + await Promise.all([label, commentGitlabIssue(deps.project, issueRef, renderSecretBlockComment(block), glab).catch(() => undefined)]); }, }; } diff --git a/src/runners/linear.ts b/src/runners/linear.ts index dd6be190..7b4a5d44 100644 --- a/src/runners/linear.ts +++ b/src/runners/linear.ts @@ -15,7 +15,7 @@ import type { CustomProviderEntry } from '../agents/registry.js'; import type { FanOutOutcome } from '../pipeline/fan-out.js'; import type { SecretBlock } from '../core/secret-scan.js'; import type { GlabRunner } from '../tasks/gitlab.js'; -import type { RunIssueDeps, SourceAdapter, PublishVerdictInput, ProofFailureKind } from './source-adapter.js'; +import type { RunIssueDeps, RunIssueResult, SourceAdapter, PublishVerdictInput, ProofFailureKind } from './source-adapter.js'; /** Everything needed to run a single Linear issue end to end. */ export interface RunLinearIssueDeps extends RunIssueDeps { @@ -23,11 +23,8 @@ export interface RunLinearIssueDeps extends RunIssueDeps { skillsDir: string; } -export interface RunLinearIssueResult { - task: Task; - /** Absent when the agent produced no changes (no PR opened). */ - prUrl?: string; -} +/** The shared run result, unnarrowed: `secretBlocked` must reach the watch loop so a withheld PR is not read as "no changes". */ +export type RunLinearIssueResult = RunIssueResult; /** * The canonical implement/review/simplify pipeline with only the implementer's prompt swapped, so @@ -85,7 +82,8 @@ export function linearAdapter(deps: RunLinearIssueDeps, gitlabProject?: string, await linkLinearIssue(task.id, prUrl); }, async signalSecretBlock(_issueRef: string, task: Task, block: SecretBlock) { - // Linear uses workflow states, not labels; the comment is the primary signal. + // Linear uses workflow states, not labels; the comment is the primary signal. The watch + // primitive that holds the issue posts how to release it (it knows the configured states). try { await commentLinearIssue(task.id, renderSecretBlockComment(block)); } catch { /* best-effort */ } diff --git a/src/runners/watch.test.ts b/src/runners/watch.test.ts index aaeef9ff..e008920c 100644 --- a/src/runners/watch.test.ts +++ b/src/runners/watch.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, expectTypeOf, vi } from 'vitest'; import { getEventListeners } from 'node:events'; import { watchOnce, @@ -10,8 +10,14 @@ import { githubIssueWatchPrimitives, gitlabWatchPrimitives, } from './watch.js'; -import { GITHUB_CLAIMED_LABEL, GITHUB_REVIEW_LABEL, GITHUB_SPEC_CLAIMED_LABEL } from '../github-labels.js'; +import { runGithubIssue } from './github.js'; +import { runGitlabIssue } from './gitlab.js'; +import { runLinearIssue } from './linear.js'; +import { GITHUB_CLAIMED_LABEL, GITHUB_REVIEW_LABEL, GITHUB_SPEC_CLAIMED_LABEL, GITHUB_SECRET_BLOCKED_LABEL } from '../github-labels.js'; import type { SpecWatchPrimitives, WatchPrimitives, WatchGitlabOptions } from './watch.js'; +import type { RunGithubIssueResult } from './github.js'; +import type { RunGitlabIssueResult } from './gitlab.js'; +import type { RunLinearIssueResult } from './linear.js'; import type { GhRunner } from '../tasks/github.js'; import type { TaskFetcher } from '../tasks/fetcher.js'; import type { LinearCliRunner } from '../tasks/linear-cli.js'; @@ -57,6 +63,97 @@ describe('watchOnce', () => { expect(calls).toContain('fail:C'); }); + it('holds a secret-blocked run for a human: onSecretBlocked instead of onNoChange, counted apart from no-change', async () => { + const calls: string[] = []; + const primitives: WatchPrimitives = { + listReady: async () => [{ id: 'A' }, { id: 'B' }], + claim: async (id) => { + calls.push(`claim:${id}`); + }, + // A: the secret scan withheld the PR (no prUrl). B: genuine empty diff. + runOne: async (id) => (id === 'A' ? { secretBlocked: true } : {}), + review: async (id) => { + calls.push(`review:${id}`); + }, + onNoChange: async (id) => { + calls.push(`nochange:${id}`); + }, + onSecretBlocked: async (id) => { + calls.push(`secret:${id}`); + }, + onFailure: async (id) => { + calls.push(`fail:${id}`); + }, + }; + + const tick = await watchOnce(primitives, { concurrency: 1 }); + + expect(tick.secretBlocked).toEqual(['A']); + expect(tick.noChange).toEqual(['B']); + expect(tick.opened).toEqual([]); + expect(tick.failed).toEqual([]); + expect(calls).toContain('secret:A'); + expect(calls).not.toContain('nochange:A'); // the claim is never reverted to the trigger + expect(calls).not.toContain('review:A'); + expect(calls).toContain('nochange:B'); + expect(calls).not.toContain('secret:B'); + }); + + it('a secret-blocked run without an onSecretBlocked primitive still skips onNoChange (claim stays)', async () => { + const onNoChange = vi.fn(async () => {}); + const primitives: WatchPrimitives = { + listReady: async () => [{ id: 'A' }], + claim: async () => {}, + runOne: async () => ({ secretBlocked: true }), + review: async () => {}, + onNoChange, + onFailure: async () => {}, + }; + + const tick = await watchOnce(primitives); + + expect(tick.secretBlocked).toEqual(['A']); + expect(tick.noChange).toEqual([]); + expect(onNoChange).not.toHaveBeenCalled(); + }); + + it('the per-source runners keep secretBlocked in their result type so the watch loop can see it', () => { + // runSourcedIssue returns { task, secretBlocked: true } on a withheld PR; a narrowed wrapper + // type would let watchOnce read that as "no changes" and revert the claim. + expectTypeOf().toHaveProperty('secretBlocked'); + expectTypeOf().toHaveProperty('secretBlocked'); + expectTypeOf().toHaveProperty('secretBlocked'); + // ...and the real seams, so a re-narrowed signature on the function itself cannot slip through. + expectTypeOf(runGithubIssue).returns.resolves.toHaveProperty('secretBlocked'); + expectTypeOf(runGitlabIssue).returns.resolves.toHaveProperty('secretBlocked'); + expectTypeOf(runLinearIssue).returns.resolves.toHaveProperty('secretBlocked'); + }); + + it('a rejecting onSecretBlocked is reported as a failure and never reverts the claim', async () => { + const onNoChange = vi.fn(async () => {}); + const onFailure = vi.fn(async () => {}); + const primitives: WatchPrimitives = { + listReady: async () => [{ id: 'A' }], + claim: async () => {}, + runOne: async () => ({ secretBlocked: true }), + review: async () => {}, + onNoChange, + onSecretBlocked: async () => { + throw new Error('label API down'); + }, + onFailure, + }; + const logs: string[] = []; + + const tick = await watchOnce(primitives, { log: (msg) => logs.push(msg) }); + + expect(tick.failed).toEqual(['A']); + expect(tick.secretBlocked).toEqual([]); + expect(onFailure).toHaveBeenCalledWith('A', expect.any(Error)); + expect(onNoChange).not.toHaveBeenCalled(); // the claim is never reverted to the trigger + expect(logs).toContain('watch A: secret blocked -> held for a human'); // logged before the edit, so the block is on record + }); + it('does not call onNoChange when runOne reports the outcome is already parked', async () => { const calls: string[] = []; const primitives: WatchPrimitives = { @@ -383,6 +480,38 @@ describe('runLoopV1', () => { expect(builtIds).toEqual(['A', 'B']); }); + it('the watch summary line counts secret-blocked runs, and omits the suffix on a clean tick', async () => { + const logs: string[] = []; + const specPrimitives: SpecWatchPrimitives = { + listReady: async () => [], + claim: async () => {}, + runSpec: async () => 'advanced', + onFailure: async () => {}, + }; + const agentPrimitives: WatchPrimitives = { + listReady: async () => [{ id: 'A' }, { id: 'B' }], + claim: async () => {}, + runOne: async (id) => (id === 'A' ? { secretBlocked: true } : { prUrl: 'pr/B' }), + review: async () => {}, + onNoChange: async () => {}, + onFailure: async () => {}, + }; + + await runLoopV1(specPrimitives, agentPrimitives, { once: true, concurrency: 1 }, (msg) => logs.push(msg)); + + expect(logs).toContain('watch A: secret blocked -> held for a human'); + expect(logs).toContain('watch: 1 PR(s), 0 no-change, 0 failed, 0 skipped, 1 secret-blocked.'); + + logs.length = 0; + await runLoopV1( + specPrimitives, + { ...agentPrimitives, runOne: async (id) => ({ prUrl: `pr/${id}` }) }, + { once: true, concurrency: 1 }, + (msg) => logs.push(msg), + ); + expect(logs).toContain('watch: 2 PR(s), 0 no-change, 0 failed, 0 skipped.'); + }); + // T3 — once mode: needs-info tickets are NOT carried into the agent pass it('once: true does not build tickets the spec pass moved to needs-info', async () => { const builtIds: string[] = []; @@ -736,6 +865,103 @@ describe('githubProjectWatchPrimitives', () => { expect(commentCall?.[bodyIdx + 1]).toContain('agent exploded'); }); + it('a secret-blocked run is not reverted to the trigger status and gets no no-change comment', async () => { + const ghCalls: string[][] = []; + const primitives = githubProjectWatchPrimitives({ + deps: { auth: { type: 'api', apiKey: 'test' } as never, repoPath: '/tmp', repoSlug: 'owner/repo' }, + projectNumber: 1, + label: 'vanguard', + triggerStatus: 'Todo', + claimedStatus: 'In Progress', + reviewStatus: 'In Review', + gh: makeFakeGh(ghCalls), + }); + + // What runGithubIssue returns when deliverChange hit the outgoing secret scan. + const tick = await watchOnce({ ...primitives, runOne: async () => ({ secretBlocked: true }) }, { concurrency: 1 }); + + // Claimed (opt_inprogress) but never moved back to Todo (opt_todo) — a revert would re-list it next poll. + expect(ghCalls.some((a) => a[1] === 'item-edit' && a.includes('opt_todo'))).toBe(false); + const comments = ghCalls.filter((a) => a[0] === 'issue' && a[1] === 'comment'); + expect(comments).toHaveLength(1); + const hold = comments[0]?.[comments[0].indexOf('--body') + 1] ?? ''; + expect(hold).not.toContain('no changes'); + expect(hold).toContain('stays in "In Progress"'); + expect(hold).toContain('move it back to "Todo"'); // Projects has no trigger label to re-apply + expect(ghCalls.some((a) => a[1] === 'item-edit' && a.includes('opt_inprogress'))).toBe(true); + expect(tick.secretBlocked).toEqual(['owner/repo#1']); + expect(tick.noChange).toEqual([]); + }); + + it('claim clears a stale secret-blocked label only on items listReady saw carrying it', async () => { + const ghCalls: string[][] = []; + const itemList = JSON.stringify({ + items: [ + { id: 'PVTI_1', status: 'Todo', content: { type: 'Issue', number: 1, repository: 'owner/repo', labels: ['vanguard', GITHUB_SECRET_BLOCKED_LABEL] } }, + { id: 'PVTI_2', status: 'Todo', content: { type: 'Issue', number: 2, repository: 'owner/repo', labels: ['vanguard'] } }, + ], + }); + const gh: GhRunner = async (args) => { + ghCalls.push(args); + if (args[0] === 'project' && args[1] === 'item-list') return itemList; + if (args[0] === 'project' && args[1] === 'view') return PROJECT_VIEW; + if (args[0] === 'project' && args[1] === 'field-list') return FIELD_LIST; + return ''; + }; + const primitives = githubProjectWatchPrimitives({ + deps: { auth: { type: 'api', apiKey: 'test' } as never, repoPath: '/tmp', repoSlug: 'owner/repo' }, + projectNumber: 1, + label: 'vanguard', + triggerStatus: 'Todo', + claimedStatus: 'In Progress', + reviewStatus: 'In Review', + gh, + }); + + const ready = await primitives.listReady(); + expect(ready.map((r) => r.id)).toEqual(['owner/repo#1', 'owner/repo#2']); + for (const { id } of ready) await primitives.claim(id); + + const labelEdits = ghCalls.filter((a) => a[0] === 'issue' && a[1] === 'edit'); + expect(labelEdits).toHaveLength(1); // only the re-triggered item + expect(labelEdits[0]?.[2]).toBe('1'); + expect(labelEdits[0]?.[labelEdits[0].indexOf('--remove-label') + 1]).toBe(GITHUB_SECRET_BLOCKED_LABEL); + expect(ghCalls.filter((a) => a[1] === 'item-edit' && a.includes('opt_inprogress'))).toHaveLength(2); // both claimed + expect(labelEdits[0]?.[labelEdits[0].indexOf('--repo') + 1]).toBe('owner/repo'); // the issue's own repo, not the watch's + }); + + it('claim survives a failing stale-label cleanup: the item is claimed and run, not read as taken', async () => { + const itemList = JSON.stringify({ + items: [{ id: 'PVTI_1', status: 'Todo', content: { type: 'Issue', number: 7, repository: 'other/repo', labels: ['vanguard', GITHUB_SECRET_BLOCKED_LABEL] } }], + }); + const ghCalls: string[][] = []; + const gh: GhRunner = async (args) => { + ghCalls.push(args); + if (args[0] === 'project' && args[1] === 'item-list') return itemList; + if (args[0] === 'project' && args[1] === 'view') return PROJECT_VIEW; + if (args[0] === 'project' && args[1] === 'field-list') return FIELD_LIST; + if (args[0] === 'issue' && args[1] === 'edit') throw new Error('HTTP 502'); + return ''; + }; + const primitives = githubProjectWatchPrimitives({ + deps: { auth: { type: 'api', apiKey: 'test' } as never, repoPath: '/tmp', repoSlug: 'owner/repo' }, + projectNumber: 1, + label: 'vanguard', + triggerStatus: 'Todo', + claimedStatus: 'In Progress', + reviewStatus: 'In Review', + gh, + }); + + const ran: string[] = []; + const tick = await watchOnce({ ...primitives, runOne: async (id) => { ran.push(id); return { prUrl: 'pr/7' }; } }, { concurrency: 1 }); + + expect(ran).toEqual(['other/repo#7']); + expect(tick.skipped).toEqual([]); + const cleanup = ghCalls.find((a) => a[0] === 'issue' && a[1] === 'edit'); + expect(cleanup?.[cleanup.indexOf('--repo') + 1]).toBe('other/repo'); // the board item's repo, not the watch's + }); + it('onNoChange comments and reverts status to the trigger status', async () => { const ghCalls: string[][] = []; const primitives = githubProjectWatchPrimitives({ @@ -864,6 +1090,98 @@ describe('githubIssueWatchPrimitives ownerLabel', () => { expect(firstArgs[labelIdx + 1]).toBe('ready for agent'); }); + it('a secret-blocked run clears the claimed label without restoring the trigger label or posting a no-change comment', async () => { + const gh = makeGhSpy(); + const primitives = githubIssueWatchPrimitives({ + deps: { auth: { type: 'api', apiKey: 'k' } as never, repoPath: '/tmp', repoSlug: 'owner/repo' }, + label: 'ready for agent', + claimedLabel: GITHUB_CLAIMED_LABEL, + reviewLabel: GITHUB_REVIEW_LABEL, + gh, + }); + + const tick = await watchOnce( + { ...primitives, listReady: async () => [{ id: '1' }], runOne: async () => ({ secretBlocked: true }) }, + { concurrency: 1 }, + ); + + const calls = (gh as ReturnType).mock.calls.map(([args]) => args as string[]); + const comments = calls.filter((args) => args[0] === 'issue' && args[1] === 'comment'); + expect(comments).toHaveLength(1); // the hold comment, not the no-change one + const hold = comments[0]?.[comments[0].indexOf('--body') + 1] ?? ''; + expect(hold).not.toContain('no changes'); + expect(hold).toContain('secret scan'); + expect(hold).toContain('re-apply the `ready for agent` label'); // the configured trigger label + const edits = calls.filter((args) => args[0] === 'issue' && args[1] === 'edit'); + // claim: trigger -> running; secret block: ONE edit swapping running -> secret-blocked, so the + // claim is never released without the hold label; the trigger label is not added back. + expect(edits).toHaveLength(2); + const release = edits[1]; + expect(release?.[release.indexOf('--remove-label') + 1]).toBe(GITHUB_CLAIMED_LABEL); + expect(release?.[release.indexOf('--add-label') + 1]).toBe(GITHUB_SECRET_BLOCKED_LABEL); + expect(release).not.toContain('ready for agent'); + expect(tick.secretBlocked).toEqual(['1']); + expect(tick.noChange).toEqual([]); + }); + + it('claim clears a stale secret-blocked label only on issues listReady saw carrying it', async () => { + const gh: GhRunner = vi.fn().mockResolvedValue( + JSON.stringify([ + { number: 1, title: 'retried', body: '', labels: [{ name: 'ready for agent' }, { name: GITHUB_SECRET_BLOCKED_LABEL }] }, + { number: 2, title: 'fresh', body: '', labels: [{ name: 'ready for agent' }] }, + ]), + ); + const primitives = githubIssueWatchPrimitives({ + deps: { auth: { type: 'api', apiKey: 'k' } as never, repoPath: '/tmp', repoSlug: 'owner/repo' }, + label: 'ready for agent', + claimedLabel: GITHUB_CLAIMED_LABEL, + reviewLabel: GITHUB_REVIEW_LABEL, + gh, + }); + + const ready = await primitives.listReady(); + for (const { id } of ready) await primitives.claim(id); + + const calls = (gh as ReturnType).mock.calls.map(([args]) => args as string[]); + const edits = calls.filter((args) => args[0] === 'issue' && args[1] === 'edit'); + expect(edits).toHaveLength(2); + const removed = (edit: string[] | undefined): string[] => + (edit ?? []).flatMap((arg, i, a) => (arg === '--remove-label' ? [a[i + 1] as string] : [])); + expect(removed(edits.find((e) => e[2] === '1'))).toEqual(['ready for agent', GITHUB_SECRET_BLOCKED_LABEL]); + expect(removed(edits.find((e) => e[2] === '2'))).toEqual(['ready for agent']); // no blind remove + }); + + it('white-label: the hold is still labelled and explained (the claim label is already the watch\'s own marker)', async () => { + const gh = makeGhSpy(); + const primitives = githubIssueWatchPrimitives({ + deps: { auth: { type: 'api', apiKey: 'k' } as never, repoPath: '/tmp', repoSlug: 'owner/repo', commitAuthor: { name: 'Dev', email: 'dev@example.com' } }, + label: 'ready for agent', + claimedLabel: GITHUB_CLAIMED_LABEL, + reviewLabel: GITHUB_REVIEW_LABEL, + gh, + }); + + const tick = await watchOnce( + { ...primitives, listReady: async () => [{ id: '1' }], runOne: async () => ({ secretBlocked: true }) }, + { concurrency: 1 }, + ); + + const calls = (gh as ReturnType).mock.calls.map(([args]) => args as string[]); + const edits = calls.filter((args) => args[0] === 'issue' && args[1] === 'edit'); + expect(edits).toHaveLength(2); // claim, then the swap running -> secret-blocked + expect(edits[1]?.[edits[1].indexOf('--add-label') + 1]).toBe(GITHUB_SECRET_BLOCKED_LABEL); + // The white-label adapter never pre-created the label (it posts no findings), and + // `gh issue edit --add-label` fails on an unknown label: the primitive ensures it before the swap. + const created = calls.findIndex((args) => args[0] === 'label' && args[1] === 'create' && args[2] === GITHUB_SECRET_BLOCKED_LABEL); + expect(created).toBeGreaterThan(-1); + expect(created).toBeLessThan(calls.indexOf(edits[1] as string[])); + // The white-label adapter posts no findings, so this is the human's only signal. + const comments = calls.filter((args) => args[0] === 'issue' && args[1] === 'comment'); + expect(comments).toHaveLength(1); + expect(comments[0]?.[comments[0].indexOf('--body') + 1]).toContain('re-apply the `ready for agent` label'); + expect(tick.secretBlocked).toEqual(['1']); + }); + it('onNoChange removes the claimed label and posts a no-change comment', async () => { const gh = makeGhSpy(); const primitives = githubIssueWatchPrimitives({ @@ -891,6 +1209,37 @@ describe('githubIssueWatchPrimitives ownerLabel', () => { }); describe('linearWatchPrimitives', () => { + it('a secret-blocked run stays in the claimed state: no revert to the trigger state, no no-change comment', async () => { + const calls: string[][] = []; + const linear: LinearCliRunner = async (args) => { + calls.push(args); + return '[]'; + }; + const primitives = linearWatchPrimitives({ + deps: { auth: { type: 'api', apiKey: 'x' }, linearKey: 'k', repoPath: '/tmp', skillsDir: '/s' } as never, + label: 'vanguard', + triggerStateName: 'Todo', + claimedState: 'In Progress', + reviewState: 'In Review', + linear, + }); + + const tick = await watchOnce( + { ...primitives, listReady: async () => [{ id: 'ENG-1' }], runOne: async () => ({ secretBlocked: true }) }, + { concurrency: 1 }, + ); + + const updates = calls.filter((a) => a[0] === 'issue' && a[1] === 'update'); + expect(updates).toEqual([['issue', 'update', 'ENG-1', '--state', 'In Progress']]); // claim only; never back to Todo + const comments = calls.filter((a) => a[0] === 'issue' && a[1] === 'comment'); + expect(comments).toHaveLength(1); + expect(comments[0]?.join(' ')).not.toContain('no changes'); + expect(comments[0]?.join(' ')).toContain('stays in "In Progress"'); + expect(comments[0]?.join(' ')).toContain('move it back to "Todo"'); + expect(tick.secretBlocked).toEqual(['ENG-1']); + expect(tick.noChange).toEqual([]); + }); + it('onNoChange comments and reverts to the trigger state name when configured', async () => { const calls: string[][] = []; const linear: LinearCliRunner = async (args) => { @@ -981,6 +1330,78 @@ describe('gitlabWatchPrimitives', () => { expect(noteCall?.some((arg) => arg.includes('boom'))).toBe(true); }); + it('a secret-blocked run clears the claimed label without restoring the trigger label or posting a no-change note', async () => { + const { glab, calls } = makeGlab(); + const opts = makeOpts(); + const primitives = gitlabWatchPrimitives({ ...opts, gl: glab }); + + const tick = await watchOnce( + { ...primitives, listReady: async () => [{ id: 'g/p#1' }], runOne: async () => ({ secretBlocked: true }) }, + { concurrency: 1 }, + ); + + const notes = calls.filter((c) => c[0] === 'issue' && c[1] === 'note'); + expect(notes).toHaveLength(1); // the hold note, not the no-change one + expect(notes[0]?.join(' ')).not.toContain('no changes'); + expect(notes[0]?.join(' ')).toContain('re-apply the `vanguard` label'); // the configured trigger label + const updates = calls.filter((c) => c[0] === 'issue' && c[1] === 'update'); + expect(updates).toHaveLength(2); // claim, then ONE swap running -> secret-blocked + const release = updates[1]; + expect(release?.[release.indexOf('--unlabel') + 1]).toBe('vanguard::running'); + expect(release?.[release.indexOf('--label') + 1]).toBe('vanguard::secret-blocked'); + expect(release).not.toContain('--label vanguard'); + expect(tick.secretBlocked).toEqual(['g/p#1']); + expect(tick.noChange).toEqual([]); + }); + + it('claim clears a stale secret-blocked label only on issues listReady saw carrying it', async () => { + const { glab, calls } = makeGlab({ + 'issue:list': JSON.stringify([ + { iid: 1, title: 'retried', description: null, labels: ['vanguard', 'vanguard::secret-blocked'] }, + { iid: 2, title: 'fresh', description: null, labels: ['vanguard'] }, + ]), + }); + const primitives = gitlabWatchPrimitives({ ...makeOpts(), gl: glab }); + + const ready = await primitives.listReady(); + for (const { id } of ready) await primitives.claim(id); + + const updates = calls.filter((c) => c[0] === 'issue' && c[1] === 'update'); + expect(updates).toHaveLength(2); + const unlabelled = (u: string[] | undefined): string[] => + (u ?? []).flatMap((arg, i, a) => (arg === '--unlabel' ? [a[i + 1] as string] : [])); + expect(unlabelled(updates.find((u) => u[2] === '1'))).toEqual(['vanguard', 'vanguard::secret-blocked']); + expect(unlabelled(updates.find((u) => u[2] === '2'))).toEqual(['vanguard']); + }); + + it('white-label: the hold is still labelled and explained (the claim label is already the watch\'s own marker)', async () => { + const { glab, calls } = makeGlab(); + const opts = makeOpts(); + const primitives = gitlabWatchPrimitives({ + ...opts, + deps: { ...opts.deps, commitAuthor: { name: 'Dev', email: 'dev@example.com' } }, + gl: glab, + }); + + const tick = await watchOnce( + { ...primitives, listReady: async () => [{ id: 'g/p#1' }], runOne: async () => ({ secretBlocked: true }) }, + { concurrency: 1 }, + ); + + const updates = calls.filter((c) => c[0] === 'issue' && c[1] === 'update'); + expect(updates).toHaveLength(2); // claim, then the swap running -> secret-blocked + expect(updates[1]?.[updates[1].indexOf('--label') + 1]).toBe('vanguard::secret-blocked'); + // `glab issue update --label` is a silent no-op on an unknown label while `--unlabel` in the + // same call lands; the primitive ensures the label before the swap (the white-label adapter never did). + const created = calls.findIndex((c) => c[0] === 'label' && c[1] === 'create' && c.includes('vanguard::secret-blocked')); + expect(created).toBeGreaterThan(-1); + expect(created).toBeLessThan(calls.indexOf(updates[1] as string[])); + const notes = calls.filter((c) => c[0] === 'issue' && c[1] === 'note'); + expect(notes).toHaveLength(1); + expect(notes[0]?.join(' ')).toContain('re-apply the `vanguard` label'); + expect(tick.secretBlocked).toEqual(['g/p#1']); + }); + it('onNoChange removes the claimed label and posts a no-change comment, without re-adding the trigger label', async () => { const { glab, calls } = makeGlab(); const opts = makeOpts(); diff --git a/src/runners/watch.ts b/src/runners/watch.ts index d5f36f35..8ec2bf0d 100644 --- a/src/runners/watch.ts +++ b/src/runners/watch.ts @@ -8,6 +8,8 @@ import { runSpecGenerator } from './spec.js'; import { assessTaskReadiness, isVanguardSpec, SPEC_TAG } from '../tasks/triage.js'; import { fanOut } from '../pipeline/fan-out.js'; import { failureReason, formatFailureComment } from '../core/errors.js'; +import { GITHUB_SECRET_BLOCKED_LABEL } from '../github-labels.js'; +import { GITLAB_SECRET_BLOCKED_LABEL } from '../gitlab-labels.js'; import type { Task } from '../tasks/fetcher.js'; import type { RunLinearIssueDeps } from './linear.js'; import type { RunGithubIssueDeps } from './github.js'; @@ -41,26 +43,54 @@ export function specComment(spec: string, opts?: { whiteLabel?: boolean }): stri const NO_CHANGE_MSG = 'Vanguard run produced no changes — no PR was opened. Cleared `vanguard:running`; re-apply the trigger label to retry. See the run log.'; +/** + * Comment posted when the secret scan withheld the PR: what the watch did with the claim and the + * exact step that releases the hold on THIS source (the configured trigger label / status / state). + * The adapter's masked-findings comment carries no procedure — it also serves one-shot runs. + */ +function holdMessage(claimNote: string, resume: string): string { + return `Vanguard held this issue: the outgoing secret scan withheld the PR. ${claimNote} To retry once the secret is out of the change, ${resume}.`; +} + /** Post the no-change comment and (optionally) revert the claim — independent I/O, run concurrently. */ async function commentAndRevert(comment: Promise, revert?: Promise): Promise { await Promise.all(revert === undefined ? [comment] : [comment, revert]); } +/** + * Outcome of one issue run as the watch loop sees it. No `prUrl` has three meanings, and only the + * first is a genuine no-change: the agent ran and the diff was empty (`onNoChange` un-sticks the + * claim); `parked` — the agent never ran, triage already routed the ticket elsewhere (needs-info); + * `secretBlocked` — the diff was withheld by the outgoing secret scan and the adapter already + * labelled/commented the issue. The last must NOT revert the claim: sources that revert to the + * trigger state (Linear, GitHub Projects) would re-list the issue next poll, re-run it, re-hit the + * secret and re-post the block comment forever. + */ +export interface WatchRunOutcome { + prUrl?: string; + parked?: boolean; + secretBlocked?: boolean; +} + export interface WatchPrimitives { /** List issues currently ready to run (trigger state + label). */ listReady: () => Promise>; /** Claim an issue so a later poll won't pick it again (e.g. move it out of the trigger state). */ claim: (id: string) => Promise; - /** - * `parked` distinguishes a genuine no-change (agent ran, empty diff) from a triage-park (agent - * never ran because the ticket was already routed elsewhere, e.g. needs-info). Only the former - * should trigger `onNoChange`. - */ - runOne: (id: string) => Promise<{ prUrl?: string; parked?: boolean }>; + /** See WatchRunOutcome: only a genuine no-change (no PR, not parked, not secret-blocked) triggers `onNoChange`. */ + runOne: (id: string) => Promise; /** Mark an issue as in review (a PR opened). */ review: (id: string) => Promise; /** No PR AND not parked: undo the claim so the issue is not stuck on the claimed marker. */ onNoChange: (id: string) => Promise; + /** + * The secret scan withheld the PR. Mark the hold WITHOUT restoring the trigger (a revert would + * re-list the issue next poll) and tell the human how to release it: label sources swap the + * claimed label for the secret-blocked label; state sources keep the claimed state. Every real + * source supplies it; absent (test doubles) the issue simply stays claimed. A rejection routes to + * onFailure and the claim stays. + */ + onSecretBlocked?: (id: string) => Promise; onFailure: (id: string, error: unknown) => Promise; } @@ -72,9 +102,11 @@ export interface WatchTick { skipped: string[]; /** Ready but left unclaimed for the next poll, because --max-tasks was already met. */ deferred: string[]; + /** PR withheld by the secret scan; held for a human (never reverted to the trigger). */ + secretBlocked: string[]; } -type Kind = 'opened' | 'noChange' | 'failed' | 'skipped' | 'deferred'; +type Kind = 'opened' | 'noChange' | 'failed' | 'skipped' | 'deferred' | 'secretBlocked'; interface WatchLogOptions { log?: (msg: string) => void; @@ -101,6 +133,11 @@ function deferredNote(tick: { deferred: string[] }): string { return tick.deferred.length > 0 ? `, ${tick.deferred.length} deferred by --max-tasks` : ''; } +/** Summary suffix for runs the secret scan withheld; empty on a clean poll. */ +function secretBlockedNote(tick: { secretBlocked: string[] }): string { + return tick.secretBlocked.length > 0 ? `, ${tick.secretBlocked.length} secret-blocked` : ''; +} + function logPoll(ready: number, opts: WatchOnceOptions, phase: string): void { const cappedNote = opts.maxTasks !== undefined && opts.maxTasks < ready ? ` (capped to ${opts.maxTasks} by --max-tasks)` : ''; operatorLog(opts, `${phase}: poll -> ${ready} ready${cappedNote}`); @@ -156,7 +193,13 @@ export async function watchOnce(primitives: WatchPrimitives, opts: WatchOnceOpti } operatorLog(opts, `${phase} ${item.id}: claim -> running`); try { - const { prUrl, parked } = await primitives.runOne(item.id); + const { prUrl, parked, secretBlocked } = await primitives.runOne(item.id); + if (secretBlocked === true) { + // Log first: if the hold edit below throws, the failure comment must not be the only record. + operatorLog(opts, `${phase} ${item.id}: secret blocked -> held for a human`); + await primitives.onSecretBlocked?.(item.id); + return { id: item.id, kind: 'secretBlocked' }; + } if (prUrl === undefined) { if (parked !== true) await primitives.onNoChange(item.id); operatorLog(opts, `${phase} ${item.id}: no change -> idle`); @@ -175,7 +218,14 @@ export async function watchOnce(primitives: WatchPrimitives, opts: WatchOnceOpti ); const ids = (kind: Kind): string[] => results.flatMap((o) => (o.status === 'fulfilled' && o.value.kind === kind ? [o.value.id] : [])); - return { opened: ids('opened'), noChange: ids('noChange'), failed: ids('failed'), skipped: ids('skipped'), deferred: ids('deferred') }; + return { + opened: ids('opened'), + noChange: ids('noChange'), + failed: ids('failed'), + skipped: ids('skipped'), + deferred: ids('deferred'), + secretBlocked: ids('secretBlocked'), + }; } export interface SpecWatchPrimitives { @@ -305,6 +355,19 @@ export function linearWatchPrimitives(opts: WatchLinearOptions): WatchPrimitives commentLinearIssue(id, NO_CHANGE_MSG, opts.linear), opts.triggerStateName !== undefined ? setLinearState(id, opts.triggerStateName, opts.linear) : undefined, ), + // Linear has no blocked state and reverting to the trigger state would re-list the issue next + // poll, so the hold keeps the claimed state; the comment names the exact state to move back to. + // Holding is only safe while `claimedState` (a state NAME) does not resolve to the `triggerState` + // TYPE that listReady filters on — the same assumption the claim itself and onFailure rest on. + onSecretBlocked: (id) => + commentLinearIssue( + id, + holdMessage( + `It stays in "${opts.claimedState}" so the watch will not re-run it.`, + `move it back to ${opts.triggerStateName !== undefined ? `"${opts.triggerStateName}"` : 'its trigger state'}`, + ), + opts.linear, + ), onFailure: (id, error) => commentLinearIssue(id, formatFailureComment('Vanguard run failed', error), opts.linear), }; } @@ -390,12 +453,12 @@ async function runSpecCore( async function triageAgentRun( id: string, fetcher: { fetch: (id: string) => Promise }, - runner: (id: string) => Promise<{ prUrl?: string }>, + runner: (id: string) => Promise, action: { comment: (body: string) => Promise; toNeedsInfo: () => Promise; }, -): Promise<{ prUrl?: string; parked?: boolean }> { +): Promise { const task = await fetcher.fetch(id); if (assessTaskReadiness(task, 'agent') === 'needs_info') { await action.comment(clarifyMessage('agent')); @@ -476,7 +539,7 @@ async function runWatchLoop(primitives: WatchPrimitives, opts: LoopControls, log log, phase: 'watch', }); - log(`watch: ${tick.opened.length} PR(s), ${tick.noChange.length} no-change, ${tick.failed.length} failed, ${tick.skipped.length} skipped${deferredNote(tick)}.`); + log(`watch: ${tick.opened.length} PR(s), ${tick.noChange.length} no-change, ${tick.failed.length} failed, ${tick.skipped.length} skipped${deferredNote(tick)}${secretBlockedNote(tick)}.`); if (opts.once === true) return; await delay(intervalMs, opts.signal); } @@ -533,7 +596,7 @@ export async function runLoopV1( listReady: async () => agentReady, }; const agent = await watchOnce(agentThisTick, { ...concurrency, ...maxTasks, log, phase: 'watch' }); - log(`watch: ${agent.opened.length} PR(s), ${agent.noChange.length} no-change, ${agent.failed.length} failed, ${agent.skipped.length} skipped${deferredNote(agent)}.`); + log(`watch: ${agent.opened.length} PR(s), ${agent.noChange.length} no-change, ${agent.failed.length} failed, ${agent.skipped.length} skipped${deferredNote(agent)}${secretBlockedNote(agent)}.`); if (opts.once === true) return; await delay(intervalMs, opts.signal); } @@ -595,9 +658,24 @@ export function githubIssueWatchPrimitives(opts: WatchGithubOptions): WatchPrimi const fetcher = new GitHubTaskFetcher(repo, opts.gh); const needsInfoLabel = opts.needsInfoLabel; const agentLabels = opts.ownerLabel !== undefined ? [opts.ownerLabel, opts.label] : [opts.label]; + // Ready issues still carrying the hold label from an earlier secret block: a human re-triggered + // them, so the claim clears it too. Only for issues seen with the label — a blind `--remove-label` + // makes `gh issue edit` fail in a repo that never created it. + const heldBefore = new Set(); return { - listReady: async () => (await fetcher.list({ labels: agentLabels })).map((task) => ({ id: task.id })), - claim: (id) => editGithubLabels(repo, id, { remove: [opts.label], add: [opts.claimedLabel] }, opts.gh), + listReady: async () => { + const tasks = await fetcher.list({ labels: agentLabels }); + heldBefore.clear(); + for (const task of tasks) if (task.labels.includes(GITHUB_SECRET_BLOCKED_LABEL)) heldBefore.add(task.id); + return tasks.map((task) => ({ id: task.id })); + }, + claim: (id) => + editGithubLabels( + repo, + id, + { remove: heldBefore.has(id) ? [opts.label, GITHUB_SECRET_BLOCKED_LABEL] : [opts.label], add: [opts.claimedLabel] }, + opts.gh, + ), runOne: needsInfoLabel === undefined ? (id) => runGithubIssue(id, opts.deps) @@ -612,6 +690,24 @@ export function githubIssueWatchPrimitives(opts: WatchGithubOptions): WatchPrimi commentGithubIssue(repo, id, NO_CHANGE_MSG, opts.gh), editGithubLabels(repo, id, { remove: [opts.claimedLabel] }, opts.gh), ), + // One edit swaps claimed -> secret-blocked, so the claim is never released without the hold + // label landing. `gh issue edit --add-label` fails on a label the repo never created, and the + // adapter only pre-creates it when it posts findings (not in white-label mode), so ensure it + // here first (best-effort, the same pre-step as addPrFailureLabel). If the swap fails it throws + // into onFailure and the claimed marker stays. The trigger label was removed on claim and is + // not restored; the comment names the label to re-apply. Same in white-label mode: the claim + // label is already the watch's own marker, and a white-label adapter posts no findings, so this + // comment is the human's only signal. + onSecretBlocked: async (id) => { + await (opts.gh ?? defaultGhRunner)(['label', 'create', GITHUB_SECRET_BLOCKED_LABEL, '--repo', repo, '--force']).catch(() => undefined); + await editGithubLabels(repo, id, { remove: [opts.claimedLabel], add: [GITHUB_SECRET_BLOCKED_LABEL] }, opts.gh); + await commentGithubIssue( + repo, + id, + holdMessage(`Swapped \`${opts.claimedLabel}\` for \`${GITHUB_SECRET_BLOCKED_LABEL}\`.`, `re-apply the \`${opts.label}\` label`), + opts.gh, + ); + }, onFailure: (id, error) => commentGithubIssue(repo, id, formatFailureComment('Vanguard run failed', error), opts.gh), }; } @@ -740,6 +836,8 @@ export function githubProjectWatchPrimitives(opts: WatchGithubProjectOptions): W // Populated on each listReady call; claim/review look up the project item node ID here. const itemNodeIds = new Map(); + // Ready items still carrying the hold label from an earlier secret block; cleared on claim. + const heldBefore = new Set(); // Single cached promise: fetches project node ID and Status field in parallel, resolved once. let projectMeta: Promise | undefined; @@ -786,6 +884,7 @@ export function githubProjectWatchPrimitives(opts: WatchGithubProjectOptions): W }>; }; itemNodeIds.clear(); + heldBefore.clear(); const ready: Array<{ id: string }> = []; for (const item of parsed.items) { const content = item.content; @@ -794,14 +893,35 @@ export function githubProjectWatchPrimitives(opts: WatchGithubProjectOptions): W itemNodeIds.set(issueRef, item.id); if (item.status !== opts.triggerStatus) continue; if (opts.label !== undefined && !(content.labels ?? []).includes(opts.label)) continue; + if ((content.labels ?? []).includes(GITHUB_SECRET_BLOCKED_LABEL)) heldBefore.add(issueRef); ready.push({ id: issueRef }); } return ready; }, - claim: (id) => setStatus(id, opts.claimedStatus), + // A re-triggered item (moved back to the trigger status by a human) sheds the hold label from + // its earlier secret block; only for items listReady saw carrying it (see githubIssueWatchPrimitives). + // The cleanup is cosmetic and runs after the status move, so it must not throw: claimGate reads + // any throw as "already claimed" and the item would sit in the claimed status with no run and + // no comment. The edit goes to the issue's own repo — a board can hold issues from other repos. + claim: async (id) => { + await setStatus(id, opts.claimedStatus); + if (heldBefore.has(id)) { + await editGithubLabels(id.slice(0, id.indexOf('#')), id, { remove: [GITHUB_SECRET_BLOCKED_LABEL] }, gh).catch(() => undefined); + } + }, runOne: (id) => runGithubIssue(id, opts.deps), review: (id) => setStatus(id, opts.reviewStatus), onNoChange: (id) => commentAndRevert(commentGithubIssue(repo, id, NO_CHANGE_MSG, gh), setStatus(id, opts.triggerStatus)), + // The only configured statuses are trigger/claimed/review and reverting to the trigger status + // would re-list the item next poll, so the hold keeps the claimed status; the comment names the + // status to move back to. The adapter already added the secret-blocked label + masked findings. + onSecretBlocked: (id) => + commentGithubIssue( + repo, + id, + holdMessage(`It stays in "${opts.claimedStatus}" so the watch will not re-run it.`, `move it back to "${opts.triggerStatus}"`), + gh, + ), onFailure: (id, error) => commentGithubIssue(repo, id, formatFailureComment('Vanguard run failed', error), gh), }; } @@ -847,12 +967,25 @@ export function gitlabWatchPrimitives(opts: WatchGitlabOptions): WatchPrimitives const fetcher = new GitLabTaskFetcher(project, glab); const needsInfoLabel = opts.needsInfoLabel; const agentLabels = opts.ownerLabel !== undefined ? [opts.ownerLabel, opts.label] : [opts.label]; + // Ready issues still carrying the hold label from an earlier secret block (re-triggered by a + // human): the claim clears it too. See githubIssueWatchPrimitives. + const heldBefore = new Set(); return { - listReady: async () => (await fetcher.list({ labels: agentLabels })).map((task) => ({ id: task.id })), + listReady: async () => { + const tasks = await fetcher.list({ labels: agentLabels }); + heldBefore.clear(); + for (const task of tasks) if (task.labels.includes(GITLAB_SECRET_BLOCKED_LABEL)) heldBefore.add(task.id); + return tasks.map((task) => ({ id: task.id })); + }, claim: (id) => // Explicitly remove trigger label so future polls don't pick it up again. // GitLab scoped :: only auto-removes within the same scope prefix. - editGitlabLabels(project, id, { remove: [opts.label], add: [opts.claimedLabel] }, glab), + editGitlabLabels( + project, + id, + { remove: heldBefore.has(id) ? [opts.label, GITLAB_SECRET_BLOCKED_LABEL] : [opts.label], add: [opts.claimedLabel] }, + glab, + ), runOne: needsInfoLabel === undefined ? (id) => runGitlabIssue(id, opts.deps) @@ -869,6 +1002,23 @@ export function gitlabWatchPrimitives(opts: WatchGitlabOptions): WatchPrimitives await commentGitlabIssue(project, id, NO_CHANGE_MSG, glab); await editGitlabLabels(project, id, { remove: [opts.claimedLabel] }, glab); }, + // One edit swaps claimed -> secret-blocked so the claim is never released without the hold + // label landing. `glab issue update --label` is a silent no-op on a project lacking the label + // (the `--unlabel` in the same call still lands, releasing the claim with no hold), and the + // adapter only pre-creates it when it posts findings (not in white-label mode), so ensure it + // here first (best-effort, as addMrFailureLabel does). A failing swap throws into onFailure + // and the claimed marker stays. Trigger label not restored (removed on claim); the note names + // the label to re-apply. Same in white-label mode (see githubIssueWatchPrimitives). + onSecretBlocked: async (id) => { + await glab(['label', 'create', '--repo', project, '--name', GITLAB_SECRET_BLOCKED_LABEL]).catch(() => undefined); + await editGitlabLabels(project, id, { remove: [opts.claimedLabel], add: [GITLAB_SECRET_BLOCKED_LABEL] }, glab); + await commentGitlabIssue( + project, + id, + holdMessage(`Swapped \`${opts.claimedLabel}\` for \`${GITLAB_SECRET_BLOCKED_LABEL}\`.`, `re-apply the \`${opts.label}\` label`), + glab, + ); + }, onFailure: (id, error) => commentGitlabIssue(project, id, formatFailureComment('Vanguard run failed', error), glab), };