diff --git a/.agents/skills/build-from-issue/SKILL.md b/.agents/skills/build-from-issue/SKILL.md index 4ec54e61a4..bdcf02e8a2 100644 --- a/.agents/skills/build-from-issue/SKILL.md +++ b/.agents/skills/build-from-issue/SKILL.md @@ -1,760 +1,34 @@ --- 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: Plan and implement work described in a GitHub issue, including verification, documentation, and a PR that closes the issue. metadata: internal: true --- # Build From Issue -Plan, iterate on feedback, and implement work described in a GitHub issue. +Use a specific GitHub issue to plan and implement a scoped change. Direct user instructions authorize the phase requested. Planning alone does not authorize implementation. For unattended work, inspect the current `state:*` label descriptions, maintainer assignments, and issue comments to infer the authorized phase. Proceed only when those records authorize the phase; do not assume every state permits implementation. -This skill operates as a stateful workflow β€” it can be run repeatedly against the same issue. Each invocation inspects the issue's labels, plan comment, and conversation history to determine the correct next action. +## Inspect the issue -## Prerequisites +1. Run `gh issue view --json number,title,body,state,labels,comments,assignees` and inspect the repository's current `state:*` labels and descriptions with `gh label list`. Infer whether triage, validation, and human acceptance have happened. Do not hard-code label names or change disposition as part of building. +2. Read the issue, comments, linked PRs, and current code. Check for an active owner or implementation. If the issue concerns a vulnerability, use `review-security-issue` and `fix-security-issue` instead. +3. Confirm that the User Story attests to the human operator's first-hand OpenShell use and gives a specific use case. If the issue lacks this, ask the operator before proceeding with planning or implementation. For a bug, require reproduction using only an OpenShell deployment; do not install third-party tools solely to demonstrate the problem. +4. If the user's direct request starts before the normal issue disposition, briefly report the discrepancy and continue with the authorized phase. Stop only when information needed to do the work is actually unavailable or a conflicting owner needs resolution. -- The `gh` CLI must be authenticated (`gh auth status`) -- You must be in a git repository with a GitHub remote +## Plan -## Invocation and Authorization +Identify the user-visible outcome, affected code, alternatives, tests, and documentation. For configuration, CLI, SDK, or other UX changes, include notional commands, configuration, or API examples so a human can review the proposed interaction. Consider existing extensibility points such as middleware, interceptors, and providers. Prefer an applicable extension when it satisfies the use case; the need to run another service alone does not disqualify it. -This skill supports two invocation modes: +Use a single issue comment beginning with `> **πŸ—οΈ build-plan**` when a plan should be recorded on GitHub. On later invocations, read that comment and any newer human feedback before taking action. Respond to unanswered feedback with a comment beginning `> **πŸ—οΈ build-from-issue-agent**`; update the existing plan comment in place when the design changes. Do not repeat a completed plan or open a second PR. Distinguish technical findings from product decisions. If the user requested only a plan, stop after the plan is available for review. -- **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. +## Implement -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. +1. Check the current branch and working tree. Preserve unrelated work. Create a branch or worktree as needed, with the branch named `/-/`. Use a Conventional Commits type for ``. +2. Implement the smallest coherent change that fulfills the acceptance criteria. Update relevant skills when behavior or commands change. Keep published documentation minimal: explain exactly what users need, avoid duplication across pages, and omit internal details with no user impact. +3. Add meaningful tests for changed behavior and follow the verification guidance in `CONTRIBUTING.md`. Select format, lint, compile or type checks, and tests for affected components and their dependencies. Run the relevant E2E lane for infrastructure, sandbox, or policy changes. Guidance and template edits need applicable Markdown, YAML, link, and consistency checks. Do not require full Rust, SDK, or repository CI solely because a commit or PR is being created; broaden checks only for a concrete remaining risk or failed check. +4. Review the diff, use a signed-off Conventional Commit, and prepare a PR following `create-github-pr`. -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. +Every PR must have its own existing issue and use `Closes #` in its Related Issue section. For work needing multiple PRs, split the scope into a closable issue per PR. A high-level issue may track those issues but should not be closed by an incomplete PR. Report the implementation, verification, and any remaining limitation in the PR description, rather than copying earlier issue diagnostics. -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. - -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. - -## Agent Comment Markers - -This skill uses two distinct markers to identify its comments: - -### Plan marker - -The implementation plan lives in a **single comment** that is edited in place as the plan evolves. It is identified by this marker on its first line: - -``` -> **πŸ—οΈ build-plan** -``` - -### Conversation marker - -All other comments (responses to human feedback, status updates, PR announcements) use this marker: - -``` -> **πŸ—οΈ build-from-issue-agent** -``` - -These markers distinguish agent comments from human comments and from other skills (e.g., `πŸ”’ security-review-agent`, `πŸ”§ security-fix-agent`). - -## State Machine Overview - -Each invocation follows this decision tree: - -``` -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? - β”‚ β†’ 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 request for agent planning; STOP - β”‚ - β”œβ”€ No plan comment + direct planning request or agent:plan-requested present? - β”‚ β†’ Generate plan via principal-engineer-reviewer - β”‚ β†’ Post plan comment - β”‚ β†’ Advance labels only for a label-driven invocation - β”‚ β†’ Continue if the direct request also authorized implementation; otherwise STOP - β”‚ - β”œβ”€ Plan exists + new human comments since last agent response? - β”‚ β†’ Respond to each comment (quote context, address feedback) - β”‚ β†’ Update the plan comment if feedback requires plan changes - β”‚ β†’ STOP - β”‚ - β”œβ”€ Plan exists + direct implementation request or 'agent:implementation-requested' label? - β”‚ β†’ 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'? - β†’ Report: "Plan is posted and awaiting review. No new comments to address." - β†’ STOP -``` - -## Step 1: Fetch the Issue - -The user provides an issue ID (e.g., `#42` or `42`). Strip any leading `#` and fetch: - -```bash -gh issue view --json number,title,body,state,labels,author -``` - -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 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." - -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. - -Never add or remove `state:accepted`, either human request label, or the `roadmap` label. - -## Step 2: Fetch and Classify Comments - -Fetch all comments: - -```bash -gh issue view --json comments --jq '.comments[] | {id: .id, body: .body, author: .author.login, createdAt: .createdAt, updatedAt: .updatedAt}' -``` - -Classify each comment into one of: - -- **Plan comment**: body starts with `> **πŸ—οΈ build-plan**` -- **Agent comment**: body starts with `> **πŸ—οΈ build-from-issue-agent**` -- **Human comment**: everything else (not agent-marked) - -Record the plan comment's `id` (needed for editing via API) and its `updatedAt` timestamp. - -## Step 3: Determine Action - -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 - -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. - -### A1: Analyze the Issue with Principal Engineer Reviewer - -Pass the issue title, description, labels, and any relevant code references to the `principal-engineer-reviewer` sub-agent. Use the Task tool: - -``` -Task tool with subagent_type="principal-engineer-reviewer" -``` - -In the prompt, instruct the reviewer to: - -1. Read the issue's user story and identify what needs to change in the codebase. Treat reporter diagnostics or solution ideas as optional context, not as authoritative or current analysis. -2. Map the requirements to existing code β€” read the relevant source files. -3. Determine the **issue type** β€” one of: `feat` (new feature), `fix` (bug fix), `refactor`, `chore`, `perf`, `docs`. -4. Propose the minimal set of changes that satisfies the requirements. -5. Sequence the work so each step is independently testable. -6. Identify what tests are needed (unit, integration, e2e) and where they should live. -7. Assess **complexity** on a scale: - - **Low**: Isolated change, < 3 files, clear path forward - - **Medium**: Multiple files/components, some design decisions, but well-scoped - - **High**: Cross-cutting changes, architectural decisions needed, significant unknowns -8. Call out risks, unknowns, and decisions that need stakeholder input. -9. Assess **gateway config documentation impact** β€” if the change adds, removes, renames, or changes defaults for gateway TOML keys or driver-specific config options, the plan must include an update to `docs/how-it-works/gateways/configuration.mdx`. If the change is surfaced through Helm or a compute-driver overview, also include `docs/how-it-works/sandboxes/runtimes.mdx` or the relevant deployment docs. -10. Assess **LSM compatibility** β€” if the change touches process identity, `/proc` filesystem access, binary execution, or inter-process visibility, flag whether it will behave differently on hosts running SELinux (enforcing) or AppArmor. In particular, tests that fork+exec into system binaries will fail on SELinux-enforcing hosts due to cross-label `/proc//exe` access restrictions. - -Perform this investigation against the current branch and current product behavior. If the issue contains earlier diagnostics, verify them rather than relying on them. - -### A2: Post the Plan Comment - -Post the plan as a comment on the issue. This is the **canonical plan comment** that will be edited in place as the plan evolves. - -```bash -gh issue comment --body "$(cat <<'EOF' -> **πŸ—οΈ build-plan** - -## Implementation Plan - -**Issue type:** `` -**Complexity:** -**Confidence:** - -### Summary -<2-3 sentences describing what will be built/changed and the approach> - -### Scope -- ``: -- ``: -- ... - -### Implementation Steps -1. -2. -3. ... - -### Test Plan -- **Unit tests:** -- **Integration tests:** -- **E2E tests:** - -### Risks & Open Questions -- - -### Documentation Impact -- - ---- -*Revision 1 β€” initial plan* -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. - -```bash -gh issue edit --remove-label "agent:plan-requested" --add-label "agent:plan-ready" -``` - -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. - ---- - -## Branch B: Respond to Feedback - -If a plan exists and there are human comments newer than the last agent response, address them. - -### B1: Process Each Unanswered Human Comment - -For each human comment that is newer than the most recent agent comment (plan `updatedAt` or conversation comment `createdAt`): - -1. Read the comment. -2. Quote the relevant portion using `>` blockquote syntax. -3. Formulate a response based on the codebase and the current plan. -4. Post a response with the conversation marker. - -```bash -gh issue comment --body "$(cat <<'EOF' -> **πŸ—οΈ build-from-issue-agent** - -> - - -EOF -)" -``` - -### B2: Update the Plan if Needed - -If any feedback requires changes to the plan, **edit the existing plan comment** rather than posting a new one. Use the GitHub API with the comment's node ID: - -```bash -gh api graphql -f query=' - mutation { - updateIssueComment(input: {id: "", body: ""}) { - issueComment { id } - } - } -' -``` - -Or use the REST API: - -```bash -gh api repos/{owner}/{repo}/issues/comments/ -X PATCH -f body="$(cat <<'EOF' -> **πŸ—οΈ build-plan** - -## Implementation Plan - -<... updated plan content ...> - ---- -*Revision β€” * -*Revision β€” * -*Revision 1 β€” initial plan* -EOF -)" -``` - -Preserve the full revision history at the bottom so readers can track how the plan evolved. - -Report to the user what feedback was addressed and whether the plan was updated. Stop. - ---- - -## 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. - -### Step 4: Scope Check - -Read the plan comment and check the **Complexity** and **Confidence** fields. - -- **If Complexity is High or Confidence is Low**, warn the user: - - > "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`. - -### Step 5: Conflict Detection - -Before creating a branch, check for conflicts: - -#### Check for existing branches - -```bash -git fetch origin -git branch -r | grep -i "" -``` - -If a remote branch referencing this issue ID exists, report it and ask the user whether to continue on that branch or abort. - -#### Check for existing PRs - -```bash -gh pr list --state open --search "Closes #" --json number,title,url -``` - -If an open PR already references this issue, report it and stop. Do not create a competing PR. - -### Step 6: Create Branch - -Determine the branch prefix from the issue type in the plan: - -| Issue type | Branch prefix | -| --- | --- | -| `feat` | `feat/` | -| `fix` | `fix/` | -| `refactor` | `refactor/` | -| `chore` | `chore/` | -| `perf` | `perf/` | -| `docs` | `docs/` | - -Get the current username and create the branch: - -```bash -USERNAME=$(gh api user --jq '.login') -git checkout main -git pull origin main -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. - -```bash -gh issue edit --remove-label "agent:implementation-requested" --remove-label "agent:plan-ready" --add-label "agent:in-progress" -``` - -### Step 8: Implement the Changes - -Follow the implementation steps from the plan. Principles: - -- **Follow the plan**: The plan was reviewed and approved. Stick to it unless you discover something that requires deviation. -- **Minimal scope**: Only change what the plan calls for. No unrelated refactors. -- **If you must deviate**: Note the deviation β€” it will be included in the PR description. - -Read the relevant source files before making changes. Implement step by step per the plan's sequence. - -### Step 9: Write Tests - -Write tests as specified in the plan's Test Plan section. Follow the project's existing test conventions. - -#### Unit tests - -- Place alongside existing tests for the module (e.g., `#[cfg(test)]` blocks in Rust, `test_*.py` for Python) -- Cover the new/changed behavior, edge cases, and error paths -- Ensure pre-existing behavior still works - -#### Integration tests - -- Place in the project's existing integration test directories -- Cover interactions between the changed components -- Test realistic scenarios including error conditions - -#### E2E tests - -- Only if the plan calls for them -- Cover the full user-facing workflow affected by the change - -#### Test naming - -Use descriptive names that document intent: -- `test_pagination_returns_correct_page_count` -- `test_rejects_negative_offset_parameter` -- `test_retry_succeeds_after_transient_failure` - -### Step 10: Verify β€” Tests, Lint, Pre-commit (Retry Loop) - -Verification has two phases: unit tests + pre-commit, then E2E tests (if applicable). Run with up to **3 attempts per phase**. - -#### Phase 1: Unit Tests and Pre-commit - -On each attempt: - -```bash -# Run pre-commit checks (linting, formatting, license headers) -mise run pre-commit -``` - -**If verification fails:** - -1. Read the error output carefully. -2. Fix the issues (test failures, lint errors, formatting). -3. Decrement the retry counter and try again. - -**If all 3 attempts fail**, stop and report to the user: -- What passed and what failed -- The specific errors from the last attempt -- That manual intervention is needed - -Do not proceed to Phase 2 or PR creation if Phase 1 is not green. - -#### Phase 2: E2E Tests (Conditional) - -**Trigger**: Run this phase if any files under `e2e/` were added or modified in this build. Check with: - -```bash -git diff --name-only main -- e2e/ -``` - -If there are no changes under `e2e/`, skip this phase entirely. - -If E2E files were modified, run the relevant E2E lane for the driver touched by the change: - -```bash -# Docker-backed gateway smoke E2E -mise run e2e:docker -``` - -Use `mise run e2e:podman`, `mise run e2e:vm`, or a Helm-backed Kubernetes E2E lane when the change targets those drivers. - -**E2E retry loop** (up to 3 attempts): - -1. Run the selected E2E lane. -2. If tests fail: - - Read the pytest output carefully β€” identify which tests failed and why. - - Distinguish between **test bugs** (the test itself is wrong) and **implementation bugs** (the code under test is wrong). - - Fix the failing code or tests. - - Decrement the retry counter and try again. -3. If tests pass, Phase 2 is green. - -**If all 3 E2E attempts fail**, stop and report to the user: -- Which E2E tests are failing -- The pytest output from the last attempt -- Whether the failures appear to be test issues or implementation issues -- That manual intervention is needed - -Do not proceed to PR creation if E2E verification is not green. - -### Step 11: Update Documentation - -Review the documentation requirements in `AGENTS.md` and update any affected -docs as part of the implementation. Keep documentation changes scoped to the -behavior or subsystem that changed. - -If the implementation changes gateway TOML parsing, `[openshell.gateway]` -fields, `[openshell.drivers.]` fields, driver config defaults, or Helm -rendering of `gateway.toml`, update `docs/how-it-works/gateways/configuration.mdx` in the -same branch. If the change affects user-facing compute-driver setup, also -update `docs/how-it-works/sandboxes/runtimes.mdx` or the relevant deployment -page. - -Use the `sync-agent-infra` skill's maintenance map to identify related skill updates when the implementation changes behavior, commands, or development workflows. Run its full consistency check when the implementation adds, removes, or renames skills or crates; changes workflow relationships or skill coverage; modifies issue or PR templates; or changes agent cross-references. Fix any drift before committing. - -### Step 12: Commit and Push - -Commit all changes using conventional commit format. The `` comes from the issue type in the plan: - -```bash -git add -git commit -m "$(cat <<'EOF' -(): - -Closes # - - -EOF -)" -``` - -Push: - -```bash -git push -u origin HEAD -``` - -### Step 13: Open PR - -Create the PR: - -```bash -gh pr create \ - --title "(): " \ - --body "$(cat <<'EOF' -> **πŸ—οΈ build-from-issue-agent** - -## Summary -<1-3 sentences describing what was built and the approach taken> - -## Related Issue -Closes # - -## Changes -- ``: -- ``: - -### Deviations from Plan - - -## Testing -- [x] `mise run pre-commit` passes -- [x] Unit tests added/updated -- [x] E2E tests added/updated (if applicable) - -**Tests added:** -- **Unit:** -- **Integration:** -- **E2E:** - -## Checklist -- [x] Follows Conventional Commits -- [x] Commits are signed off (DCO) - -**Documentation updated:** -- ``: -EOF -)" -``` - -**Display the PR URL** so it's easily clickable: - -``` -Created PR [#](https://github.com/OWNER/REPO/pull/) -``` - -### Step 14: Post-Build Cleanup - -#### Post summary comment on the issue - -```bash -gh issue comment --body "$(cat <<'EOF' -> **πŸ—οΈ build-from-issue-agent** - -## Implementation Complete - -PR: [#](https://github.com/OWNER/REPO/pull/) - -### What was built -<1-2 sentence summary> - -### Tests -- Unit: tests added -- Integration: -- E2E: - -### Docs updated -- - -The issue will auto-close when the PR is merged. -EOF -)" -``` - -#### Post E2E attestation comment on the PR - -If E2E tests were run in Phase 2 of Step 10, post an attestation comment on the **PR** documenting that local E2E tests passed. This is necessary because E2E tests are not yet running in CI β€” this comment serves as the verification record for reviewers. - -Collect the metadata before posting: - -```bash -# Get the commit SHA that was tested -COMMIT_SHA=$(git rev-parse HEAD) - -# Get the test output summary (last few lines of pytest output) -# This was captured during the Phase 2 run β€” include the pass/fail/skip counts -``` - -Post the attestation: - -```bash -gh pr comment --body "$(cat <<'EOF' -> **πŸ—οΈ build-from-issue-agent** - -## E2E Test Attestation - -Local E2E tests passed. CI does not currently run E2E tests, so this comment serves as the verification record. - -| Field | Value | -|-------|-------| -| **Commit** | `` | -| **Command** | `` | -| **Gateway mode** | `` | -| **Result** | βœ… All passed | - -### Test Summary - -``` - -``` - -### Tests Executed -- `::` β€” PASSED -- `::` β€” PASSED -- ... -EOF -)" -``` - -Include **every test** that ran (not just the new ones) so the reviewer can see full coverage. If any tests were skipped, note them and explain why. - -#### 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: - -```bash -gh issue edit --remove-label "agent:in-progress" --add-label "agent:pr-opened" -``` - -#### Report workflow run URL - -Get the workflow run URL from the PR so the user can monitor CI: - -```bash -BRANCH=$(gh pr view --json headRefName --jq '.headRefName') -gh run list --branch "$BRANCH" --limit 1 --json databaseId,status,url -``` - -Report the workflow run URL and suggest the user can use the `watch-github-actions` skill to monitor it. - ---- - -## Branch D: Resume In-Progress Build - -If the `agent:in-progress` label is present, the skill was previously started but may not have completed. - -1. Check for an existing branch matching the issue ID: - ```bash - git branch -r | grep -i "" - ``` -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. - ---- - -## Useful Commands Reference - -| Command | Description | -| --- | --- | -| `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 issue comment --body "..."` | Post a comment on an issue | -| `gh api repos/{owner}/{repo}/issues/comments/ -X PATCH -f body="..."` | Edit an existing comment | -| `gh issue edit --add-label "..."` | Add labels | -| `gh issue edit --remove-label "..."` | Remove labels | -| `gh pr list --state open --search "..."` | Search for open PRs | -| `gh pr create --title "..." --body "..."` | Create a pull request | -| `gh api user --jq '.login'` | Get current GitHub username | -| `mise run pre-commit` | Run pre-commit checks (lint, format, license headers) | -| `mise run e2e:docker` | Run smoke E2E against a standalone Docker-backed gateway | -| `mise run e2e:podman` | Run smoke E2E against a Podman-backed gateway | -| `mise run e2e:vm` | Run smoke E2E against the VM compute driver | - -## Example Usage - -### First run β€” no plan exists - -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 -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 -8. Report to user: "Plan posted on issue #42. Awaiting review." - -### Second run β€” human left feedback - -User says: "Check on issue #42" - -1. Fetch issue #42 and comments -2. Find existing plan comment (Revision 1) -3. Find new human comment: "Should we also paginate the search endpoint?" -4. Post response quoting the question, explaining that search pagination is out of scope for this issue but could be a follow-up -5. Report to user: "Responded to feedback on #42. Plan unchanged." - -### Third run β€” human revised scope, plan needs update - -User says: "Check issue #42" - -1. Fetch issue #42 and comments -2. Find plan + new human comment: "Actually, let's include search pagination. Updated the issue description." -3. Post response acknowledging the scope change -4. Edit the plan comment to include search endpoint pagination β€” Revision 2 -5. Report to user: "Updated plan to include search pagination (Revision 2)." - -### Fourth run β€” implementation requested - -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 -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 -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 -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 -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 - -### Run on issue with existing PR - -User says: "Build issue #42" - -1. Fetch issue #42 β€” `agent:pr-opened` label present -2. Find existing PR #789 linked to the issue -3. Report: "PR [#789](...) already exists for issue #42. Nothing to build." - -### Run on high-complexity issue - -User says: "Build issue #99" - -1. Fetch issue #99 β€” warn about any missing expected workflow labels, then continue because the user directly requested implementation -2. Plan exists: complexity High, confidence Low, has open questions -3. Warn user: "Issue #99 is rated High complexity / Low confidence. Proceeding but flagging for your awareness." -4. Continue with build +Do not apply acceptance or roadmap decisions on behalf of a maintainer. Do not introduce `agent:*` workflow labels. diff --git a/.agents/skills/build-openshell-mxc-windows/SKILL.md b/.agents/skills/build-openshell-mxc-windows/SKILL.md index 56a86fcd37..a6f71388ff 100644 --- a/.agents/skills/build-openshell-mxc-windows/SKILL.md +++ b/.agents/skills/build-openshell-mxc-windows/SKILL.md @@ -215,8 +215,8 @@ compatibility under emulation is not part of these tasks. The aggregate commands above on an ARM64 host. The repository-wide `mise run pre-commit` task is also supported on Windows. -Run `rust:lockfiles:check`, `sdk:ts:ci`, `go:ci`, and `test:e2e-parity` through -the Windows-aware tasks when validating those surfaces. Do not count the Go +Run `rust:lockfiles:check`, `sdk:ts:ci`, and `go:ci` through the Windows-aware +tasks when validating those surfaces. Do not count the Go Windows ARM64 race-detector exclusion or POSIX permission-bit skips as security coverage. SDK test dependencies must remain at their lockfile versions. Its Rust check, Clippy, and test dependencies enter the same MSVC environment @@ -265,7 +265,7 @@ MXC on Windows. Each other `compute-driver-*` feature installs its own Windows rejection stub without linking that driver crate. The default `in-tree-compute-drivers` alias enables all five features. An MXC-only build uses `--no-default-features --features compute-driver-mxc` (add `telemetry` -and `bundled-z3` as needed). +and `openshell-server/prebuilt-z3` as needed). | Driver | Windows build behavior | Runtime behavior | |---|---|---| diff --git a/.agents/skills/create-github-issue/SKILL.md b/.agents/skills/create-github-issue/SKILL.md index 2510b79272..f47ace151b 100644 --- a/.agents/skills/create-github-issue/SKILL.md +++ b/.agents/skills/create-github-issue/SKILL.md @@ -19,7 +19,7 @@ This project uses YAML form issue templates. When creating issues, match the tem ### Bug Reports -Do not add a type label automatically. The body must include a **User Story**, **Problem Statement**, **Impact / Why This Matters**, and **Acceptance Criteria**, followed by bug-specific reproduction steps and environment details. Logs are optional and must be concise and redacted. Apply area or topic labels only when they are clearly known. +Do not add a type label automatically. Confirm that the human operator personally uses OpenShell and directly encountered the problem or needs the feature for a specific use case. If that first-hand attestation or concrete use case is missing, ask for it before creating the issue. Frame the issue entirely in terms of OpenShell. The body must include a **User Story**, **Problem Statement**, **Impact / Why This Matters**, and **Acceptance Criteria**, followed by bug-specific reproduction steps using only OpenShell deployments and environment details. Do not install third-party tools to demonstrate reproducibility. Logs are optional and must be concise and redacted. If the issue suggests a change to configuration, CLI, SDK, or other user experience, include a notional example of the proposed interaction for human review. Inspect current repository labels before applying any; use only labels whose meaning is clear. ```bash gh issue create \ @@ -27,7 +27,7 @@ gh issue create \ --body "$(cat <<'EOF' ## User Story -As a , I want , so that . +I use OpenShell for . I directly encountered or need so that . ## Problem Statement @@ -52,18 +52,20 @@ As a , I want , so that . - OS: - Runtime, deployment, or integration: +## Suggested UX (if applicable) + + + ## Logs -``` - -``` + EOF )" ``` ### Feature Requests -Do not add a type label automatically. The body must include a **User Story**, **Problem Statement**, **Impact / Why This Matters**, **Proposed Design**, **Acceptance Criteria**, and **Alternatives Considered**. The proposed design should define the user-facing workflow and externally observable behavior without prescribing internal implementation. Agent investigation is optional. Apply area or topic labels only when they are clearly known. +Do not add a type label automatically. Confirm that the human operator personally uses OpenShell and directly encountered the problem or needs the feature for a specific use case. If that first-hand attestation or concrete use case is missing, ask for it before creating the issue. Frame the issue entirely in terms of OpenShell. The body must include a **User Story**, **Problem Statement**, **Impact / Why This Matters**, **Proposed Design**, **Acceptance Criteria**, and **Alternatives Considered**. The proposed design should define the user-facing workflow and externally observable behavior without prescribing internal implementation. Agent investigation is optional. If the issue suggests a change to configuration, CLI, SDK, or other user experience, include a notional example of the proposed interaction for human review. Inspect current repository labels before applying any; use only labels whose meaning is clear. ```bash gh issue create \ @@ -71,7 +73,7 @@ gh issue create \ --body "$(cat <<'EOF' ## User Story -As a , I want , so that . +I use OpenShell for . I directly encountered or need so that . ## Problem Statement @@ -85,13 +87,17 @@ As a , I want , so that . +## Suggested UX (if applicable) + + + ## Acceptance Criteria - [ ] ## Alternatives Considered - + ## Agent Investigation @@ -102,12 +108,16 @@ EOF ### Tasks -For internal tasks that don't fit bug/feature templates: +For internal tasks that do not fit bug/feature templates, still obtain the operator's first-hand OpenShell use case before creating the issue: ```bash gh issue create \ --title ": " \ --body "$(cat <<'EOF' +## User Story + + + ## Description @@ -125,7 +135,7 @@ EOF GitHub built-in issue types (`Bug`, `Feature`, `Task`) should come from the matching issue template when possible, or be set manually afterward. Do not try to emulate them through labels. -Creating an issue does not accept it or queue agent work. Agents never apply `state:accepted`, the `roadmap` label, add issues to the roadmap project, or apply `agent:plan-requested` or `agent:implementation-requested`. Community issues proceed through `triage-issue`; a human accepts technically validated work with `state:accepted` or roadmap placement. The request labels queue work for unattended agents. A user may instead direct an agent to a specific issue; the agent warns about missing expected workflow labels and continues with the requested phase without changing them. +Creating an issue does not accept it. Inspect the repository’s current `state:*` labels and follow its triage β†’ validation β†’ human acceptance process. Agents may assess facts, but only humans decide whether to accept work or place it on the roadmap. A direct user request authorizes the requested planning or implementation phase without changing issue disposition. ## Useful Options @@ -150,5 +160,5 @@ Created issue [#123](https://github.com/OWNER/REPO/issues/123) Use the issue number to: -- Reference in commits: `git commit -m "Fix validation error (fixes #123)"` -- Create a branch following project convention: `-/` +- Reference in signed-off Conventional Commits: `git commit --signoff -m "fix(cli): validate empty requests (fixes #123)"` +- Create a branch following project convention: `/-/`, where `` is a Conventional Commits type. diff --git a/.agents/skills/create-github-pr/SKILL.md b/.agents/skills/create-github-pr/SKILL.md index dd0463df8b..b1c73aa5a2 100644 --- a/.agents/skills/create-github-pr/SKILL.md +++ b/.agents/skills/create-github-pr/SKILL.md @@ -13,7 +13,7 @@ Create pull requests on GitHub using the `gh` CLI. - The `gh` CLI must be authenticated (`gh auth status`) - You must have commits on a branch that's pushed to the remote -- For issue-backed work, the branch should follow `-/`. Exempt issue-less changes may use `/`. +- Every PR must close an existing issue. The branch should follow `/-/`. ## Before Creating a PR @@ -30,13 +30,11 @@ deployment docs. Use the `sync-agent-infra` skill's maintenance map to identify related skill updates when the branch changes behavior, commands, or development workflows. Run its full consistency check when the branch adds, removes, or renames skills or crates; changes workflow relationships or skill coverage; modifies issue or PR templates; or changes agent cross-references. Resolve any drift before creating the PR. -### Run Pre-commit Checks +### Verify the Affected Areas -Run the local pre-commit task before opening a PR: +Use the verification guidance in `CONTRIBUTING.md` to select checks for the changed files and behavior. Guidance, skills, and template changes need applicable Markdown, YAML, link, and consistency checks. Run Rust or SDK suites when those components or their dependencies can be affected. Shared APIs, schemas, dependencies, and build changes may require broader checks even when component source files are unchanged. -```bash -mise run pre-commit -``` +`mise run ci` and `mise run pre-commit` are broad convenience tasks, not blanket PR prerequisites. Broaden validation only for a concrete remaining risk or failed check, and report what actually ran. ### Verify Branch State @@ -49,21 +47,13 @@ Before creating a PR, verify: git branch --show-current ``` -2. **Branch follows naming convention** - Use `-/` for issue-backed work or `/` for an exempt issue-less change. +2. **Branch follows naming convention** - Use `/-/`, where `` is a Conventional Commits type. ```bash - # Example: 1234-add-pagination/jd + # Example: feat/1234-add-pagination/johntmyers git branch --show-current ``` -3. **Consider squashing commits** - For cleaner history, squash related commits before pushing: - - ```bash - # Squash last N commits into one - git reset --soft HEAD~N - git commit -m "feat(component): description" - ``` - ### Push Your Branch Ensure your branch is pushed to the remote: @@ -116,19 +106,25 @@ gh pr create --title "PR title" --body "PR description" ### Link to an Issue -Features, user-visible behavior changes, public API changes, architecture changes, and multi-PR efforts must link an accepted issue. Use `Closes #` in the body to auto-close the issue when merged: +Every PR must close its own issue. Verify that the issue exists, remains open, and covers the PR scope. Use `Closes #` in the body so merge closes it: ```bash gh pr create \ - --title "Fix validation error for empty requests" \ - --body "Closes #123 + --title "fix(cli): validate empty requests" \ + --body "## Summary -## Summary -- Added validation for empty request bodies -- Returns 400 instead of 500" +Validate empty request bodies. + +## Related Issue + +Closes #123 + +## Changes + +- Return 400 instead of 500" ``` -Small documentation fixes, mechanical maintenance, and obvious localized bug fixes may omit a separate issue when the PR contains enough context to review the decision and implementation together. In that case, write `No issue required: ` in the Related Issue section. Do not use this exception for security fixes; follow `SECURITY.md`. +If the work needs multiple PRs, create a separate closable issue for each PR. A higher-level tracking issue may link the component issues, but no PR should close that tracking issue until all its work is complete. Follow `SECURITY.md` for vulnerability disclosure. First-time external contributors must be vouched before their PRs are accepted; the vouch check may close unvouched PRs. Check the current vouch process before opening a PR for an external contributor. ### Create as Draft @@ -138,12 +134,6 @@ For work-in-progress that's not ready for review: gh pr create --draft --title "WIP: New feature" ``` -### With Labels - -```bash -gh pr create --title "Title" --label "area:cli" --label "topic:security" -``` - ### Target a Different Branch Default target is `main`. To target a different branch: @@ -161,15 +151,15 @@ PR descriptions must follow the project's [PR template](.github/PULL_REQUEST_TEM ## Related Issue - + ## Changes ## Testing -- [ ] `mise run pre-commit` passes -- [ ] Unit tests added/updated +- [ ] Checks appropriate to the affected code and behavior pass +- [ ] Unit tests added/updated (if applicable) - [ ] E2E tests added/updated (if applicable) ## Checklist @@ -201,7 +191,7 @@ Closes #456 ## Testing -- [x] `mise run pre-commit` passes +- [x] Relevant CLI format, lint, and unit checks pass - [x] Unit tests added/updated - [ ] E2E tests added/updated (if applicable) diff --git a/.agents/skills/create-spike/SKILL.md b/.agents/skills/create-spike/SKILL.md index 5c6d16c2ec..d5e0878fc4 100644 --- a/.agents/skills/create-spike/SKILL.md +++ b/.agents/skills/create-spike/SKILL.md @@ -1,318 +1,27 @@ --- name: create-spike -description: Investigate a plain-language problem description by deeply exploring the codebase, then create a structured GitHub issue with technical findings. Prequel to build-from-issue β€” maps vague ideas to concrete, buildable issues. Trigger keywords - spike, investigate, explore, research issue, technical investigation, create spike, new spike, feasibility, codebase exploration. +description: Investigate an OpenShell problem and create a structured issue with technical findings for human disposition. metadata: internal: true --- # Create Spike -Investigate a problem, map it to the codebase, and produce a structured GitHub issue ready for human disposition and roadmap placement. +Investigate a specific OpenShell need and record the findings in a GitHub issue. Use `create-github-issue` for the issue structure and `triage-issue` for the distinction between technical validation and human acceptance. A spike does not authorize implementation or a roadmap decision. -A **spike** is an exploratory investigation. The user has a vague idea β€” a feature they want, a bug they've noticed, a performance concern β€” but hasn't mapped it to code, assessed feasibility, or structured it as a buildable issue. This skill does that mapping. +## Before investigating -## Prerequisites +Ask the human operator to attest that they personally use OpenShell and directly encountered the problem or need the feature for a specific use case. If that context is absent, request it before creating the issue. Do not invent a user story or file a generic platform wish as their first-hand need. Search existing issues to avoid duplication. Follow `SECURITY.md` for suspected vulnerabilities instead of filing a public issue. -- The `gh` CLI must be authenticated (`gh auth status`) -- You must be in a git repository with a GitHub remote +## Investigate -## Workflow Overview +1. Reconstruct the current OpenShell workflow and the claimed gap. For a bug, use reproduction steps requiring only OpenShell deployments; do not install third-party tools solely to demonstrate it. +2. Explore the relevant code and tests. Separate observed behavior, likely cause, and open questions. If the claim cannot be validated, state the exact missing evidence. +3. For a feature, describe the desired external behavior and evaluate alternatives, including relevant middleware, interceptors, providers, or other extension points. Prefer an applicable extension when it satisfies the use case; the need to run another service alone does not disqualify it. +4. For configuration, CLI, SDK, or other UX changes, include notional commands, configuration, or API examples for human review. Leave internal implementation choices open unless they are essential constraints. -``` -User describes a problem - β”‚ - β”œβ”€ Step 1: Gather the problem statement - β”‚ └─ Ask ONE round of clarifying questions if genuinely needed - β”‚ - β”œβ”€ Step 2: Deep codebase investigation via principal-engineer-reviewer - β”‚ └─ Map the problem to code, assess feasibility, identify risks - β”‚ - β”œβ”€ Step 3: Determine labels from the repo - β”‚ - β”œβ”€ Step 4: Create a GitHub issue with structured findings - β”‚ - └─ Step 5: Report to user with issue URL and next steps -``` +## Record the result -## Step 1: Gather the Problem Statement +Create an issue with User Story, Problem Statement, Impact / Why This Matters, Proposed Design when relevant, Acceptance Criteria, Alternatives Considered, and concise Agent Investigation. Include OpenShell-only reproduction and environment details for bugs. Inspect current GitHub `state:*` labels and descriptions before applying the one that matches the evidence; do not hard-code label names. Do not apply an acceptance state or add the issue to the roadmap. -The user provides a problem description. This could be: - -- A feature idea: "I want sandboxes to be able to reach private IPs" -- A bug report: "The retry logic in the proxy seems too aggressive" -- A performance concern: "Policy evaluation is slow for large rule sets" -- A refactoring goal: "The config parsing is scattered across too many modules" - -Extract from the user's input: - -1. **What** they want (the desired outcome or observed problem) -2. **Why** they want it (motivation, use case, or trigger) -3. **Constraints** they've mentioned (backwards compatibility, performance targets, etc.) - -### Clarification policy - -If the problem is too vague to determine which area of the codebase to investigate, ask **ONE** round of clarifying questions. Do not over-interrogate. Examples of when to ask: - -- "Make things faster" β€” ask which component or operation is slow -- "Fix the networking" β€” ask what specific behavior is wrong - -Examples of when NOT to ask: - -- "The retry logic in the proxy is too aggressive" β€” clear enough, start investigating -- "Allow sandbox egress to private IP space" β€” clear enough, start investigating -- "The OPA policy evaluation needs caching" β€” clear enough, start investigating - -## Step 2: Deep Codebase Investigation - -This is the core of the skill. Use the Task tool with the `principal-engineer-reviewer` sub-agent to perform a thorough codebase investigation. - -``` -Task tool with subagent_type="principal-engineer-reviewer" -``` - -The prompt to the reviewer **must** instruct it to: - -1. **Identify which components/subsystems are involved.** Don't just guess from names β€” read the code to confirm. - -2. **Read the relevant source files thoroughly.** Not just grep for keywords β€” actually read and understand the logic. Follow the call chain from entry point through to the relevant behavior. - -3. **Map the current architecture for the affected area.** How do the components interact? What's the data flow? Where are the boundaries? - -4. **Identify the exact code paths that would need to change.** Provide file paths and line numbers. Name the functions, structs, and modules. - -5. **Assess feasibility and complexity:** - - **Low**: Isolated change, < 3 files, clear path forward - - **Medium**: Multiple files/components, some design decisions, but well-scoped - - **High**: Cross-cutting changes, architectural decisions needed, significant unknowns - -6. **Identify risks, edge cases, and design decisions that need human input.** What could go wrong? What trade-offs exist? What decisions shouldn't be made by an agent? - -7. **Check for existing patterns in the codebase that should be followed.** If there's a convention for how similar features are implemented, note it. The implementation should be consistent. - -8. **Look at relevant tests to understand test coverage expectations.** What test patterns exist? What level of coverage is expected for this area? - -9. **Check design records** in `rfc/` and the affected crate `README.md` files for relevant decisions and constraints. - -10. **Assess gateway config documentation impact.** If the change would add, remove, rename, or change defaults for gateway TOML keys or driver-specific config options, call out that `docs/how-it-works/gateways/configuration.mdx` must be updated. If the change is surfaced through Helm or compute-driver setup docs, call out the relevant deployment or compute-driver docs too. - -11. **Assess Linux Security Module (LSM) impact.** If the change involves process identity, `/proc` filesystem access, file labeling, binary execution, or inter-process visibility, call out whether it will behave differently on hosts running SELinux (enforcing) or AppArmor. For example: reading `/proc//exe` across an SELinux domain boundary returns ENOENT, not EACCES. Tests that fork+exec into system binaries (different SELinux label) will fail on enforcing hosts. Flag any LSM-sensitive code paths and recommend mitigations. - -12. **Determine the issue type:** `feat`, `fix`, `refactor`, `chore`, `perf`, or `docs`. - -### What makes a good investigation prompt - -Include in the prompt to the reviewer: - -- The user's problem statement (verbatim or lightly paraphrased) -- Any constraints the user mentioned -- A clear instruction to return: component list, file references with line numbers, architecture summary, feasibility assessment, risks, and the issue type - -### What to do with the results - -The reviewer will return a detailed analysis. You'll use this to populate the issue body (Step 4). The issue should contain both the stakeholder-readable summary and the full technical investigation β€” everything in one place. - -## Step 3: Determine Labels - -Fetch the available labels from the repository: - -```bash -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 -- **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 - -## Step 4: Create the GitHub Issue - -Create the issue with a structured body containing both the stakeholder-readable summary and the full technical investigation. The title should follow conventional commit format. - -```bash -gh issue create \ - --title ": " \ - --label "" --label "" \ - --body "$(cat <<'EOF' -## Problem Statement - - - -## Technical Context - - - -## Affected Components - -| Component | Key Files | Role | -|-----------|-----------|------| -| | ``, `` | | -| ... | ... | ... | - -## Technical Investigation - -### Architecture Overview - - - -### Code References - -| Location | Description | -|----------|-------------| -| `:` | | -| `:` | | -| ... | ... | - -### Current Behavior - - - -### What Would Need to Change - - - -### Alternative Approaches Considered - - - -### Patterns to Follow - - - -## Proposed Approach - - - -## Scope Assessment - -- **Complexity:** -- **Confidence:** -- **Estimated files to change:** -- **Issue type:** `` - -## Risks & Open Questions - -- -- -- ... - -## Disposition Readiness - -- **State:** `` -- **Assessment:** -- **Missing evidence:** - -## Test Considerations - -- -- -- -- - ---- -*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.* -EOF -)" -``` - -**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: - -``` -Created issue [#](https://github.com/OWNER/REPO/issues/) -``` - -## Step 5: Report to User - -After creating the issue, report: - -1. The issue URL (as a clickable markdown link) -2. A 2-3 sentence summary of what was found -3. Key risks or decisions that need human attention -4. Next steps: - -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. - -For `state: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. - -## Design Principles - -1. **Everything goes in the issue body.** Do NOT post follow-up comments. The issue body should contain both the stakeholder-readable summary and the full technical investigation, all in one place. - -2. **Do NOT create an implementation plan.** The spike identifies the problem space and proposes a direction. The implementation plan is `build-from-issue`'s responsibility, created after human review of the spike. - -3. **One round of clarification max.** Don't turn this into an interrogation. If the user provides enough to identify the area of the codebase, start investigating. - -4. **The issue should save `build-from-issue` work.** When `build-from-issue` runs, it reads the issue body as input context. The technical investigation section should contain enough detail that its `principal-engineer-reviewer` can build on the investigation rather than starting from scratch. - -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. - -## Useful Commands Reference - -| Command | Description | -| --- | --- | -| `gh issue create --title "..." --body "..." --label "..."` | Create a new issue | -| `gh label list --limit 100` | List available labels in the repo | -| `gh issue edit --add-label "..."` | Add labels to an issue | -| `gh issue view --json number,title,body,state,labels` | Fetch issue metadata | - -## Example Usage - -### Feature spike - -User says: "Allow sandbox egress to private IP space via networking policy" - -1. Problem is clear β€” no clarification needed -2. Fire `principal-engineer-reviewer` to investigate: - - Finds `is_internal_ip()` SSRF check in `proxy.rs` that blocks RFC 1918 addresses - - Reads OPA policy evaluation pipeline in `opa.rs` and `crates/openshell-sandbox/data/sandbox-policy.rego` - - Reads proto definitions in `sandbox.proto` for `NetworkEndpoint` - - Maps the 4-layer defense model: netns, seccomp, OPA, SSRF check - - Reads RFC 0002 and the `openshell-policy` crate README - - Identifies exact insertion points: policy field addition, SSRF check bypass path, OPA rule extension - - Assesses: Medium complexity, High confidence, ~6 files -3. Fetch labels β€” select `area:sandbox`, `area:proxy`, `area:policy`, `state:validated` -4. Create issue: `feat: allow sandbox egress to private IP space via networking policy` β€” body includes both the summary and full investigation (code references, architecture context, alternative approaches) -5. Report: "Created issue #59. The investigation found that private IP blocking is enforced at the SSRF check layer in the proxy. The proposed approach adds a policy-level override. A human must now accept or decline it and place it on the roadmap if accepted." - -### Bug investigation spike - -User says: "The proxy retry logic seems too aggressive β€” I'm seeing cascading failures under load" - -1. Problem is clear enough β€” investigate retry behavior in the proxy -2. Fire `principal-engineer-reviewer`: - - Finds retry configuration in proxy request handling - - Reads the retry loop, backoff strategy, and timeout settings - - Checks if there's circuit breaker logic - - Maps the failure propagation path - - Identifies that retries happen without backoff jitter, causing thundering herd - - Assesses: Low complexity, High confidence, ~2 files -3. Fetch labels β€” select `area:proxy`, `state:validated` -4. Create issue: `fix: proxy retry logic causes cascading failures under load` β€” body includes both the summary and full investigation (retry code references, current behavior trace, comparison to standard backoff patterns) -5. Report: "Created issue #74. The proxy retries without jitter or circuit breaking, which amplifies failures under load. A human must now accept or decline it and place it on the roadmap if accepted." - -### Performance/refactoring spike - -User says: "Policy evaluation is getting slow β€” can we cache compiled OPA policies?" - -1. Problem is clear β€” investigate OPA policy evaluation performance -2. Fire `principal-engineer-reviewer`: - - Reads the OPA evaluation pipeline end to end - - Measures where policies are loaded and compiled (per-request vs. cached) - - Checks if there's an existing caching layer - - Reads the policy reload/hot-swap mechanism - - Identifies that policies are recompiled on every evaluation - - Assesses: Medium complexity, Medium confidence (cache invalidation is a design decision), ~4 files -3. Fetch labels β€” select `area:policy`, `state:validated` -4. Create issue: `perf: cache compiled OPA policies to reduce evaluation latency` β€” body includes both the summary and full investigation (compilation hot path, per-request overhead, cache invalidation strategies with trade-offs) -5. Report: "Created issue #81. Policies are recompiled per-request with no caching. The main design decision is the cache invalidation strategy. A human must now accept or decline it and place it on the roadmap if accepted." +Report the issue URL, technical findings, uncertainties, and the human disposition needed. For subsequent authorized implementation, use `build-from-issue`. Every eventual PR must close an issue covering its own scope; split multi-PR efforts into separate closable issues and use a high-level issue only for tracking. diff --git a/.agents/skills/fix-security-issue/SKILL.md b/.agents/skills/fix-security-issue/SKILL.md index 1452cbe66c..30e6fa15bd 100644 --- a/.agents/skills/fix-security-issue/SKILL.md +++ b/.agents/skills/fix-security-issue/SKILL.md @@ -1,322 +1,18 @@ --- 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 an authorized fix for a reviewed security issue and open a PR that closes its issue. metadata: internal: true --- # Fix Security Issue -Implement a code fix for a security issue that has already been reviewed by the `review-security-issue` skill. +Use this skill after an authorized `review-security-issue` review identifies an actionable concern. Follow `SECURITY.md`; do not disclose vulnerability details in a public issue. A direct user request to fix a specific reviewed issue authorizes implementation. For unattended work, inspect current `state:*` label descriptions, maintainer assignments, and comments to verify that remediation is authorized. Do not infer approval from a state that only records technical validation. -## Prerequisites +1. Fetch the issue and its comments with `gh issue view --json number,title,body,state,labels,comments`. Inspect current repository labels and confirm this is a security issue. Find the review marked `> **πŸ”’ security-review-agent**` and its remediation plan. If the review is missing or found the issue not actionable, stop and report that result. +2. Verify the review against current code. Adapt the plan when code has changed, and record material deviations. Check for an existing owner, branch, or PR. +3. Create a branch or worktree using `fix/-/`, preserving unrelated changes. Implement the smallest safe fix and add regression tests for the security boundary. Avoid logging secrets or adding public exploit detail. +4. Follow the verification guidance in `CONTRIBUTING.md`. Run format, lint, compile or type checks, and regression tests for the affected security boundary and dependent components, plus the relevant E2E lane for sandbox or policy changes. Broaden verification when the fix spans components or a concrete risk remains; do not require unaffected Rust or SDK suites solely to create a signed-off commit or PR. +5. Follow `create-github-pr` and use `Closes #` for the reviewed issue. Every PR must close its own issue; split multi-PR remediations into separate issues in the authorized security workflow. Keep the PR description appropriately scoped to its disclosure venue. -- 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 a prior security review comment (posted by `review-security-issue`) with a **Legitimate concern** determination and a remediation plan - -## Agent Comment Marker - -All PR descriptions and comments posted by this skill **must** begin with the following marker line: - -``` -> **πŸ”§ security-fix-agent** -``` - -This distinguishes fix-agent content from review-agent comments (`πŸ”’ security-review-agent`) and human comments. - -## Step 1: Identify the Issue - -The user may provide an issue number directly, or ask the agent to find issues to fix. - -### 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. - -### If no issue number is provided - -Scan for open issues labeled `topic:security` and `agent:implementation-requested`: - -```bash -gh issue list --label "topic:security" --label "agent:implementation-requested" --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. -- **If one issue is found**, proceed to Step 2 with that issue. -- **If multiple issues are found**, list them for the user and ask which one to work on. If the user said to handle all of them, process them sequentially (one full fix cycle per issue). - -## Step 2: Fetch the Issue and Validate Labels - -Fetch the issue details: - -```bash -gh issue view --json number,title,body,state,labels,author -``` - -### Validate the Security Label and Invocation Mode - -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. - -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. - -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. - -### Validate the security review - -Once labels are confirmed, fetch the comments to find the security review: - -```bash -gh issue view --json comments --jq '[.comments[] | select(.body | contains("security-review-agent"))]' -``` - -- **If no `security-review-agent` comment is found**, report to the user that this issue has not been reviewed yet. Suggest running the `review-security-issue` skill first. Stop. -- **If the review determination is "Not actionable"**, report to the user that the review found no actionable concern. There is nothing to fix. Stop. -- **If the review determination is "Legitimate concern"**, extract the **Remediation Plan** and **Severity Assessment** sections from the review comment. Proceed to Step 3. - -## Step 3: Plan the Implementation - -Before writing code, analyze the remediation plan from the review comment: - -1. Identify all files and components mentioned in the remediation plan. -2. Read those files to understand the current code. -3. Determine if the remediation plan is still accurate given the current state of the code (the codebase may have changed since the review). -4. Break the fix into discrete, testable changes. - -If the remediation plan references files or components that no longer exist or have changed significantly, adapt the plan accordingly and note the deviations. - -## Step 4: Create a Branch - -Create a working branch for the fix: - -```bash -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: - -```bash -gh issue edit --remove-label "agent:implementation-requested" --remove-label "agent:plan-ready" --add-label "agent:in-progress" -``` - -## Step 5: Implement the Fix - -Implement the changes described in the remediation plan. Follow these principles: - -- **Minimal scope**: Only change what is necessary to address the security concern. Avoid unrelated refactors. -- **Defense in depth**: Where appropriate, add multiple layers of protection (input validation, output encoding, access checks, etc.). -- **No regressions**: Ensure existing tests still pass after the fix. - -After implementing, run the project's pre-commit checks: - -```bash -mise run pre-commit -``` - -Fix any issues that arise before proceeding. - -## Step 6: Write Tests - -Every security fix **must** include tests that verify the vulnerability is resolved. Choose the appropriate test level(s) based on the nature of the fix: - -### Unit tests - -Add unit tests when the fix changes a specific function, method, or module in isolation. Place them alongside the existing tests for that module (e.g., same `tests/` directory or `#[cfg(test)]` block for Rust, `test_*.py` for Python). - -Unit tests should cover: -- The previously-vulnerable code path now rejects malicious input or behaves correctly -- Edge cases around the security boundary (empty input, oversized input, special characters, etc.) -- That legitimate inputs continue to work as before - -### Integration / E2E tests - -Add integration or end-to-end tests when the vulnerability spans multiple components or is triggered via an API endpoint, CLI command, or network boundary. Place them in the project's existing integration or e2e test directories. - -Integration tests should cover: -- The full attack scenario described in the security review is no longer exploitable -- The fix holds under realistic conditions (authenticated vs. unauthenticated, different roles, etc.) - -### Test naming - -Name tests descriptively to document the security concern: -- `test_rejects_sql_injection_in_search_query` -- `test_blocks_path_traversal_in_file_upload` -- `test_enforces_auth_on_admin_endpoint` - -### Verify - -Run the full relevant test suite to confirm both the new tests pass and no existing tests regress: - -```bash -# Run tests relevant to the changed components -# The specific command depends on the project area affected -``` - -If the review identified a specific exploit scenario, verify that it is no longer possible with the fix in place. - -## Step 7: Update Documentation - -Review the documentation requirements in `AGENTS.md` and update any affected -docs as part of the security fix. If the fix is purely internal, such as -switching to parameterized queries with no external behavior change, -documentation updates may not be needed. - -## Step 8: Commit, Push, and Open PR - -### Commit - -Commit all changes (implementation, tests, and documentation) using conventional commit format: - -```bash -git add -git commit -m "$(cat <<'EOF' -fix(security): - -Closes # - - -EOF -)" -``` - -### Push - -```bash -git push -u origin HEAD -``` - -### Create the PR - -Create a PR that closes the security issue. Put the full fix summary in the PR description rather than commenting on the issue -- the `Closes #` directive will auto-close the issue when merged. - -```bash -gh pr create \ - --title "fix(security): " \ - --label "topic:security" \ - --body "$(cat <<'EOF' -> **πŸ”§ security-fix-agent** - -Closes # - -## Security Fix - -### Summary -<1-3 sentences describing the security issue and how it was fixed> - -### Severity Assessment -- **Impact:** -- **Exploitability:** -- **Affected components:** - -### Changes Made -- ``: -- ``: - -### Tests Added -- **Unit:** -- **Integration/E2E:** - -### Documentation Updated -- ``: - -### Verification - -EOF -)" -``` - -**Display the PR URL** so it's easily clickable: - -``` -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: - -```bash -gh issue edit --remove-label "agent:in-progress" --add-label "agent:pr-opened" -``` - -## Step 9: Report to User - -Summarize what was done: - -1. Which issue was addressed and link to it -2. What the vulnerability was -3. What changes were made (files, approach) -4. What tests were added and at which level (unit, integration, e2e) -5. What documentation was updated -6. Link to the PR - -## Useful Commands Reference - -| Command | Description | -| --- | --- | -| `gh issue list --label "topic:security" --label "agent:implementation-requested" --state open` | Find security issues whose fixes a human requested | -| `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 | -| `gh api user --jq '.login'` | Get current GitHub username | -| `gh issue view ` | View issue details | -| `mise run pre-commit` | Run pre-commit checks | - -## Example Usage - -### Fix a specific issue - -User says: "Fix security issue #42" - -1. Fetch issue #42 and its comments -2. Find the `security-review-agent` review with determination "Legitimate concern" -3. Extract the remediation plan (e.g., add input sanitization to API handler) -4. Create branch `fix/security-42-input-sanitization` -5. Implement the fix -6. Add unit tests for the sanitization function and an integration test for the endpoint -7. Update affected documentation per `AGENTS.md`, if needed -8. Commit, push, and open PR with `Closes #42` -9. Report the PR link and changes to the user - -### Scan and fix requested security issues - -User says: "Fix any ready security issues" - -1. Query for open issues with labels `topic:security` + `agent:implementation-requested` -2. Find issue #78: "SQL injection in search endpoint" -3. Fetch the review comment -- determination is "Legitimate concern" -4. Implement parameterized queries -5. Add `test_rejects_sql_injection_in_search_query` unit test and e2e test for the search endpoint -6. Update affected documentation per `AGENTS.md`, if needed -7. Commit, push, open PR with `Closes #78`, report to user - -### Issue with non-actionable review - -User says: "Fix security issue #99" - -1. Fetch issue #99 and its comments -2. Find the `security-review-agent` review with determination "Not actionable" -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` - -User says: "Fix security issue #55" - -1. Fetch issue #55 metadata -2. Labels are `["topic:security"]` -- missing `agent:implementation-requested` -3. Confirm that a legitimate security review and remediation plan exist -4. Warn that `agent:implementation-requested` 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 - -User says: "Fix security issue #60" - -1. Fetch issue #60 metadata -- `topic:security` is present and the user directly requested the fix -2. Fetch comments -- no `security-review-agent` comment found -3. Report to the user: "Issue #60 has not been reviewed yet. Run the review-security-issue skill first." -4. Stop +Begin any fix comments with `> **πŸ”§ security-fix-agent**`. Do not change human disposition or introduce `agent:*` workflow labels. diff --git a/.agents/skills/helm-dev-environment/SKILL.md b/.agents/skills/helm-dev-environment/SKILL.md index cb3023f476..932d0d905d 100644 --- a/.agents/skills/helm-dev-environment/SKILL.md +++ b/.agents/skills/helm-dev-environment/SKILL.md @@ -83,7 +83,10 @@ capability-free workload Pod and a directly managed capability-free supervisor Pod. One namespace-wide NetworkPolicy denies direct egress from every OpenShell workload Pod. The `pkiInitJob` hook (a pre-install Job that runs `openshell-gateway generate-certs`) -generates mTLS secrets on first install. The default Skaffold values export +generates gateway and CLI TLS secrets on first install. Supervisor Pods project +only `ca.crt` and authenticate gateway RPCs with sandbox bearer tokens. User +client certificates and private keys remain outside supervisor and workload Pods. +The default Skaffold values export gateway and Kubernetes-driver traces to the collector service installed by `helm:k3s:create`. Envoy Gateway is opt-in; see the Optional Add-ons section. diff --git a/.agents/skills/launch-openshell-gator/SKILL.md b/.agents/skills/launch-openshell-gator/SKILL.md index 80a84ca744..2f9680d411 100644 --- a/.agents/skills/launch-openshell-gator/SKILL.md +++ b/.agents/skills/launch-openshell-gator/SKILL.md @@ -158,7 +158,7 @@ sandbox_name="gator-pr-${pr_number}-supervised" "Review and monitor PR #${pr_number} through the gator-gate workflow. Scope this invocation only to PR #${pr_number}." ``` -The launcher queries the gateway's selected compute driver, builds the gator image in the matching Docker or Podman image store, stages the immutable payload, imports provider profiles, configures provider credentials and refresh, and starts the agent supervisor as the sandbox's canonical main process. The detached main process survives loss of the host CLI connection and reconnects to a restarted gateway. Unless `--keep` is set, the sandbox is marked ephemeral so the gateway deletes it after the supervisor exits. `CONTAINER_ENGINE`, when set, must match the gateway driver. +The launcher queries the gateway's selected compute driver, builds the gator image in the matching Docker or Podman image store, stages the immutable payload, imports provider profiles, configures provider credentials and refresh, and starts the agent supervisor as the sandbox's canonical main process. The detached main process survives loss of the host CLI connection and reconnects to a restarted gateway. Unless `--keep` is set, the sandbox is marked ephemeral so the gateway deletes it after the canonical main process exits and its terminal result is finalized. `CONTAINER_ENGINE`, when set, must match the gateway driver. The launcher streams image-build and provisioning output until the detached workload is ready, then exits. Use `openshell logs ` or the TUI for runtime output. @@ -217,7 +217,7 @@ sandbox_name="gator-pr-${pr_number}-supervised" --gateway "$gateway_name" \ --name "$sandbox_name" \ --watch \ - "Review and monitor PR #${pr_number} through the gator-gate workflow. Scope this invocation only to PR #${pr_number}. The operator explicitly authorizes applying the test:e2e label, posting /ok to test for the current head SHA, and rerunning the relevant current-head workflow when the E2E Label Help bot says that is required." + "Review and monitor PR #${pr_number} through the gator-gate workflow. Scope this invocation only to PR #${pr_number}. The operator explicitly authorizes applying the test:e2e label, posting /ok to test with the full 40-character current head SHA, and rerunning the relevant current-head workflow when the E2E Label Help bot says that is required." ``` ## Model Or Image Experiments diff --git a/.agents/skills/review-security-issue/SKILL.md b/.agents/skills/review-security-issue/SKILL.md index efb054df80..0a014a62b5 100644 --- a/.agents/skills/review-security-issue/SKILL.md +++ b/.agents/skills/review-security-issue/SKILL.md @@ -1,196 +1,19 @@ --- name: review-security-issue -description: Given a GitHub issue, review the issue for security implications. You'll make a determination if the claim in the issue is legitimate and should be addressed or will be a "won't fix." Trigger keywords - security issue, review security ticket, review security issue. +description: Review an authorized security issue for validity, severity, and a remediation plan. metadata: internal: true --- # Review Security Issue -Review an issue that outlines a security, vulnerability, or privacy concern. +Review a security concern through its authorized private workflow. Do not file or expand a vulnerability in a public issue; follow `SECURITY.md`. A direct request to review authorizes review only, not remediation. For unattended review, inspect current `state:*` label descriptions, maintainer assignments, and comments to verify that review is authorized. -## Prerequisites +## Assess -- 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. +1. Fetch the issue and comments with `gh issue view --json title,body,state,labels,comments`. Inspect current repository labels rather than assuming exact names. Verify that this is an authorized security issue and that a prior review does not already answer the request. +2. Inspect affected code and verify the claim. Assess impact, exploitability, prerequisites, affected surface, and a concrete attack scenario. Separate evidence from assumptions and give a severity with rationale. +3. If actionable, propose a remediation plan with code areas, safe rollout, and focused tests. If not actionable, explain the evidence and recommended disposition. Do not decide product acceptance or silently close the issue. +4. Post the review only when the request authorizes posting. Begin the comment with `> **πŸ”’ security-review-agent**` so later reviews can detect it. Keep sensitive details in the authorized private venue. -## Agent Comment Marker - -All comments posted by this skill **must** begin with the following marker line so that prior reviews can be detected and human comments can be distinguished from agent comments: - -``` -> **πŸ”’ security-review-agent** -``` - -This marker is used in Step 2 to detect prior reviews and in Step 5 to distinguish agent comments from human comments. - -## Step 1: Fetch the Issue - -The user will provide an issue ID (e.g., `#42` or `42`). Strip any leading `#` and fetch the issue contents. - -```bash -gh issue view -``` - -To also retrieve the full issue body as JSON (useful for parsing): - -```bash -gh issue view --json title,body,state,labels,author -``` - -## Step 2: Check if Review is Needed - -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 `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. - -Next, fetch existing comments on the issue: - -```bash -gh issue view --json comments --jq '.comments[].body' -``` - -Search the comments for the agent marker (`> **πŸ”’ security-review-agent**`). - -- **If the marker is found** and no subsequent human comments exist that ask follow-up questions or challenge the review, you are done. Report to the user that a review already exists. -- **If the marker is found** but there are newer human comments with questions or objections, proceed to Step 5 to address them. -- **If the marker is not found**, proceed to Step 3. - -## Step 3: Analyze the Issue - -Pass the issue title, description, and any relevant code references to the `principal-engineer-reviewer` sub-agent for analysis. Use the Task tool: - -``` -Task tool with subagent_type="principal-engineer-reviewer" -``` - -In the prompt, instruct the reviewer to approach the issue with a security-focused lens, specifically evaluating: - -- **Validity**: Is this a real security, vulnerability, or privacy concern? -- **Severity**: What is the potential impact (data exposure, privilege escalation, denial of service, etc.)? -- **Exploitability**: How easy is it to exploit? Does it require authentication, specific conditions, or access? -- **Attack scenario**: What are the concrete steps an attacker would take to exploit this, from their perspective? -- **Affected surface**: Which components, endpoints, or code paths are affected? -- **Recommendation**: Should this be fixed, mitigated, accepted as risk, or closed as not actionable? - -## Step 4: Post the Review - -Based on the analysis from Step 3, post a comment on the issue. - -### If the concern is legitimate - -Post a comment with a remediation plan: - -```bash -gh issue comment --body "$(cat <<'EOF' -> **πŸ”’ security-review-agent** - -## Security Review - -**Determination:** Legitimate concern - -### Summary -<1-3 sentences describing the security issue and its impact> - -### Severity Assessment -- **Impact:** -- **Exploitability:** -- **Affected components:** - -### Attack Scenario -Step-by-step from the attacker's perspective: -1. -2. -3. - -### Remediation Plan -1. -2. -3. ... - -### Additional Notes - -EOF -)" -``` - -### If the concern is not actionable - -Post a comment with a rationale: - -```bash -gh issue comment --body "$(cat <<'EOF' -> **πŸ”’ security-review-agent** - -## Security Review - -**Determination:** Not actionable - -### Rationale - - -### References - -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: - -```bash -gh issue edit --remove-label "agent:plan-requested" --add-label "agent:plan-ready" -``` - -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. - -## Step 6: Address Follow-up Comments - -After posting (or if a prior review exists with new human comments), review all comments that do **not** contain the `> **πŸ”’ security-review-agent**` marker. These are human comments. - -For each unanswered human comment: - -1. Read the question or objection. -2. Formulate a response based on the codebase and the prior security analysis. -3. Post a reply that begins with the agent marker. - -**Important:** The authenticated user posting these comments may be a real person's account. Humans may reply to your comments directly. Always use the agent marker to distinguish your comments from theirs. - -## Useful Commands Reference - -| Command | Description | -| --- | --- | -| `gh issue view ` | View issue details | -| `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 | - -## Example Usage - -### Review a security issue - -User says: "Review security issue #42" - -1. Fetch issue #42 via `gh issue view 42` -2. Fetch comments and check for the `security-review-agent` marker -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 -7. Report the finding and posted comment to the user - -### Re-review with new comments - -User says: "Check on security issue #42 again" - -1. Fetch issue #42 and its comments -2. Find existing `security-review-agent` review from a prior run -3. Detect two new human comments asking about scope of the vulnerability -4. Post responses to each, prefixed with the agent marker -5. Report to the user what was addressed +A human decides whether to authorize remediation. Route an authorized fix to `fix-security-issue`. Do not introduce `agent:*` workflow labels. diff --git a/.agents/skills/sync-agent-infra/SKILL.md b/.agents/skills/sync-agent-infra/SKILL.md index 68a623c392..f16c171bea 100644 --- a/.agents/skills/sync-agent-infra/SKILL.md +++ b/.agents/skills/sync-agent-infra/SKILL.md @@ -1,214 +1,48 @@ --- name: sync-agent-infra -description: Detect and fix drift across agent-first infrastructure files. Ensures skill inventories, workflow chains, architecture tables, issue/PR templates, and cross-references stay consistent when skills, crates, or workflows change. Run after adding, removing, or renaming skills or components. Trigger keywords - sync agent infra, sync skills, update agent docs, check agent consistency, agent infra drift, sync contributing, sync agents. +description: Reconcile contributor skills, AGENTS.md, CONTRIBUTING.md, issue and PR templates, and workflow references after repository workflow changes. metadata: internal: true --- # Sync Agent Infrastructure -Detect and fix drift across the agent-first infrastructure files. These files reference each other and must stay consistent: +Keep contributor guidance consistent without copying procedural workflows into `AGENTS.md`. That file holds repository coding conventions and pointers; the relevant skills hold issue, PR, and maintenance procedures. `CONTRIBUTING.md` explains the human workflow. -| File | What it tracks | -|------|---------------| -| `AGENTS.md` | Project identity, workflow chains, architecture overview, issue/PR conventions, skill maintenance pointer | -| `CONTRIBUTING.md` | Skills table, workflow chains, "When to Open an Issue" guidance, skill references | -| `CONTRIBUTING.md` issue lifecycle section | Human-facing issue states, roadmap decisions, acceptance signals, and direct-versus-queued agent ownership | -| `README.md` | "Use OpenShell with Your Agent" and "Built With Agents" sections | -| `.github/ISSUE_TEMPLATE/bug_report.yml` | Skill name references in diagnostic guidance | -| `.github/ISSUE_TEMPLATE/feature_request.yml` | Skill name references in investigation guidance | -| `.github/ISSUE_TEMPLATE/config.yml` | Contact link text referencing skills | -| `.github/workflows/issue-triage.yml` | Comment text referencing skills | -| `.agents/skills/triage-issue/SKILL.md` | Skill name references in gate check and diagnosis steps | -| `skills/*/SKILL.md` | Standalone user instructions and links to documentation, included files, and related skills | -| `.agents/skills/create-github-pr/SKILL.md` | Pre-PR agent infrastructure check | -| `.agents/skills/review-github-pr/SKILL.md` | Review-time agent infrastructure check | -| `.agents/skills/build-from-issue/SKILL.md` | Label awareness and pre-commit agent infrastructure check | -| `.claude/agents/principal-engineer-reviewer.md` | Shared review-time agent infrastructure check | +## When to run -## When to Run +Run after adding, removing, or renaming a skill or crate; changing issue or PR conventions; changing development workflows; or modifying templates or agent cross references. Run before opening a PR that touches these areas. -- After adding, removing, renaming, or moving a skill in `skills/` or `.agents/skills/` -- After adding, removing, or renaming a crate in `crates/` -- After changing workflow chain relationships between skills -- After changing which product or development areas a skill covers -- After modifying issue or PR templates -- Before opening a PR that touches any of the above +## Maintenance map -## Skill Maintenance Map - -Use this map when product behavior, commands, or development workflows change. It is a routing aid, not an exhaustive dependency list. Search both `skills/` and `.agents/skills/` for the changed command, field, component, or workflow before concluding that no other skill needs an update. - -| Change area | Skills to review | +| Change | Skills to inspect | |---|---| +| Issues, triage, labels, or feature proposals | `create-github-issue`, `triage-issue`, `create-spike`, `build-from-issue` | +| PR template, vouch behavior, or review conventions | `create-github-pr`, `review-github-pr`, `build-from-issue` | +| Security assessment or remediation | `review-security-issue`, `fix-security-issue` | +| Published docs workflow | `update-docs-from-commits` | | CLI commands, flags, defaults, or workflows | `openshell-cli` | -| Sandbox policy schema, presets, or enforcement behavior | `generate-sandbox-policy`, `openshell-cli` | -| Supervisor middleware policy, registrations, runtime, or failure behavior | `generate-sandbox-policy`, `openshell-cli`, `debug-openshell-cluster` | -| Gateway deployment, Helm, runtime drivers, or health checks | `debug-openshell-cluster`, `helm-dev-environment` | -| Inference providers, native model endpoints, or migration from the retired managed endpoint | `debug-inference`, `openshell-cli`, `generate-sandbox-policy` | -| TUI architecture, navigation, data fetching, or UX | `tui-development` | -| Release artifacts or post-publish smoke coverage | `test-release-canary` | -| GitHub Actions workflows, required checks, or CI diagnostics | `watch-github-actions`; also `test-release-canary` for release smoke coverage | -| Gator harness, sandbox image, supervision, or model overrides | `launch-openshell-gator` | -| SBOM generation, dependency metadata, or license workflows | `sbom` | -| Issue templates, labels, contribution gates, or spike/build workflow | `triage-issue`, `create-spike`, `build-from-issue`, `create-github-issue` | -| PR template, review conventions, or vouch behavior | `create-github-pr`, `review-github-pr`, `build-from-issue` | -| Security review or remediation workflow | `review-security-issue`, `fix-security-issue` | -| RFC template, numbering, or lifecycle | `create-rfc` | -| Documentation structure, navigation, or doc-update workflow | `update-docs-from-commits` | -| Skills, crates, workflow chains, issue/PR templates, or agent cross-references | `sync-agent-infra` | - -## Prerequisites - -You must be in the OpenShell repository root. - -## Step 1: Inventory Current State - -Gather the source of truth for each category. - -### Skills - -List public and contributor skill directories separately: - -```bash -ls -1 skills/ -ls -1 .agents/skills/ -``` - -The directories are canonical by audience: `skills/` contains public, installable user/operator skills and `.agents/skills/` contains internal contributor workflows. Every other file must agree with both inventories. - -### Crates - -List all crate directories: - -```bash -ls -1 crates/ -``` - -### Workflow Chains - -The canonical workflow chains are defined in `AGENTS.md` under "## Workflow Chains". Read that section β€” it is the source of truth for skill pipelines. - -### Labels - -The canonical label set is used by skills and templates. The key labels are: `state:triage-needed`, `state:needs-info`, `state:validated`, `state:accepted`, `agent:plan-requested`, `agent:plan-ready`, `agent:implementation-requested`, `agent:in-progress`, `agent:pr-opened`, `roadmap`, `topic:security`, `good first issue`, `help wanted`, `spike`, and the relevant `area:*`, `topic:*`, `integration:*`, and `test:*` labels. Lifecycle and `agent:*` request labels gate unattended queue pickup. They do not prevent a direct user request: the agent warns about each missing or incomplete expected workflow label and continues with the requested phase without changing those labels. - -## Step 2: Check Each File for Drift - -For each file in the table above, check for the following inconsistencies: - -### `CONTRIBUTING.md` - -1. **Public skills table** β€” Every skill in `skills/` must appear in "Skills for Using OpenShell" and no contributor skill may appear there. -2. **Contributor skills table** β€” Every skill in `.agents/skills/` must appear in "Agent Skills for Contributors" and no public skill may appear there. -3. **Inventory paths** β€” No skill in either table should reference a directory that does not exist. -4. **Workflow chains** β€” Must match `AGENTS.md` workflow chains exactly. -5. **Skill references in prose** β€” Any named skill must exist in exactly one canonical skill directory. - -### `AGENTS.md` - -1. **Architecture overview** β€” Every crate in `crates/` must appear in the architecture table. The `python/`, `proto/`, `deploy/`, `.agents/` rows must also be present. -2. **Skill layout** β€” The architecture table must contain separate `skills/` and `.agents/skills/` rows with accurate audience descriptions. -3. **Workflow chains** β€” Verify each skill named in a chain exists in exactly one of the two skill directories. -4. **Issue/PR conventions** β€” Verify referenced skills (`create-github-issue`, `create-github-pr`, `build-from-issue`) exist. -5. **Skill maintenance pointer** β€” Verify it still points to `sync-agent-infra` and does not duplicate the maintenance map from this skill. - -### Issue Lifecycle Documentation - -1. **`CONTRIBUTING.md` issue lifecycle section** β€” State, roadmap, acceptance-signal, and agent-workflow meanings must match `AGENTS.md`. -2. **Invocation modes** β€” Lifecycle and `agent:*` request labels must gate unattended queue pickup without blocking a direct user request to a specific agent. -3. **Direct-mode warnings** β€” Guidance must require the agent to warn about each missing or incomplete expected workflow label, continue with the requested phase, and leave labels unchanged. - -### `README.md` - -1. **Public installation guidance** β€” The README must distinguish `skills/` from `.agents/skills/`, include `npx skills add NVIDIA/OpenShell`, and list only canonical public skills as installable. -2. **"Built With Agents"** β€” Contributor skill names must exist under `.agents/skills/`. Workflow descriptions should be consistent with `AGENTS.md` chains. - -### Issue Templates - -1. **`bug_report.yml`** β€” Must collect a User Story, Problem Statement, Impact / Why This Matters, Acceptance Criteria, Reproduction Steps, and Environment. Logs are optional and bug-specific; reporter diagnostics must not be required. -2. **`feature_request.yml`** β€” Must collect a User Story, Problem Statement, Impact / Why This Matters, Proposed Design, Acceptance Criteria, and Alternatives Considered. The design describes workflow and observable behavior without prescribing internal implementation; agent investigation is optional. -3. **`config.yml`** β€” Skill category descriptions in contact links should be accurate. - -### Issue Triage Workflow - -1. **`issue-triage.yml`** β€” Skill names in the redirect comment must exist. - -### Skill Cross-References - -1. **`triage-issue`** β€” Skills referenced in gate check and diagnosis steps must exist. -2. **`openshell-cli`** β€” Companion skills table entries must exist in one canonical location. -3. **`build-from-issue`** β€” Label names must match the project's label taxonomy. Lifecycle and request labels must gate unattended queue pickup, while direct requests warn on workflow discrepancies and continue. -4. **`create-spike`** β€” Reference to `build-from-issue` as next step must be accurate. -5. **`review-security-issue`** / **`fix-security-issue`** β€” Cross-references between the two must be accurate. -6. **PR creation and review checks** β€” The `create-github-pr`, `review-github-pr`, `build-from-issue`, and `principal-engineer-reviewer` references to `sync-agent-infra` must exist and use trigger conditions aligned with this skill. - -### Skill Layout, Metadata, and Portability - -1. **Placement** β€” The four public skills (`openshell-cli`, `generate-sandbox-policy`, `debug-inference`, and `debug-openshell-cluster`) must live only in `skills/`. Every other repository skill must live only in `.agents/skills/`. -2. **Internal metadata** β€” Every `.agents/skills/*/SKILL.md` must set `metadata.internal: true`. Public skills must not set internal metadata. Treat this as a discovery filter, not an access-control boundary. -3. **Unique names** β€” Parse the `name` field from every `SKILL.md` under both roots. Every name must be globally unique and match the documented inventory. -4. **Local references** β€” Every relative Markdown link and referenced file in a skill must resolve within that installed skill directory unless the reference is an explicit published URL. -5. **Canonical paths** β€” Contributor skills that name the source location of a public skill must use `skills//...`, never `.agents/skills//...`. -6. **Public portability** β€” Public skills must not require repository-relative files under `docs/`, `crates/`, `deploy/`, or `.agents/`; source builds; `mise`; or repository E2E workflows. Use installed `openshell --help` for command syntax and Markdown endpoints under `https://docs.nvidia.com/openshell/latest/` (URLs ending in `.md`) for product documentation. -7. **No canonical documentation copies** β€” Review public reference files and large command/schema blocks. Remove material that merely copies CLI help, policy schemas, RFCs, or published operational documentation; retain only skill-specific reasoning and worked interactions. -8. **Discovery** β€” Run `npx -y skills add . --list` from a clean checkout or disposable copy. It must list exactly the four public skills. Remove any generated lock file or installed directory after the check. - -## Step 3: Report Drift - -If any inconsistencies are found, report them in a structured format: - -```markdown -## Agent Infrastructure Drift Report - -### Skills Inventory -- PUBLIC ADDED (exists in skills/ but missing from CONTRIBUTING.md): -- PUBLIC REMOVED (documented as public but missing from skills/): -- CONTRIBUTOR ADDED (exists in .agents/skills/ but missing from CONTRIBUTING.md): -- CONTRIBUTOR REMOVED (documented as contributor but missing from .agents/skills/): -- METADATA/PATH/NAME ERRORS: -- OK: public and contributor skills consistent - -### Architecture Table -- ADDED (exists in crates/ but missing from AGENTS.md): -- REMOVED (in AGENTS.md but missing from crates/): -- OK: components consistent - -### Workflow Chains -- STALE: references non-existent skill -- OK: chains consistent - -### Cross-References -- : references non-existent skill -- : references non-existent label