From 6e248916524712bda8427d3a788fa3adfa5a0716 Mon Sep 17 00:00:00 2001 From: Jim Meyer Date: Mon, 28 Sep 2026 18:56:54 -0700 Subject: [PATCH] feat(workflow): gate queued agent phases on four-axis labels Signed-off-by: Jim Meyer --- .agents/skills/build-from-issue/SKILL.md | 88 +++++++------- .agents/skills/create-spike/SKILL.md | 24 ++-- .agents/skills/fix-security-issue/SKILL.md | 34 +++--- .agents/skills/review-security-issue/SKILL.md | 18 +-- scripts/workflow_gate.py | 114 ++++++++++++++++++ scripts/workflow_gate_test.py | 90 ++++++++++++++ 6 files changed, 287 insertions(+), 81 deletions(-) create mode 100644 scripts/workflow_gate.py create mode 100644 scripts/workflow_gate_test.py diff --git a/.agents/skills/build-from-issue/SKILL.md b/.agents/skills/build-from-issue/SKILL.md index 4ec54e61a4..dfbad15cd5 100644 --- a/.agents/skills/build-from-issue/SKILL.md +++ b/.agents/skills/build-from-issue/SKILL.md @@ -1,6 +1,6 @@ --- name: build-from-issue -description: Given a GitHub issue number, plan and implement the work described in the issue. Supports direct user requests and unattended queue processing through the `agent:*` workflow labels. Includes tests, documentation updates, and PR creation. Trigger keywords - build from issue, implement issue, work on issue, build issue, start issue. +description: Given a GitHub issue number, plan and implement the work described in the issue. Supports direct user requests and unattended queue processing through the four-axis workflow labels. Includes tests, documentation updates, and PR creation. Trigger keywords - build from issue, implement issue, work on issue, build issue, start issue. metadata: internal: true --- @@ -20,16 +20,16 @@ This skill operates as a stateful workflow — it can be run repeatedly against This skill supports two invocation modes: -- **Direct mode:** A user explicitly asks the agent to plan or implement a specific issue. The request itself authorizes the requested phase; the corresponding `agent:*` request label is not required. -- **Queue mode:** An always-on or unattended agent scans for work without a live user directing it to a specific issue. In this mode, `agent:plan-requested` authorizes planning and `agent:implementation-requested` authorizes implementation. +- **Direct mode:** A user explicitly asks the agent to plan or implement a specific issue. The request itself authorizes the requested phase; queue labels are not required. +- **Queue mode:** An unattended agent scans for work without a live user directing it. Planning requires acceptance or roadmap placement with `needs:plan` and human-applied `ready-for:agent`. Implementation requires `needs:pr`, `ready-for:agent`, and an approved plan. Run `uv run --no-project python scripts/workflow_gate.py --issue --phase plan|implement` before either queued phase; stop if it denies the phase. A direct request authorizes only what it says. A request to review or plan does not authorize implementation. A request to build, implement, or work on an issue authorizes both the planning needed to perform the work and implementation unless the user asks to stop after planning. -The two request labels remain human-only queue controls. Under **no circumstances** should this skill or any agent apply them, ask to apply them, or suggest automating their application. +Acceptance, roadmap placement, and queue authorization remain human-only decisions. Under **no circumstances** should this skill apply `state:accepted`, `roadmap`, or `ready-for:agent` to authorize itself. -In direct mode, issue lifecycle and `agent:*` workflow labels are advisory rather than gates. Inspect the labels and warn the user about each expected label that is missing or any lifecycle label that indicates the normal workflow is incomplete, then continue with the requested phase. Do not ask the user to fix the labels first. A direct request does not change the issue's disposition or make the labels accurate; it only authorizes the requested work. +In direct mode, run the gate with `--direct` to report queue discrepancies, then warn the user about the missing or contradictory labels and continue with the requested phase. Do not ask the user to fix the labels first. A direct request does not change the issue's disposition or make the labels accurate; it only authorizes the requested work. The gate still blocks `topic:security` from this general build skill. -If direct work begins on an issue that was not already in the label-driven workflow, do not introduce `agent:in-progress` or `agent:pr-opened` solely for that invocation. If a matching request label is present, preserve the existing label transitions so unattended agents can track the workflow. +If direct work begins on an issue that was not already in the label-driven workflow, do not introduce workflow labels solely for that invocation. For queued work, transition `state:*`, `needs:*`, and `ready-for:*` as described below without adding acceptance or authorization labels. ## Agent Comment Markers @@ -63,16 +63,16 @@ Fetch issue + comments ├─ topic:security present? │ → Route to review-security-issue or fix-security-issue; STOP │ - ├─ Direct mode + expected lifecycle or agent-workflow labels missing/incomplete? + ├─ Direct mode + expected workflow labels missing/incomplete? │ → Warn which labels are missing or incomplete; continue with the requested phase │ ├─ Queue mode + triage incomplete, awaiting information, or awaiting human disposition? │ → Report the blocking state and STOP │ - ├─ No plan comment and no direct planning request and agent:plan-requested absent? + ├─ No plan comment and no direct planning request and queue planning gate denied? │ → No request for agent planning; STOP │ - ├─ No plan comment + direct planning request or agent:plan-requested present? + ├─ No plan comment + direct planning request or queue planning gate passed? │ → Generate plan via principal-engineer-reviewer │ → Post plan comment │ → Advance labels only for a label-driven invocation @@ -83,20 +83,18 @@ Fetch issue + comments │ → Update the plan comment if feedback requires plan changes │ → STOP │ - ├─ Plan exists + direct implementation request or 'agent:implementation-requested' label? + ├─ 'state:in-review' and 'needs:pr' labels present? + │ → Check for an existing PR; link to it and STOP if found + │ + ├─ 'state:in-progress' + 'needs:pr' present? + │ → Confirm queue authorization or direct request, then resume the existing branch + │ + ├─ Plan exists + direct implementation request or queue implementation gate passed? │ → Run scope check (warn if high complexity) │ → Check for conflicting branches/PRs │ → BUILD (Steps 6–14) │ - ├─ 'agent:in-progress' label present? - │ → Detect existing branch and resume if possible - │ → Otherwise report current state - │ - ├─ 'agent:pr-opened' label present? - │ → Report that PR already exists, link to it - │ → STOP - │ - └─ Plan exists + no new comments + neither a direct implementation request nor 'agent:implementation-requested'? + └─ Plan exists + no new comments + neither a direct implementation request nor queue authorization? → Report: "Plan is posted and awaiting review. No new comments to address." → STOP ``` @@ -113,15 +111,15 @@ If the issue is closed, report that and stop. If `topic:security` is present, stop. General build agents must not plan or implement security issues. Route planning/review to `review-security-issue` and authorized remediation to `fix-security-issue`. -In queue mode, stop before planning on `state:triage-needed` or `state:needs-info`, and stop on `state:validated` without roadmap placement. Require `state:accepted` or roadmap placement before queue work proceeds. If no plan exists, require `agent:plan-requested`; require `agent:implementation-requested` before queue-mode implementation. +In queue mode, run the gate for the intended phase. `state:new` with `ready-for:agent` authorizes screening only. `needs:info` or `ready-for:human` blocks unattended build work. A plan request never authorizes implementation. The human records approval by changing the issue from `needs:plan`, `ready-for:human` to `needs:pr`, `ready-for:agent` after reviewing the plan. In direct mode, inspect the same expected workflow state but do not stop because a lifecycle or agent-workflow label is absent or incomplete. Before continuing, warn the user with the specific discrepancy, for example: -> "Issue #42 is missing `state:accepted` or roadmap placement and `agent:implementation-requested`. Those labels are expected in the queued workflow, but your direct request authorizes implementation, so I am continuing without changing them." +> "Issue #42 is missing `state:accepted` or roadmap placement, `needs:pr`, and `ready-for:agent`. Those are expected for queued implementation, but your direct request authorizes this phase, so I am continuing without changing them." -If `state:triage-needed`, `state:needs-info`, or `state:validated` is present, name that state in the warning and explain what it normally means. Continue unless the issue lacks information that is actually necessary to perform the requested work; in that case, report the concrete missing information rather than treating the label itself as the blocker. +If `state:new`, `needs:info`, or `state:validated` is present, name it in the warning and explain what it normally means. Continue unless the issue lacks information actually needed to perform the requested work; report that concrete blocker rather than treating the label itself as the blocker. -Never add or remove `state:accepted`, either human request label, or the `roadmap` label. +Never add `state:accepted` or `ready-for:agent`, and never change roadmap placement. A human applies `ready-for:agent` to queue each phase. An agent may replace `state:accepted` with an active state after authorization and may clear `ready-for:agent` at handoff. ## Step 2: Fetch and Classify Comments @@ -146,7 +144,7 @@ Using the state machine above, determine what to do based on: 1. Whether a plan comment exists 2. Whether there are human comments newer than the last agent comment (plan or conversation) 3. Whether this is direct mode and which phase the user requested -4. Which lifecycle and agent-workflow labels are present (`state:*`, `agent:plan-requested`, `agent:plan-ready`, `agent:implementation-requested`, `agent:in-progress`, and `agent:pr-opened`) and which discrepancies require a direct-mode warning +4. Which `state:*`, `needs:*`, and `ready-for:*` labels are present; whether a human recorded acceptance or roadmap placement; and which discrepancies require a direct-mode warning Follow the appropriate branch below. @@ -154,7 +152,7 @@ Follow the appropriate branch below. ## Branch A: Generate the Plan -If no plan comment exists, generate one when the user directly requested planning or implementation, or when `agent:plan-requested` is present. Otherwise report that no one has requested agent planning and stop. +If no plan comment exists, generate one when the user directly requested planning or implementation, or when the queued planning gate passes. Otherwise report that no one has authorized agent planning and stop. ### A1: Analyze the Issue with Principal Engineer Reviewer @@ -228,13 +226,13 @@ EOF ### A3: Mark the Plan Ready in Queue Mode -If `agent:plan-requested` was present, replace it with `agent:plan-ready`. Do not add `agent:plan-ready` for a direct invocation that was not already using the label workflow. +If the queued planning gate passed, move the issue to `state:in-review`, keep `needs:plan`, and replace `ready-for:agent` with `ready-for:human`. Do not change workflow labels for an unlabeled direct invocation. ```bash -gh issue edit --remove-label "agent:plan-requested" --add-label "agent:plan-ready" +gh issue edit --remove-label "state:accepted" --remove-label "state:validated" --remove-label "state:in-progress" --remove-label "ready-for:agent" --add-label "state:in-review" --add-label "ready-for:human" ``` -If the direct request authorized implementation, continue to Branch C. Otherwise report that the plan has been posted and stop. In queue mode, a human reviews the plan and applies `agent:implementation-requested` before an unattended agent can build. +If the direct request authorized implementation, continue to Branch C. Otherwise report that the plan has been posted and stop. In queue mode, a human reviews the plan, replaces `needs:plan` with `needs:pr`, and applies `ready-for:agent` before an unattended agent can build. Only the human may perform that authorization transition. --- @@ -302,7 +300,7 @@ Report to the user what feedback was addressed and whether the plan was updated. ## Branch C: Build -Proceed with implementation when the plan exists and either the user directly requested implementation or `agent:implementation-requested` is present. An existing `agent:in-progress` or `agent:pr-opened` label still triggers the resume or existing-PR checks below. +Proceed with implementation when the plan exists and either the user directly requested implementation or the queue implementation gate passes. Existing `state:in-progress` or `state:in-review` with `needs:pr` still triggers the resume or existing-PR checks below. ### Step 4: Scope Check @@ -312,7 +310,7 @@ Read the plan comment and check the **Complexity** and **Confidence** fields. > "This issue is rated High complexity / Low confidence. The plan includes open questions that may need human decisions during implementation. Proceeding, but flagging this for your awareness." - Continue — do not hard-stop. The user directly requested implementation or chose to apply `agent:implementation-requested`. + Continue — do not hard-stop. The user directly requested implementation or a human queued it with `needs:pr` and `ready-for:agent`. ### Step 5: Conflict Detection @@ -359,10 +357,10 @@ git checkout -b -/$USERNAME ### Step 7: Mark Queue Work In Progress -If `agent:implementation-requested` is present, replace it and `agent:plan-ready` with `agent:in-progress`. In direct mode without a request label, do not add an agent-workflow label. +If the queued implementation gate passed, move to `state:in-progress`, keeping `needs:pr` and `ready-for:agent`. In direct mode without queue authorization, do not add workflow labels. ```bash -gh issue edit --remove-label "agent:implementation-requested" --remove-label "agent:plan-ready" --add-label "agent:in-progress" +gh issue edit --remove-label "state:accepted" --remove-label "state:in-review" --add-label "state:in-progress" ``` ### Step 8: Implement the Changes @@ -629,10 +627,10 @@ Include **every test** that ran (not just the new ones) so the reviewer can see #### Update labels -If `agent:in-progress` is present, replace it with `agent:pr-opened`. Do not add `agent:pr-opened` for an unlabeled direct invocation: +If queued implementation was authorized, move from `state:in-progress` to `state:in-review` and hand off the open PR to a human. Keep `needs:pr` until the PR is accepted. Do not add workflow labels for an unlabeled direct invocation: ```bash -gh issue edit --remove-label "agent:in-progress" --add-label "agent:pr-opened" +gh issue edit --remove-label "state:in-progress" --remove-label "ready-for:agent" --add-label "state:in-review" --add-label "ready-for:human" ``` #### Report workflow run URL @@ -650,7 +648,7 @@ Report the workflow run URL and suggest the user can use the `watch-github-actio ## Branch D: Resume In-Progress Build -If the `agent:in-progress` label is present, the skill was previously started but may not have completed. +If `state:in-progress`, `needs:pr`, and `ready-for:agent` are present, the skill may have been started but not completed. 1. Check for an existing branch matching the issue ID: ```bash @@ -658,7 +656,7 @@ If the `agent:in-progress` label is present, the skill was previously started bu ``` 2. If found, check it out and inspect the state (are there uncommitted changes? committed but not pushed? pushed but no PR?). 3. Resume from the appropriate step (9, 10, 12, or 13). -4. If the state is unrecoverable, report to the user and suggest starting fresh. Queue mode requires a human to reapply `agent:implementation-requested`; a new direct implementation request can resume without it. +4. If the state is unrecoverable, report to the user and suggest starting fresh. Queue mode requires a human to reauthorize `needs:pr` and `ready-for:agent`; a new direct implementation request can resume without queue labels. --- @@ -687,12 +685,12 @@ If the `agent:in-progress` label is present, the skill was previously started bu User says: "Plan issue #42" 1. Fetch issue #42 — title: "Add pagination to dataset list endpoint" -2. Notice that `state:accepted` and `agent:plan-requested` are absent; warn that the issue does not match the queued workflow, then continue because the user directly requested planning +2. Notice that acceptance, `needs:plan`, and `ready-for:agent` are absent; warn that the issue does not match the queued workflow, then continue because the user directly requested planning 3. Fetch comments — no `🏗️ build-plan` marker found 4. Pass issue to `principal-engineer-reviewer` for analysis 5. Reviewer produces a plan: feat type, Medium complexity, 3 implementation steps, unit + integration tests needed 6. Post the plan comment with the `🏗️ build-plan` marker -7. Because this direct invocation was unlabeled, leave the `agent:*` workflow labels unchanged +7. Because this direct invocation was not queued, leave the workflow labels unchanged 8. Report to user: "Plan posted on issue #42. Awaiting review." ### Second run — human left feedback @@ -719,34 +717,34 @@ User says: "Check issue #42" User says: "Build issue #42" -1. Fetch issue #42 — `state:accepted` is present but `agent:implementation-requested` is absent; warn about the missing queue label and continue because the user directly requested implementation +1. Fetch issue #42 — `state:accepted` is present but `needs:pr` and `ready-for:agent` are absent; warn about the missing queue labels and continue because the user directly requested implementation 2. Plan exists (Revision 2), complexity: Medium, confidence: High 3. No conflicting branches or PRs 4. Create branch `feat/42-add-pagination/jmyers` -5. Leave `agent:*` labels unchanged because this direct invocation was not picked up from the queue +5. Leave workflow labels unchanged because this direct invocation was not picked up from the queue 6. Implement pagination for both endpoints per the plan 7. Add unit tests for pagination logic, integration tests for both endpoints 8. `mise run pre-commit` passes on first attempt 9. E2E tests skipped (no changes under `e2e/`) 10. Commit, push, create PR with `Closes #42` 11. Post summary comment on issue with PR link -12. No agent-workflow label transition is needed +12. No issue workflow label transition is needed 13. Report PR URL and workflow run status to user ### Run directly on an issue outside the workflow state machine User says: "Build issue #42" -1. Fetch issue #42 — it has `state:triage-needed`; neither `state:accepted` nor `agent:implementation-requested` is present -2. Warn that triage and acceptance are incomplete and name the missing implementation request label +1. Fetch issue #42 — it has `state:new` and `ready-for:agent` for screening; neither acceptance nor `needs:pr` is present +2. Warn that screening and acceptance are incomplete and name the missing implementation need 3. Continue through planning and implementation because the user directly requested the work -4. Do not add, remove, or reinterpret lifecycle or agent-workflow labels +4. Do not add, remove, or reinterpret issue workflow labels ### Run on issue with existing PR User says: "Build issue #42" -1. Fetch issue #42 — `agent:pr-opened` label present +1. Fetch issue #42 — `state:in-review`, `needs:pr`, and `ready-for:human` are present 2. Find existing PR #789 linked to the issue 3. Report: "PR [#789](...) already exists for issue #42. Nothing to build." diff --git a/.agents/skills/create-spike/SKILL.md b/.agents/skills/create-spike/SKILL.md index 7e24cc8115..9418d75d5a 100644 --- a/.agents/skills/create-spike/SKILL.md +++ b/.agents/skills/create-spike/SKILL.md @@ -121,12 +121,13 @@ gh label list --limit 100 Based on the investigation results, select appropriate labels: -- **Do not add issue type labels** — GitHub built-in issue types come from issue templates or manual follow-up, not labels +- **Add one `type:*` label** when the investigation establishes bug, feature, chore, or spike work. GitHub's built-in issue type is separate metadata; Conventional Commit types do not determine this label. - **Include area labels** if they exist in the repo (e.g., `area:sandbox`, `area:proxy`, `area:policy`, `area:cli`) - **Do not invent labels** — only use labels that already exist in the repo - **Add `state:validated` only when the evidence is sufficient for human disposition** — the spike established a coherent problem or proposal and completed the factual assessment needed for a human yes/no decision -- **Add `state:needs-info` instead when material evidence is missing** — identify the exact evidence, reproduction details, or decision input still needed in the issue body -- **Never add `state:accepted`, an `agent:*` label, or the `roadmap` label** — acceptance, roadmap placement, and requests for agent work require a human decision +- **Use `state:new` with `needs:info` when material evidence is missing** — identify the exact evidence, reproduction details, or decision input still needed in the issue body +- **Add `ready-for:human`** for disposition or an information request. Do not queue an agent for further work on the new issue. +- **Never add `state:accepted`, `ready-for:agent`, or the `roadmap` label** — acceptance, roadmap placement, and agent queue authorization require a human decision ## Step 4: Create the GitHub Issue @@ -135,7 +136,8 @@ Create the issue with a structured body containing both the stakeholder-readable ```bash gh issue create \ --title ": " \ - --label "" --label "" \ + --label "" --label "" \ + --label "" --label "ready-for:human" \ --body "$(cat <<'EOF' ## Problem Statement @@ -201,7 +203,7 @@ gh issue create \ ## Disposition Readiness -- **State:** `` +- **State:** `` - **Assessment:** - **Missing evidence:** @@ -213,11 +215,13 @@ gh issue create \ - --- -*Created by spike investigation. `state:validated` means the issue is ready for human disposition; `state:needs-info` means specific evidence is still required. A human applies `state:accepted` or places the issue on the roadmap if OpenShell should pursue the work. To queue unattended agent planning, a human applies `agent:plan-requested`; on a direct request, the agent warns about missing expected workflow labels and continues without changing them.* +*Created by spike investigation. `state:validated` means the issue is ready for human disposition; `state:new` with `needs:info` means specific evidence is still required. A human applies `state:accepted` or places the issue on the roadmap if OpenShell should pursue the work. After acceptance, a human may queue planning with `needs:plan` and `ready-for:agent`; a direct request authorizes only its stated phase without changing queue labels.* EOF )" ``` +Add `--label needs:info` when the chosen state is `state:new` because evidence is missing. + **Do NOT post a follow-up comment on the issue.** All findings must be contained in the issue body itself. **Display the issue URL** so it's easily clickable: @@ -237,11 +241,11 @@ After creating the issue, report: For `state:validated`: -> Review the issue and decide whether OpenShell should pursue it. If yes, apply `state:accepted`, associate it with a roadmap item, or do both. Either action records acceptance; roadmap placement additionally records sequencing. The work may remain human-owned. Apply `agent:plan-requested` to queue planning for an unattended agent, or directly ask an agent to use `build-from-issue`; on a direct request, the agent warns about missing expected workflow labels and continues without changing them. If no, close it as not planned and record the rationale. +> Review the issue and decide whether OpenShell should pursue it. If yes, apply `state:accepted`, associate it with a roadmap item, or do both. Either action records acceptance; roadmap placement additionally records sequencing. The work may remain human-owned. Apply `needs:plan` and `ready-for:agent` to queue planning for an unattended agent, or directly ask an agent to use `build-from-issue`; on a direct request, the agent warns about missing expected workflow labels and continues without changing them. If no, close it as not planned and record the rationale. -For `state:needs-info`: +For `state:new` with `needs:info`: -> Collect the missing evidence identified in the issue. Leave it off the roadmap. Once the evidence is sufficient, replace `state:needs-info` with `state:validated` for human disposition. +> Collect the missing evidence identified in the issue. Leave it off the roadmap. Once the evidence is sufficient, replace `state:new` with `state:validated` and clear `needs:info` for human disposition. ## Design Principles @@ -255,7 +259,7 @@ For `state:needs-info`: 5. **Cross-reference `build-from-issue`.** Mention it as the natural next step in the issue body footer. -6. **Treat validation as an evidence threshold, not an automatic spike outcome.** Apply `state:validated` only when the investigation supports a human accept/decline decision. Otherwise apply `state:needs-info`, state what is missing, and leave the issue off the roadmap. +6. **Treat validation as an evidence threshold, not an automatic spike outcome.** Apply `state:validated` only when the investigation supports a human accept/decline decision. Otherwise apply `state:new` with `needs:info`, state what is missing, and leave the issue off the roadmap. ## Useful Commands Reference diff --git a/.agents/skills/fix-security-issue/SKILL.md b/.agents/skills/fix-security-issue/SKILL.md index 1452cbe66c..afd47c318a 100644 --- a/.agents/skills/fix-security-issue/SKILL.md +++ b/.agents/skills/fix-security-issue/SKILL.md @@ -1,6 +1,6 @@ --- name: fix-security-issue -description: Implement a fix for a reviewed security issue. Takes a directly requested issue number or scans for issues labeled `topic:security` and `agent:implementation-requested`. Reads the security review from the issue comments and implements the remediation plan. Trigger keywords - fix security issue, remediate security, implement security fix, patch vulnerability. +description: Implement a fix for a reviewed security issue. Takes a directly requested issue number or scans for issues labeled `topic:security`, `needs:pr`, and `ready-for:agent`. Reads the security review and implements the approved remediation plan. Trigger keywords - fix security issue, remediate security, implement security fix, patch vulnerability. metadata: internal: true --- @@ -13,7 +13,7 @@ Implement a code fix for a security issue that has already been reviewed by the - The `gh` CLI must be authenticated (`gh auth status`) - You must be in a git repository with a GitHub remote -- The issue must have `topic:security`. In unattended scan mode it must also have `agent:implementation-requested`; for a direct user request, warn if that workflow label is missing and continue without changing it. +- The issue must have `topic:security`. In unattended scan mode it must also have human-applied `needs:pr` and `ready-for:agent` after plan approval; for a direct user request, warn if that queue tuple is missing and continue without changing it. - The issue must have a prior security review comment (posted by `review-security-issue`) with a **Legitimate concern** determination and a remediation plan ## Agent Comment Marker @@ -32,14 +32,14 @@ The user may provide an issue number directly, or ask the agent to find issues t ### If an issue number is provided -Strip any leading `#` and proceed to Step 2 with that issue ID. The user's explicit fix request authorizes implementation. If `agent:implementation-requested` is absent, warn that the expected workflow label is missing and continue without changing it. +Strip any leading `#` and proceed to Step 2 with that issue ID. The user's explicit fix request authorizes implementation. If `needs:pr` or `ready-for:agent` is absent, warn about the expected queue labels and continue without changing them. ### If no issue number is provided -Scan for open issues labeled `topic:security` and `agent:implementation-requested`: +Scan for open issues labeled `topic:security`, `needs:pr`, and `ready-for:agent`: ```bash -gh issue list --label "topic:security" --label "agent:implementation-requested" --state open --json number,title,labels,updatedAt +gh issue list --label "topic:security" --label "needs:pr" --label "ready-for:agent" --state open --json number,title,labels,updatedAt ``` - **If no issues are found**, report to the user that there are no security issues ready for fixing and stop. @@ -59,11 +59,11 @@ gh issue view --json number,title,body,state,labels,author Check the issue's `labels` array from the response above: - `topic:security` is required because this specialized skill handles security issues. -- `agent:implementation-requested` is required only when an unattended agent discovered the issue by scanning the queue. +- `needs:pr` and `ready-for:agent` are required only when an unattended agent discovered the issue by scanning the queue. Also require prior human acceptance or roadmap placement and an approved remediation plan. -If `topic:security` is missing, report that this skill only handles security issues and stop. If queue mode selected an issue without `agent:implementation-requested`, report that it is not ready for unattended pickup and stop. +If `topic:security` is missing, report that this skill only handles security issues and stop. In queue mode run `uv run --no-project python scripts/workflow_gate.py --issue --phase implement --security`; report its reasons and stop if denied. For a direct request, add `--direct`, warn about queue discrepancies, and continue only if the specialized gate confirms a legitimate review and remediation plan. -Never apply `agent:implementation-requested` yourself. In direct mode, warn about its absence and continue; the missing label does not block the user's request to fix the specific issue. +Never apply `state:accepted` or `ready-for:agent` yourself. In direct mode, warn about missing queue authorization and continue; those missing labels do not block the user's explicit fix request. ### Validate the security review @@ -98,10 +98,10 @@ git checkout -b fix/security-- Follow the project's branch naming conventions. The branch name should reference the issue ID. -In queue mode, replace the human request and ready-plan labels with the agent execution state. For an unlabeled direct invocation, do not add an agent-workflow label: +In queue mode, move the accepted issue to the active state while keeping `needs:pr` and `ready-for:agent`. For an unqueued direct invocation, do not add workflow labels: ```bash -gh issue edit --remove-label "agent:implementation-requested" --remove-label "agent:plan-ready" --add-label "agent:in-progress" +gh issue edit --remove-label "state:accepted" --remove-label "state:in-review" --add-label "state:in-progress" ``` ## Step 5: Implement the Fix @@ -236,10 +236,10 @@ EOF Created PR [#](https://github.com/OWNER/REPO/pull/) ``` -In queue mode, replace `agent:in-progress` with `agent:pr-opened` after the PR is created. For an unlabeled direct invocation, do not add an agent-workflow label: +In queue mode, hand the open PR to a human after creation. Keep `needs:pr` until the PR is accepted. For an unqueued direct invocation, do not add workflow labels: ```bash -gh issue edit --remove-label "agent:in-progress" --add-label "agent:pr-opened" +gh issue edit --remove-label "state:in-progress" --remove-label "ready-for:agent" --add-label "state:in-review" --add-label "ready-for:human" ``` ## Step 9: Report to User @@ -257,7 +257,7 @@ Summarize what was done: | Command | Description | | --- | --- | -| `gh issue list --label "topic:security" --label "agent:implementation-requested" --state open` | Find security issues whose fixes a human requested | +| `gh issue list --label "topic:security" --label "needs:pr" --label "ready-for:agent" --state open` | Find security fixes a human queued | | `gh issue view --json number,title,body,state,labels,author` | Fetch full issue metadata | | `gh issue view --json comments` | Fetch all comments on an issue | | `gh pr create --title "..." --body "..."` | Create a pull request | @@ -285,7 +285,7 @@ User says: "Fix security issue #42" User says: "Fix any ready security issues" -1. Query for open issues with labels `topic:security` + `agent:implementation-requested` +1. Query for open issues with labels `topic:security`, `needs:pr`, and `ready-for:agent` 2. Find issue #78: "SQL injection in search endpoint" 3. Fetch the review comment -- determination is "Legitimate concern" 4. Implement parameterized queries @@ -302,14 +302,14 @@ User says: "Fix security issue #99" 3. Report to the user: "Issue #99 was reviewed and determined to be not actionable. No fix is needed." 4. Stop -### Directly requested issue without `agent:implementation-requested` +### Directly requested issue without queue labels User says: "Fix security issue #55" 1. Fetch issue #55 metadata -2. Labels are `["topic:security"]` -- missing `agent:implementation-requested` +2. Labels are `["topic:security"]` -- missing `needs:pr` and `ready-for:agent` 3. Confirm that a legitimate security review and remediation plan exist -4. Warn that `agent:implementation-requested` is missing from the expected workflow state +4. Warn that the implementation queue tuple is missing from the expected workflow state 5. Proceed because the user's direct request authorizes implementation; leave the labels unchanged ### Issue without a review diff --git a/.agents/skills/review-security-issue/SKILL.md b/.agents/skills/review-security-issue/SKILL.md index efb054df80..b9849fb180 100644 --- a/.agents/skills/review-security-issue/SKILL.md +++ b/.agents/skills/review-security-issue/SKILL.md @@ -13,7 +13,7 @@ Review an issue that outlines a security, vulnerability, or privacy concern. - The `gh` CLI must be authenticated (`gh auth status`) - You must be in a git repository with a GitHub remote -- The issue must have `topic:security`. In unattended queue mode it must also have `agent:plan-requested`; for a direct user request, warn if that workflow label is missing and continue without changing it. +- The issue must have `topic:security`. In unattended queue mode it must also have human-applied `needs:plan` and `ready-for:agent`; for a direct user request, warn if the queue tuple is missing and continue without changing it. Never publish exploit details in a public issue; follow `SECURITY.md` for vulnerability reports. ## Agent Comment Marker @@ -43,10 +43,10 @@ gh issue view --json title,body,state,labels,author First, check the issue's labels from the metadata fetched in Step 1. -- **If the issue has `agent:implementation-requested`**, the issue has already been reviewed and a human authorized remediation. There is no review to perform. Suggest using `fix-security-issue` and stop. +- **If the issue has `needs:pr` and `ready-for:agent` with an approved security review**, a human authorized remediation. Suggest using `fix-security-issue` and stop. - **If `topic:security` is missing**, report that this specialized skill only reviews security issues and stop. -- **If this is queue mode and `agent:plan-requested` is missing**, report that the issue is not ready for unattended pickup and stop. -- **If the user directly requested review of this issue**, warn that `agent:plan-requested` is missing, then proceed without it. Never add or offer to add the human-only request label. +- **If this is queue mode**, run `uv run --no-project python scripts/workflow_gate.py --issue --phase plan --security`; report its reasons and stop if it denies the phase. `ready-for:agent` at `state:new` authorizes intake screening only, never a security review. +- **If the user directly requested review of this issue**, run the gate with `--direct`, warn about its queue discrepancies, then proceed. Never add or offer to add human-only authorization labels. Next, fetch existing comments on the issue: @@ -141,13 +141,13 @@ EOF ## Step 5: Mark the Security Plan Ready -After posting a legitimate-concern review with a remediation plan, replace `agent:plan-requested` with `agent:plan-ready` only when the request label was present: +After posting a legitimate-concern review with a remediation plan, hand queued work to a human. Keep `needs:plan` until the human approves the remediation plan: ```bash -gh issue edit --remove-label "agent:plan-requested" --add-label "agent:plan-ready" +gh issue edit --remove-label "state:accepted" --remove-label "state:validated" --remove-label "ready-for:agent" --add-label "state:in-review" --add-label "ready-for:human" ``` -This signals that an unattended agent produced a remediation plan that awaits human review. For an unlabeled direct invocation, leave the `agent:*` labels unchanged. A later direct request can authorize remediation without `agent:implementation-requested`; warn that the expected label is missing and continue, while unattended remediation still requires that label. For a not-actionable determination, remove `agent:plan-requested` if present, do not add another `agent:*` label, and report that a human should close the issue or record the risk decision. +This signals that a remediation plan awaits human review. For an unqueued direct invocation, leave workflow labels unchanged. A later direct request can authorize remediation without queue labels; warn about the discrepancy and continue, while unattended remediation requires a human to apply `needs:pr` and `ready-for:agent` after approval. For a not-actionable determination, clear `ready-for:agent` only if this was queued, set `ready-for:human`, and report that a human should close the issue or record the risk decision. ## Step 6: Address Follow-up Comments @@ -169,7 +169,7 @@ For each unanswered human comment: | `gh issue view --json title,body,state,labels,author` | Fetch full issue metadata as JSON | | `gh issue view --json comments --jq '.comments[].body'` | Fetch all comments on an issue | | `gh issue comment --body "..."` | Post a comment on an issue | -| `gh issue edit --remove-label "agent:plan-requested" --add-label "agent:plan-ready"` | Mark a remediation plan ready for human review | +| `gh issue edit --remove-label "ready-for:agent" --add-label "ready-for:human"` | Hand a remediation plan to a human for review | ## Example Usage @@ -182,7 +182,7 @@ User says: "Review security issue #42" 3. No prior review found -- pass issue to `principal-engineer-reviewer` with security lens 4. Reviewer determines it's a legitimate XSS vulnerability in the API response handler 5. Post a comment with severity assessment and remediation plan -6. If `agent:plan-requested` was present, replace it with `agent:plan-ready`; otherwise leave the direct invocation unlabeled +6. If queued with `needs:plan` and `ready-for:agent`, hand it to `ready-for:human`; otherwise leave the direct invocation's labels unchanged 7. Report the finding and posted comment to the user ### Re-review with new comments diff --git a/scripts/workflow_gate.py b/scripts/workflow_gate.py new file mode 100644 index 0000000000..c332070e32 --- /dev/null +++ b/scripts/workflow_gate.py @@ -0,0 +1,114 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Check issue labels before queued planning or implementation.""" + +import argparse +import json +import subprocess +from dataclasses import dataclass + +AXES = ("type:", "state:", "needs:", "ready-for:") +ACCEPTED_STATES = {"state:accepted", "state:in-progress", "state:in-review"} + + +@dataclass(frozen=True) +class GateResult: + allowed: bool + problems: tuple[str, ...] + + +def assess( + labels: set[str], + phase: str, + *, + has_plan: bool = False, + direct: bool = False, + specialized: bool = False, +) -> GateResult: + if phase not in {"plan", "implement"}: + raise ValueError("phase must be plan or implement") + if "topic:security" in labels and not specialized: + return GateResult( + False, ("topic:security requires a specialized security skill",) + ) + if specialized and "topic:security" not in labels: + return GateResult(False, ("specialized security work requires topic:security",)) + + problems = [] + for axis in AXES: + found = sorted(label for label in labels if label.startswith(axis)) + if len(found) > 1: + problems.append(f"conflicting {axis} labels: {', '.join(found)}") + if axis == "state:" and not found: + problems.append("missing state:* label") + + accepted = bool(labels & ACCEPTED_STATES or "roadmap" in labels) + if not accepted and (not specialized or phase == "implement"): + problems.append("missing state:accepted or roadmap placement") + expected_need = "needs:plan" if phase == "plan" else "needs:pr" + if expected_need not in labels: + problems.append(f"missing {expected_need}") + if "ready-for:agent" not in labels: + problems.append("missing ready-for:agent") + if phase == "implement" and not has_plan: + problems.append("approved implementation plan is not present") + if "state:new" in labels: + problems.append("state:new permits screening only") + + if specialized and phase == "implement" and not has_plan: + return GateResult(False, tuple(problems)) + return GateResult(direct or not problems, tuple(problems)) + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--issue", type=int, required=True) + parser.add_argument("--phase", choices=("plan", "implement"), required=True) + parser.add_argument( + "--direct", + action="store_true", + help="Report queue discrepancies for a direct request", + ) + parser.add_argument( + "--security", action="store_true", help="Apply the specialized security gate" + ) + args = parser.parse_args() + + result = subprocess.run( + ["gh", "issue", "view", str(args.issue), "--json", "state,labels,comments"], + check=True, + capture_output=True, + text=True, + ) + issue = json.loads(result.stdout) + labels = {label["name"] for label in issue["labels"]} + marker = "> **🔒 security-review-agent**" if args.security else "> **🏗️ build-plan**" + has_plan = any( + comment["body"].startswith(marker) + and ( + not args.security + or ( + "**Determination:** Legitimate concern" in comment["body"] + and "### Remediation Plan" in comment["body"] + ) + ) + for comment in issue["comments"] + ) + gate = assess( + labels, + args.phase, + has_plan=has_plan, + direct=args.direct, + specialized=args.security, + ) + if issue["state"] != "OPEN": + gate = GateResult(False, ("issue is closed",)) + print(json.dumps({"allowed": gate.allowed, "problems": gate.problems})) + if not gate.allowed: + raise SystemExit(2) + + +if __name__ == "__main__": + main() diff --git a/scripts/workflow_gate_test.py b/scripts/workflow_gate_test.py new file mode 100644 index 0000000000..827583aa45 --- /dev/null +++ b/scripts/workflow_gate_test.py @@ -0,0 +1,90 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Queued work must have the human-authorized workflow tuple.""" + +import unittest + +import workflow_gate + + +class WorkflowGateTest(unittest.TestCase): + def test_intake_eligibility_does_not_authorize_build_work(self): + labels = {"state:new", "ready-for:agent"} + self.assertFalse(workflow_gate.assess(labels, "plan").allowed) + self.assertFalse( + workflow_gate.assess(labels, "implement", has_plan=True).allowed + ) + + def test_plan_queue_requires_acceptance_need_and_actor(self): + labels = {"state:accepted", "needs:plan", "ready-for:agent"} + self.assertTrue(workflow_gate.assess(labels, "plan").allowed) + self.assertFalse( + workflow_gate.assess(labels, "implement", has_plan=True).allowed + ) + self.assertFalse( + workflow_gate.assess(labels - {"ready-for:agent"}, "plan").allowed + ) + self.assertFalse( + workflow_gate.assess( + {"roadmap", "needs:plan", "ready-for:agent"}, "plan" + ).allowed + ) + + def test_roadmap_label_can_record_acceptance(self): + labels = {"roadmap", "state:validated", "needs:plan", "ready-for:agent"} + self.assertTrue(workflow_gate.assess(labels, "plan").allowed) + + def test_implementation_requires_approved_plan_and_tuple(self): + labels = {"state:in-progress", "needs:pr", "ready-for:agent"} + self.assertFalse(workflow_gate.assess(labels, "implement").allowed) + self.assertTrue( + workflow_gate.assess(labels, "implement", has_plan=True).allowed + ) + self.assertFalse( + workflow_gate.assess( + labels | {"needs:plan"}, "implement", has_plan=True + ).allowed + ) + + def test_direct_request_reports_discrepancies_but_proceeds(self): + result = workflow_gate.assess({"state:validated"}, "implement", direct=True) + self.assertTrue(result.allowed) + self.assertIn("needs:pr", " ".join(result.problems)) + + def test_security_never_enters_general_build_skill(self): + labels = {"topic:security", "state:accepted", "needs:pr", "ready-for:agent"} + self.assertFalse( + workflow_gate.assess( + labels, "implement", has_plan=True, direct=True + ).allowed + ) + + def test_specialized_review_does_not_authorize_remediation(self): + labels = {"topic:security", "state:validated", "needs:plan", "ready-for:agent"} + self.assertTrue(workflow_gate.assess(labels, "plan", specialized=True).allowed) + self.assertFalse( + workflow_gate.assess( + labels, "implement", has_plan=True, specialized=True + ).allowed + ) + + def test_specialized_remediation_needs_human_approval_and_review(self): + labels = {"topic:security", "state:accepted", "needs:pr", "ready-for:agent"} + self.assertFalse( + workflow_gate.assess(labels, "implement", specialized=True).allowed + ) + self.assertTrue( + workflow_gate.assess( + labels, "implement", has_plan=True, specialized=True + ).allowed + ) + self.assertFalse( + workflow_gate.assess( + {"topic:security"}, "implement", specialized=True, direct=True + ).allowed + ) + + +if __name__ == "__main__": + unittest.main()