Repository navigation
fix(spawn): guard resolveSpawnIdentity against non-uuid names - #1628
Conversation
After PR #1627 (generateWorkerId returns UUID), resolveSpawnIdentity still ran SELECT id FROM agents WHERE id = ${name} with role-name strings, hitting: PostgresError: invalid input syntax for type uuid: "engineer-4-eac7" This blocks every fresh dispatch where the role name is not a UUID (every wish-work, every direct genie spawn). Fix: at function entry, if name is not UUID-shaped, skip the canonical-lookup query and return a fresh canonical with a generated UUID workerId. Role-based dispatch continues working; UUID-keyed lookups still flow through the original code path. Validation: bun run typecheck clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the resolveSpawnIdentity function to handle non-UUID agent names by returning a fresh canonical identity, preventing database lookup failures following a schema migration. The review feedback suggests using the existing UUID_REGEX constant for better maintainability and points out a potential issue where parallel ID generation might still violate UUID constraints in the database.
| // Migration 061 + PR #1627: agents.id is UUID. Non-UUID names cannot match | ||
| // by id; skip the canonical-lookup query and return a fresh canonical with | ||
| // a generated UUID workerId. Keeps role-based dispatch working post-061. | ||
| if (!/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(name)) { |
There was a problem hiding this comment.
Use the existing UUID_REGEX constant (defined at line 2258) instead of duplicating the regex literal. This improves maintainability and ensures consistency across the module.
Additionally, while this guard prevents crashes for role-based names (like "engineer"), the parallel spawn logic later in this function (line 2413) still constructs workerId as ${name}-${shortId}. If agents.id is a UUID type in Postgres, those parallel IDs will still cause invalid input syntax for type uuid errors. You should consider updating the parallel ID generation to ensure it always produces a valid UUID.
if (!UUID_REGEX.test(name)) {There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8886e5f073
ℹ️ 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".
| if (!/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(name)) { | ||
| return { kind: 'canonical', workerId: uuidFactory(), sessionUuid: uuidFactory() }; |
There was a problem hiding this comment.
Preserve role-scoped identity for non-UUID spawn names
Returning early for any non-UUID name bypasses all existing canonical lookup/liveness logic and always mints a brand-new workerId, so repeated genie spawn <role> calls no longer correlate with the same role identity. In this code path, identity.workerId is later assigned to effectiveRole (handleWorkerSpawn), which means downstream registration/lookup uses a fresh UUID instead of the requested role name, breaking the intended canonical/parallel decision path and making role-based resume/dispatch behavior diverge for normal role names.
Useful? React with 👍 / 👎.
Summary
After PR #1627 (
generateWorkerIdreturns UUID) merged, every fresh dispatch hits a NEW spot of the sameagents.idUUID assumption:resolveSpawnIdentity(src/term-commands/agents.ts:2330) runs:with the role-name string, but
agents.idis now UUID-only.Fix
Add UUID-regex guard at function entry. Non-UUID names skip the lookup and return a fresh canonical immediately. UUID-keyed lookups still flow through the original code path.
Companion to PRs #1626 + #1627
engineer-3colliding across wishes)generateWorkerIdto return UUIDresolveSpawnIdentity— same UUID-coercion blind spot in a different code pathAfter all three merge, spawn pipeline is fully unblocked end-to-end post-migration-061.
Validation
bun run typecheckcleanTest plan
genie work autopg-distribution-cutoverdispatches G4 withoutinvalid input syntaxerrorgenie spawn engineer --team genieproduces fresh canonical row with UUID id🤖 Generated with Claude Code