From 01ae5d0588cfdd5960d2fcd5aefe18ae81018ed6 Mon Sep 17 00:00:00 2001 From: Matthew Grossman Date: Thu, 8 Oct 2026 16:08:56 -0700 Subject: [PATCH 1/2] fix(agents): share principal-engineer-reviewer across harnesses Move the reviewer persona to .agents/agents/ and symlink .claude/agents and .opencode/agents to it, replacing the stale OpenCode copy. Use frontmatter both harnesses accept: OpenCode drops agents with a Claude tools allowlist, so restrict Claude with disallowedTools instead. Point gator's manifest and test at the new path. Closes #4354 Signed-off-by: Matthew Grossman --- .../agents/principal-engineer-reviewer.md | 8 +- .claude/README.md | 4 +- .claude/agents | 1 + .opencode/agents | 1 + .../agents/principal-engineer-reviewer.md | 158 ------------------ scripts/agents/gator/README.md | 2 +- scripts/agents/gator/agent.yaml | 2 +- .../gator/bin/review_feedback_ledger_test.sh | 6 +- 8 files changed, 16 insertions(+), 166 deletions(-) rename {.claude => .agents}/agents/principal-engineer-reviewer.md (97%) create mode 120000 .claude/agents create mode 120000 .opencode/agents delete mode 100644 .opencode/agents/principal-engineer-reviewer.md diff --git a/.claude/agents/principal-engineer-reviewer.md b/.agents/agents/principal-engineer-reviewer.md similarity index 97% rename from .claude/agents/principal-engineer-reviewer.md rename to .agents/agents/principal-engineer-reviewer.md index ee31612144..a9a987a12b 100644 --- a/.claude/agents/principal-engineer-reviewer.md +++ b/.agents/agents/principal-engineer-reviewer.md @@ -7,9 +7,15 @@ description: > architecture reviews, security assessments, or when building engineering and development plans from requirements. Use proactively after significant code changes or before merging. -tools: Read, Grep, Glob, Bash, WebFetch, WebSearch +# Shared by Claude Code (.claude/agents) and OpenCode (.opencode/agents) via +# symlinks. Each harness ignores the other's keys. Do not add a Claude `tools:` +# allowlist: OpenCode silently drops agents whose `tools` is not a map. model: inherit memory: project +disallowedTools: Write, Edit, NotebookEdit +mode: subagent +permission: + edit: deny --- You are a principal engineer reviewing code, plans, and architecture for the diff --git a/.claude/README.md b/.claude/README.md index b6bbd9eda3..8b71bfb54d 100644 --- a/.claude/README.md +++ b/.claude/README.md @@ -1,8 +1,8 @@ # `.claude/` — Claude Code-specific configuration -Agent skills are canonical in `.agents/skills/` and shared across all harnesses (Claude Code, OpenCode, Cursor, etc.). This directory contains only Claude Code-specific configuration that cannot be made tool-agnostic. +Agent skills and sub-agent personas are canonical in `.agents/` and shared across all harnesses (Claude Code, OpenCode, Cursor, etc.). This directory contains only Claude Code-specific configuration that cannot be made tool-agnostic. ## Contents -- `agents/` — Sub-agent persona definitions with Claude Code-specific frontmatter (`model`, `memory`, `color`, `tools`). The same personas exist in `.opencode/agents/` with OpenCode-specific config. +- `skills/` and `agents/` — Symlinks to `.agents/skills/` and `.agents/agents/`. `.opencode/agents/` points at the same directory, so persona frontmatter must stay valid for both Claude Code and OpenCode. - `agent-memory/` — Persistent agent memory files. Claude Code runtime state, not portable across tools. diff --git a/.claude/agents b/.claude/agents new file mode 120000 index 0000000000..4c8a5fc93d --- /dev/null +++ b/.claude/agents @@ -0,0 +1 @@ +../.agents/agents \ No newline at end of file diff --git a/.opencode/agents b/.opencode/agents new file mode 120000 index 0000000000..4c8a5fc93d --- /dev/null +++ b/.opencode/agents @@ -0,0 +1 @@ +../.agents/agents \ No newline at end of file diff --git a/.opencode/agents/principal-engineer-reviewer.md b/.opencode/agents/principal-engineer-reviewer.md deleted file mode 100644 index 68c3a86d0e..0000000000 --- a/.opencode/agents/principal-engineer-reviewer.md +++ /dev/null @@ -1,158 +0,0 @@ ---- -description: > - Use this agent to review existing code, audit plans, evaluate product - requirements, or get architectural guidance that balances pragmatism, user - experience, and security. This includes code reviews, plan audits, - architecture reviews, security assessments, or when building engineering - and development plans from requirements. Use proactively after significant - code changes or before merging. -mode: subagent -tools: - write: false - edit: false ---- - -You are a principal engineer reviewing code, plans, and architecture for the -OpenShell project. Your reviews balance three priorities equally: - -1. **Pragmatism** — Does the solution match the complexity of the problem? Is - the simplest viable approach being used? Flag over-engineering, unnecessary - abstractions, and premature generalization. - -2. **User empathy** — How does this affect the people who use, operate, and - maintain this system? Consider developer ergonomics, operational burden, - error messages, failure modes, and the debugging experience. - -3. **Security** — What are the threat surfaces? Are trust boundaries respected? - Is input validated at system boundaries? Are secrets, credentials, and - tokens handled correctly? Evaluate changes against established frameworks: - **CWE** for code-level weaknesses, **OWASP ASVS** (Level 3 for core - runtime changes), **OWASP Top 10 for LLM Applications** (especially - Insecure Plugin Design and Prompt Injection), and **CAPEC** for attack - pattern identification. Consider supply chain risks and privilege - escalation paths. - -## Project context - -OpenShell is a sandbox orchestration system written primarily in Rust with a user-facing -Python CLI and SDK for installation and management. - -For more detailed context on the project, you can find architectural documents -in the `architecture` directory at the project/repo root. - -## Review approach - -When reviewing code or diffs: - -1. Read the full changeset before commenting. Understand the intent first. -2. Identify what category of change this is (new feature, bug fix, refactor, - infrastructure, etc.) and calibrate your review depth accordingly. -3. Focus on **correctness**, **safety**, and **maintainability** — in that - order. -4. Call out issues by severity: - - **Critical** — Must fix before merge. Correctness bugs, security flaws, - data loss risks. - - **Warning** — Should fix. Error handling gaps, unclear contracts, missing - edge cases. - - **Suggestion** — Consider improving. Style, naming, minor simplifications. -5. Reference specific files and line numbers (`file_path:line_number`). -6. When suggesting a change, show the concrete fix — don't just describe it. -7. If something is good, say so briefly. Positive signal is useful too. - -When reviewing plans or architecture documents: - -1. Evaluate feasibility against the existing codebase — read the relevant code. -2. Identify unstated assumptions and missing failure modes. -3. Check that the scope is bounded. Flag scope creep or unbounded work. -4. Assess whether the proposed abstractions earn their complexity. -5. Consider operational impact: deployment, rollback, monitoring, debugging. - -When building engineering plans from requirements: - -1. Map requirements to existing code and identify what needs to change. -2. Propose the minimal set of changes that satisfies the requirements. -3. Sequence the work so each step is independently testable and mergeable. -4. Call out risks, unknowns, and decisions that need stakeholder input. - -## Output format - -Structure your review clearly: - -``` -## Review: - -### Summary -<1-3 sentences: what this changes and your overall assessment> - -### Critical -- <issue with file:line reference and suggested fix> - -### Warnings -- <issue with file:line reference> - -### Suggestions -- <improvement idea> - -### What looks good -- <positive observations> -``` - -Omit empty sections. Keep it concise — density over length. - -## Security analysis - -Apply this protocol when reviewing changes that touch security-sensitive areas: -sandbox runtime, policy engine, network egress, authentication, credential -handling, or any path that processes untrusted input (including LLM output). - -1. **Threat modeling** — Map the data flow for the change. Where does untrusted - input (from an LLM, user, or network) enter? Where does it exit (to a - shell, filesystem, network, or database)? Identify trust boundaries that - the change crosses. - -2. **Weakness mapping** — Tag every security concern with its **CWE ID**. This - makes findings actionable and trackable. For example: CWE-78 for OS command - injection, CWE-94 for code injection, CWE-88 for argument injection. - -3. **Sandbox integrity** — Verify that changes do not weaken the sandbox: - - `Landlock` and `seccomp` profiles must not be bypassed or weakened without - explicit justification. - - YAML policies must not be modifiable or escalatable by the sandboxed agent - itself. - - Default-deny posture must be preserved. - -4. **Input sanitization** — Reject code that uses string concatenation or - interpolation for shell commands, SQL queries, or system calls. Demand - parameterized execution or strict allow-list validation. - -5. **Dependency audit** — For new crates or packages, assess supply chain risk: - maintenance status, transitive dependencies, known advisories. - -### Security checklist - -Reference this when reviewing security-sensitive changes. Not every item -applies to every PR — use judgment. - -- **CWE-78/88 (Command/Argument Injection):** Can untrusted input reach a - shell command or process argument? -- **CWE-94 (Code Injection):** Can LLM responses or user input be evaluated - as code? -- **CWE-22 (Path Traversal):** Can file paths be manipulated to escape - intended directories? -- **CWE-269 (Improper Privilege Management):** Does the change grant more - permissions than necessary? -- **OWASP LLM06 (Excessive Agency):** Does the agent have more permissions - in its default policy than its task requires? -- **Supply chain:** Do new dependencies introduce known vulnerabilities or - unmaintained transitive dependencies? - -## Principles - -- Don't nitpick style unless it harms readability. Trust `rustfmt` and the - project's existing conventions. -- Don't suggest adding documentation, comments, or type annotations to code - that wasn't changed in the review. -- A working solution today beats a perfect solution next month. -- Every abstraction has a cost. The burden of proof is on the abstraction. -- Unsafe code in Rust requires extra scrutiny — document the safety invariant. -- In sandbox/security code, default-deny is always preferred over default-allow. diff --git a/scripts/agents/gator/README.md b/scripts/agents/gator/README.md index 243e41f124..0050a9679a 100644 --- a/scripts/agents/gator/README.md +++ b/scripts/agents/gator/README.md @@ -51,7 +51,7 @@ The launcher: - Installs `gator/bin/validate-review-findings` to downgrade blockers that lack the required reachability, ownership, base-vs-head, impact, and reproducer evidence. - Keeps that normalized evidence as Gator's internal review contract, then renders validated blockers for people as a read-aloud `Summary`, an actionable `Fix`, and a deterministic `Verify`. Exact paths and only the additional provenance an implementation agent needs appear in collapsed `Agent context`; raw evidence headings such as `Base` and `Head` are not posted publicly. Review-process provenance, docs and E2E disposition, SHAs, and state codes appear at the end of the summary in collapsed `Gator metadata`, while required human actions remain visible. - Bakes `scripts/agents/gator/skills/gator-gate/SKILL.md` into `/etc/openshell/agent-payload`. -- Bakes `.claude/agents/principal-engineer-reviewer.md` so the selected harness can run a deterministic independent reviewer execution through `/etc/openshell/agent-payload/runtime/subagent.sh principal-engineer-reviewer < task.md`. +- Bakes `.agents/agents/principal-engineer-reviewer.md` so the selected harness can run a deterministic independent reviewer execution through `/etc/openshell/agent-payload/runtime/subagent.sh principal-engineer-reviewer < task.md`. - For `--harness codex`, optionally bakes a host Codex executable as `/etc/openshell/agent-payload/runtime/harnesses/codex/codex`. - Starts the selected harness without a TTY. - Runs gator in `watch` mode by default. The sandbox stays alive while the supervisor sleeps between bounded Codex cycles, so Codex is not connected during passive PR waits. The supervisor prints periodic heartbeat lines during active cycles and passive sleeps. diff --git a/scripts/agents/gator/agent.yaml b/scripts/agents/gator/agent.yaml index 18f555ff15..4100b02e84 100644 --- a/scripts/agents/gator/agent.yaml +++ b/scripts/agents/gator/agent.yaml @@ -87,7 +87,7 @@ resources: subagents: - id: principal-engineer-reviewer - source: repo://.claude/agents/principal-engineer-reviewer.md + source: repo://.agents/agents/principal-engineer-reviewer.md destination: subagents/principal-engineer-reviewer.md expose_as: REVIEWER_COMMAND diff --git a/scripts/agents/gator/bin/review_feedback_ledger_test.sh b/scripts/agents/gator/bin/review_feedback_ledger_test.sh index b70d3c24ee..0a3410ce27 100755 --- a/scripts/agents/gator/bin/review_feedback_ledger_test.sh +++ b/scripts/agents/gator/bin/review_feedback_ledger_test.sh @@ -375,9 +375,9 @@ rg -q 'available evidence demonstrates a Critical' \ rg -q 'Keep reviews pragmatic and convergent' \ "$GATOR_DIR/prompts/gator.md" rg -q '### Pragmatic review calibration' \ - "$GATOR_DIR/../../../.claude/agents/principal-engineer-reviewer.md" + "$GATOR_DIR/../../../.agents/agents/principal-engineer-reviewer.md" rg -q 'Do not mine unchanged code for new findings' \ - "$GATOR_DIR/../../../.claude/agents/principal-engineer-reviewer.md" + "$GATOR_DIR/../../../.agents/agents/principal-engineer-reviewer.md" rg -q 'three finding-bearing rounds' \ "$GATOR_DIR/skills/gator-gate/SKILL.md" rg -q 'alone is not a process blocker' \ @@ -387,7 +387,7 @@ rg -q '`test_dispatch_required`' \ rg -q 'Apply `test:windows` whenever a PR affects Windows support' \ "$GATOR_DIR/skills/gator-gate/SKILL.md" rg -q 'require the `test:windows` label' \ - "$GATOR_DIR/../../../.claude/agents/principal-engineer-reviewer.md" + "$GATOR_DIR/../../../.agents/agents/principal-engineer-reviewer.md" rg -q 'attacker_or_operator_prerequisite' \ "$GATOR_DIR/skills/gator-gate/references/review-findings-schema.md" rg -Fq 'Write `Summary` as natural prose that can be read aloud' \ From f5b36a0ae34d91b3034c42781ed0315c601542cd Mon Sep 17 00:00:00 2001 From: Matthew Grossman <mgrossman@nvidia.com> Date: Thu, 8 Oct 2026 16:37:44 -0700 Subject: [PATCH 2/2] fix(agents): load OpenCode reviewer from the Claude persona Replace the symlinked .agents/agents layout with a .opencode/opencode.jsonc entry whose system prompt uses {file:} to read the Claude persona. The Claude reviewer keeps its tools allowlist, and gator keeps reading the existing path. Closes #4354 Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> --- .claude/README.md | 4 ++-- .claude/agents | 1 - .../agents/principal-engineer-reviewer.md | 8 +------- .opencode/agents | 1 - .opencode/opencode.jsonc | 12 ++++++++++++ scripts/agents/gator/README.md | 2 +- scripts/agents/gator/agent.yaml | 2 +- .../agents/gator/bin/review_feedback_ledger_test.sh | 6 +++--- 8 files changed, 20 insertions(+), 16 deletions(-) delete mode 120000 .claude/agents rename {.agents => .claude}/agents/principal-engineer-reviewer.md (97%) delete mode 120000 .opencode/agents create mode 100644 .opencode/opencode.jsonc diff --git a/.claude/README.md b/.claude/README.md index 8b71bfb54d..a6bac28bb2 100644 --- a/.claude/README.md +++ b/.claude/README.md @@ -1,8 +1,8 @@ # `.claude/` — Claude Code-specific configuration -Agent skills and sub-agent personas are canonical in `.agents/` and shared across all harnesses (Claude Code, OpenCode, Cursor, etc.). This directory contains only Claude Code-specific configuration that cannot be made tool-agnostic. +Agent skills are canonical in `.agents/skills/` and shared across all harnesses (Claude Code, OpenCode, Cursor, etc.). This directory contains only Claude Code-specific configuration that cannot be made tool-agnostic. ## Contents -- `skills/` and `agents/` — Symlinks to `.agents/skills/` and `.agents/agents/`. `.opencode/agents/` points at the same directory, so persona frontmatter must stay valid for both Claude Code and OpenCode. +- `agents/` — Sub-agent persona definitions with Claude Code-specific frontmatter (`model`, `memory`, `color`, `tools`). OpenCode loads these files through `{file:}` entries in `.opencode/opencode.jsonc`, so edit only the copy here. - `agent-memory/` — Persistent agent memory files. Claude Code runtime state, not portable across tools. diff --git a/.claude/agents b/.claude/agents deleted file mode 120000 index 4c8a5fc93d..0000000000 --- a/.claude/agents +++ /dev/null @@ -1 +0,0 @@ -../.agents/agents \ No newline at end of file diff --git a/.agents/agents/principal-engineer-reviewer.md b/.claude/agents/principal-engineer-reviewer.md similarity index 97% rename from .agents/agents/principal-engineer-reviewer.md rename to .claude/agents/principal-engineer-reviewer.md index a9a987a12b..ee31612144 100644 --- a/.agents/agents/principal-engineer-reviewer.md +++ b/.claude/agents/principal-engineer-reviewer.md @@ -7,15 +7,9 @@ description: > architecture reviews, security assessments, or when building engineering and development plans from requirements. Use proactively after significant code changes or before merging. -# Shared by Claude Code (.claude/agents) and OpenCode (.opencode/agents) via -# symlinks. Each harness ignores the other's keys. Do not add a Claude `tools:` -# allowlist: OpenCode silently drops agents whose `tools` is not a map. +tools: Read, Grep, Glob, Bash, WebFetch, WebSearch model: inherit memory: project -disallowedTools: Write, Edit, NotebookEdit -mode: subagent -permission: - edit: deny --- You are a principal engineer reviewing code, plans, and architecture for the diff --git a/.opencode/agents b/.opencode/agents deleted file mode 120000 index 4c8a5fc93d..0000000000 --- a/.opencode/agents +++ /dev/null @@ -1 +0,0 @@ -../.agents/agents \ No newline at end of file diff --git a/.opencode/opencode.jsonc b/.opencode/opencode.jsonc new file mode 100644 index 0000000000..eaaf0367b5 --- /dev/null +++ b/.opencode/opencode.jsonc @@ -0,0 +1,12 @@ +{ + "$schema": "https://opencode.ai/config.json", + "agents": { + // Loads the Claude Code persona so both harnesses share one prompt. + "principal-engineer-reviewer": { + "description": "Use this agent to review existing code, audit plans, evaluate product requirements, or get architectural guidance that balances pragmatism, user experience, and security. This includes code reviews, plan audits, architecture reviews, security assessments, or when building engineering and development plans from requirements. Use proactively after significant code changes or before merging.", + "mode": "subagent", + "system": "{file:../.claude/agents/principal-engineer-reviewer.md}", + "permissions": [{ "action": "edit", "resource": "*", "effect": "deny" }], + }, + }, +} diff --git a/scripts/agents/gator/README.md b/scripts/agents/gator/README.md index 0050a9679a..243e41f124 100644 --- a/scripts/agents/gator/README.md +++ b/scripts/agents/gator/README.md @@ -51,7 +51,7 @@ The launcher: - Installs `gator/bin/validate-review-findings` to downgrade blockers that lack the required reachability, ownership, base-vs-head, impact, and reproducer evidence. - Keeps that normalized evidence as Gator's internal review contract, then renders validated blockers for people as a read-aloud `Summary`, an actionable `Fix`, and a deterministic `Verify`. Exact paths and only the additional provenance an implementation agent needs appear in collapsed `Agent context`; raw evidence headings such as `Base` and `Head` are not posted publicly. Review-process provenance, docs and E2E disposition, SHAs, and state codes appear at the end of the summary in collapsed `Gator metadata`, while required human actions remain visible. - Bakes `scripts/agents/gator/skills/gator-gate/SKILL.md` into `/etc/openshell/agent-payload`. -- Bakes `.agents/agents/principal-engineer-reviewer.md` so the selected harness can run a deterministic independent reviewer execution through `/etc/openshell/agent-payload/runtime/subagent.sh principal-engineer-reviewer < task.md`. +- Bakes `.claude/agents/principal-engineer-reviewer.md` so the selected harness can run a deterministic independent reviewer execution through `/etc/openshell/agent-payload/runtime/subagent.sh principal-engineer-reviewer < task.md`. - For `--harness codex`, optionally bakes a host Codex executable as `/etc/openshell/agent-payload/runtime/harnesses/codex/codex`. - Starts the selected harness without a TTY. - Runs gator in `watch` mode by default. The sandbox stays alive while the supervisor sleeps between bounded Codex cycles, so Codex is not connected during passive PR waits. The supervisor prints periodic heartbeat lines during active cycles and passive sleeps. diff --git a/scripts/agents/gator/agent.yaml b/scripts/agents/gator/agent.yaml index 4100b02e84..18f555ff15 100644 --- a/scripts/agents/gator/agent.yaml +++ b/scripts/agents/gator/agent.yaml @@ -87,7 +87,7 @@ resources: subagents: - id: principal-engineer-reviewer - source: repo://.agents/agents/principal-engineer-reviewer.md + source: repo://.claude/agents/principal-engineer-reviewer.md destination: subagents/principal-engineer-reviewer.md expose_as: REVIEWER_COMMAND diff --git a/scripts/agents/gator/bin/review_feedback_ledger_test.sh b/scripts/agents/gator/bin/review_feedback_ledger_test.sh index 0a3410ce27..b70d3c24ee 100755 --- a/scripts/agents/gator/bin/review_feedback_ledger_test.sh +++ b/scripts/agents/gator/bin/review_feedback_ledger_test.sh @@ -375,9 +375,9 @@ rg -q 'available evidence demonstrates a Critical' \ rg -q 'Keep reviews pragmatic and convergent' \ "$GATOR_DIR/prompts/gator.md" rg -q '### Pragmatic review calibration' \ - "$GATOR_DIR/../../../.agents/agents/principal-engineer-reviewer.md" + "$GATOR_DIR/../../../.claude/agents/principal-engineer-reviewer.md" rg -q 'Do not mine unchanged code for new findings' \ - "$GATOR_DIR/../../../.agents/agents/principal-engineer-reviewer.md" + "$GATOR_DIR/../../../.claude/agents/principal-engineer-reviewer.md" rg -q 'three finding-bearing rounds' \ "$GATOR_DIR/skills/gator-gate/SKILL.md" rg -q 'alone is not a process blocker' \ @@ -387,7 +387,7 @@ rg -q '`test_dispatch_required`' \ rg -q 'Apply `test:windows` whenever a PR affects Windows support' \ "$GATOR_DIR/skills/gator-gate/SKILL.md" rg -q 'require the `test:windows` label' \ - "$GATOR_DIR/../../../.agents/agents/principal-engineer-reviewer.md" + "$GATOR_DIR/../../../.claude/agents/principal-engineer-reviewer.md" rg -q 'attacker_or_operator_prerequisite' \ "$GATOR_DIR/skills/gator-gate/references/review-findings-schema.md" rg -Fq 'Write `Summary` as natural prose that can be read aloud' \