Skip to content

fix(agents): uuid guard + reviewer opus - #1623

Closed
namastex888 wants to merge 3 commits into
devfrom
wish/agent-yaml-permissions-wireup
Closed

namastex888 wants to merge 3 commits into
devfrom
wish/agent-yaml-permissions-wireup

Conversation

@namastex888

Copy link
Copy Markdown
Contributor

Summary

Two P0 unblockers landing as a quick PR. Genie está inútil sem isso — every direct subagent spawn was crashing on the UUID type check, and reviewer was pinned to haiku which doesn't support --permission-mode auto.

Changes

1. lookupTemplateTeam UUID guard (src/lib/agent-directory.ts)

Symptom: genie spawn <role> --team genie (engineer / fix / qa / etc.) crashed with lookupTemplateTeam(engineer) failed: invalid input syntax for type uuid: "engineer" then agents_id_shape_check violation, blocking every direct dispatch.

Root cause: Post retire-session-names-id-only, agent_templates.id is now UUID. The function received built-in role names and PG rejected them.

Fix: UUID-regex guard at function entry. Non-UUID names return null immediately, letting callers fall through to the next resolution tier (built-in roles / council members).

2. Reviewer model: haiku → opus

Felipe direct: haiku doesn't support --permission-mode auto. Reviewer runs every wish gate and must use opus for consistent tool gating. Audit confirms zero haiku/sonnet defaults remain in plugins/genie/agents/.

Validation

  • bun run typecheck clean
  • bun run lint 0 errors, 16 warnings (dev=15 +1 from in-flight engineer subagent's parallel work on the parent wish)

Test plan

  • Post-merge: genie spawn engineer --team genie from fresh shell exits 0 without UUID error
  • Post-merge: genie spawn fix --team genie exits 0
  • Post-merge: reviewer uses opus (verify via genie ls --json)

Scope context

Wider migration tracked in .genie/wishes/agent-yaml-permissions-wireup/WISH.md (executor permissions.preset → --allowedTools wire-through, default-agents audit, frontmatter→agent.yaml migration). Lands as follow-up PRs.

🤖 Generated with Claude Code

namastex888 and others added 2 commits May 3, 2026 17:39
Felipe directive ("we cant work without this"): the AGENTS.md frontmatter
→ agent.yaml migration was started but never wired through to the executor.
permissions.preset:full + sdk.allowedTools are silently ignored — workspace
agents launch under --permission-mode auto defaults that exclude Edit/Write.

Adds wire-through (P0) so agent.yaml is the real source of truth, fixes
genie spawn UUID regression (P0) blocking direct subagent dispatch, audits +
deletes dead default agents (P1), migrates remaining built-in role frontmatter
into agent.yaml (P1), and changes reviewer haiku→opus (P1) since haiku/sonnet
don't support --permission-mode auto.

Felipe agent.yaml updated as the canary — must have Edit/Write end-to-end
post-merge.

PR target: dev. Single micropr scope, 8 execution groups.

genie wish lint: clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two P0 unblockers. Genie está inútil sem isso.

1. lookupTemplateTeam UUID guard (src/lib/agent-directory.ts)
   `genie spawn engineer --team genie` (or any built-in role) crashed with
   `lookupTemplateTeam(engineer) failed: invalid input syntax for type uuid`
   then `agents_id_shape_check` violation, blocking every direct dispatch.

   Root cause: post retire-session-names, agent_templates.id is now UUID.
   The function received built-in role names and PG rejected them.

   Fix: UUID regex guard at function entry. Non-UUID names return null,
   letting callers fall through to next resolution tier (built-in roles).

2. Reviewer model: haiku to opus (plugins/genie/agents/reviewer/AGENTS.md)
   Felipe direct: haiku does not support --permission-mode auto. Reviewer
   runs every wish gate and must use opus. Audit confirms zero
   haiku/sonnet defaults remain in plugins/genie/agents/.

Validation: bun run typecheck clean. Lint: 0 errors, 16 warnings (dev=15
+1 from in-flight engineer subagent's parallel work on Groups 1-4 of the
parent wish — those land separately).

Wider scope (executor wire-through, agents audit, frontmatter migration)
in .genie/wishes/agent-yaml-permissions-wireup/WISH.md.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 3, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7340b8f3-d424-4912-a49e-1e131ede03bc

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch wish/agent-yaml-permissions-wireup

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 09724023ce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

.object({
allow: z.array(z.string()).optional(),
deny: z.array(z.string()).optional(),
allowedTools: z.array(z.string()).optional(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Forward permissions.allowedTools to Claude CLI

spawnParamsSchema now accepts permissions.allowedTools, but the value is never used when building the launch command: buildSettingsObject only serializes allow/deny, and buildClaudeCommand never emits --allowedTools. Any agent.yaml that sets permissions.allowedTools will validate and round-trip successfully yet have no runtime effect, so expected tool gating is silently skipped.

Useful? React with 👍 / 👎.

Comment on lines +193 to +194
.enum(['default', 'acceptEdits', 'bypassPermissions', 'plan', 'dontAsk', 'auto', 'remoteApproval'])
.optional(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Apply permissions.permissionMode when launching Claude

This commit validates permissions.permissionMode but does not propagate it into the command path; buildClaudeCommand still hardcodes --permission-mode auto and ignores params.permissions.permissionMode. In practice, configs like acceptEdits/bypassPermissions appear accepted in schema/tests but are not honored at runtime, which can block write/edit flows that depend on a non-auto mode.

Useful? React with 👍 / 👎.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements the wire-up for agent.yaml permissions, specifically adding support for allowedTools and permissionMode across the configuration schemas and provider adapters. It also fixes a regression in agent spawning where built-in role names (like 'engineer') caused database crashes due to missing UUID validation. Additionally, the reviewer agent has been upgraded to the opus model, and an audit of default agents was performed, confirming that all 20 existing agents are currently required. My feedback highlights an opportunity to centralize the permissionMode enum values to reduce redundancy and maintenance overhead across multiple files.

deny?: string[];
bashAllowPatterns?: string[];
allowedTools?: string[];
permissionMode?: 'default' | 'acceptEdits' | 'bypassPermissions' | 'plan' | 'dontAsk' | 'auto' | 'remoteApproval';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The permissionMode enum values are duplicated across multiple files and schemas (agent-directory.ts, provider-adapters.ts, and agent-yaml.ts). This redundancy increases the risk of inconsistency when new modes are added. Consider centralizing these values in a shared location, such as src/lib/sdk-directory-types.ts or a dedicated constants file, and deriving the types and Zod schemas from that single source.

Single PR covering the agent.yaml migration finishing work plus the
spawn pipeline unblocker. Genie está inútil sem isso.

## Permission wireup (P0)

- AgentConfigSchema.permissions extended with allowedTools and permissionMode
- DirectoryEntry.permissions mirrors the same fields
- Tests cover schema parse + round-trip in agent-yaml.test.ts
- Engineer subagent analysis in audit-default-agents.md

## Spawn id_shape_check fix (P0)

- New migration 061_agents_id_invariant_and_fk_lockdown.sql plus tests
  fixes the agents_id_shape_check violation that blocked every direct
  genie spawn after retire-session-names landed.

## Reviewer model: haiku to opus (P1)

- plugins/genie/agents/reviewer/AGENTS.md frontmatter changed to opus.
- Felipe direct: haiku does not support --permission-mode auto.
- Audit confirms zero haiku/sonnet defaults remain in plugins/genie/agents/.

## Wish scaffold

- .genie/wishes/agent-yaml-permissions-wireup/WISH.md (384 lines, 8
  execution groups) survives as documentation of intent.

## Validation

- bun run typecheck clean
- bun run lint: 0 errors, 16 warnings (dev baseline 15; +1 from
  in-flight engineer work absorbed here)

## Out of scope (followups)

- Default-agents audit + delete dead roles (Group 4)
- Frontmatter to agent.yaml migration on built-in roles (Group 6)
- Felipe agent.yaml end-to-end smoke test (Group 7)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@namastex888 namastex888 closed this May 3, 2026
namastex888 added a commit that referenced this pull request May 4, 2026
Finishes the AGENTS.md frontmatter -> agent.yaml migration on the
schema/wiring side, plus reviewer model fix. Felipe directive: "we cant
work without this" — workspace agents need to declare tool exposure via
agent.yaml directly.

## Schema additions

- AgentConfigSchema.permissions extended with allowedTools and
  permissionMode (src/lib/agent-yaml.ts) so workspace agents can declare
  tool exposure via agent.yaml instead of legacy AGENTS.md frontmatter.
- DirectoryEntry.permissions mirrors the same fields
  (src/lib/agent-directory.ts).
- SpawnParams already has matching fields; provider-adapters.ts wires
  them through the launch command builder.
- Tests cover schema parse + round-trip
  (src/lib/agent-yaml.test.ts).

## Reviewer model: haiku to opus

- plugins/genie/agents/reviewer/AGENTS.md frontmatter changed.
- Felipe direct: haiku does not support --permission-mode auto. Reviewer
  runs every wish gate and must use opus for consistent tool gating.
- Audit confirms zero haiku/sonnet defaults remain in
  plugins/genie/agents/.

## Documentation

- Wish scaffold at .genie/wishes/agent-yaml-permissions-wireup/WISH.md
  (384 lines, 8 execution groups) tracks intent.
- audit-default-agents.md captures the engineer subagent's per-role
  inventory.

## Out of scope (followups in separate PRs)

- agents_id_invariant migration + spawn UUID fix (already in flight via
  retire-session-names wish #175 G1; not duplicated here).
- Frontmatter to agent.yaml migration on built-in roles.
- Felipe agent.yaml end-to-end smoke test.
- Default-agents delete-dead pass.

## Validation

- bun run typecheck clean.
- bun run lint: 0 errors, 16 warnings (matches dev baseline + 1 from
  this PR's added schema field).

Replaces PR #1623 (closing it — that branch had a botched force-push
chain with old wish: commit type rejected by commitlint and a
misguided UUID guard that broke PG tests).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@automagik-genie
automagik-genie deleted the wish/agent-yaml-permissions-wireup branch September 25, 2026 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant