-
Notifications
You must be signed in to change notification settings - Fork 56
fix(spawn): preserve role name as workerId; align skill text with built-in name #1663
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,7 +39,7 @@ Trace must run in **isolation** — the subagent must not modify any source file | |
|
|
||
| ```bash | ||
| # Spawn a tracer subagent (read-only investigation) | ||
| genie agent spawn tracer | ||
| genie agent spawn trace | ||
| ``` | ||
|
|
||
| ## Task Lifecycle Integration (v4) | ||
|
|
@@ -63,7 +63,7 @@ An engineer reports that `genie work` dispatches engineers but they sit idle. Th | |
|
|
||
| ```bash | ||
| # 1. Spawn a tracer (read-only — no code changes) | ||
| genie agent spawn tracer | ||
| genie agent spawn trace | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| # 2. Send the symptoms | ||
| genie agent send 'Trace: genie work dispatches engineers but they start idle at the prompt. No task received. genie wish status shows in_progress but nothing happens. Check dispatch.ts workDispatchCommand and protocol-router.ts sendMessage.' --to tracer | ||
|
|
||
64 changes: 64 additions & 0 deletions
64
src/term-commands/__tests__/spawn-identity-name-guard.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| /** | ||
| * Regression: fix-spawn-uuid-rename-regression wish (Bug A). | ||
| * | ||
| * Commit 8886e5f0 added a guard in resolveSpawnIdentity that early-returns | ||
| * when the spawn `name` is not UUID-shaped — correct, because the SQL probe | ||
| * downstream would crash trying to cast a role-name string to UUID. But the | ||
| * early-return minted a fresh UUID for `workerId` instead of preserving the | ||
| * human-readable spawn name. That UUID then propagated as custom_name and | ||
| * role through findOrCreateAgent → registerSpawnWorker → hireAgent, breaking | ||
| * every `--to <role-name>` resolver tier on dev-local. | ||
| * | ||
| * The fix preserves `name` as `workerId` for non-UUID inputs (one line at | ||
| * agents.ts:2373). This file is a standalone regression test that does not | ||
| * touch PG — the early-return path is pure (no DB access), so we can verify | ||
| * it without the heavy pgserve fixture in the parent agents.test.ts file. | ||
| */ | ||
|
|
||
| import { describe, expect, test } from 'bun:test'; | ||
| import { resolveSpawnIdentity } from '../agents.js'; | ||
|
|
||
| describe('resolveSpawnIdentity — non-UUID name guard (regression: spawn-uuid-rename)', () => { | ||
| test('non-UUID name preserves name as workerId (Bug A primary fix)', async () => { | ||
| const fixedSessionUuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'; | ||
| const identity = await resolveSpawnIdentity('engineer', 'team-x', () => fixedSessionUuid); | ||
| expect(identity.kind).toBe('canonical'); | ||
| expect(identity.workerId).toBe('engineer'); | ||
| expect(identity.sessionUuid).toBe(fixedSessionUuid); | ||
| }); | ||
|
|
||
| test('hyphenated role names are preserved verbatim', async () => { | ||
| const identity = await resolveSpawnIdentity( | ||
| 'council--architect', | ||
| 'team-y', | ||
| () => '11111111-2222-3333-4444-555555555555', | ||
| ); | ||
| expect(identity.kind).toBe('canonical'); | ||
| expect(identity.workerId).toBe('council--architect'); | ||
| }); | ||
|
|
||
| test('parallel-suffix worker names (engineer-4-eac7) are preserved', async () => { | ||
| // PR #1627 motivating case: SQL probe used to crash on this input. | ||
| // Post-guard, it must early-return with workerId === name (not a UUID). | ||
| const identity = await resolveSpawnIdentity( | ||
| 'engineer-4-eac7', | ||
| 'team-z', | ||
| () => '22222222-3333-4444-5555-666666666666', | ||
| ); | ||
| expect(identity.kind).toBe('canonical'); | ||
| expect(identity.workerId).toBe('engineer-4-eac7'); | ||
| }); | ||
|
|
||
| test('uuidFactory is called exactly once (sessionUuid only) on the early-return path', async () => { | ||
| // Pre-fix: factory was called twice (workerId + sessionUuid both minted). | ||
| // Post-fix: factory is called once (sessionUuid only); workerId === name. | ||
| let calls = 0; | ||
| const factory = () => { | ||
| calls += 1; | ||
| return `${'0'.repeat(8)}-0000-0000-0000-${String(calls).padStart(12, '0')}`; | ||
| }; | ||
| const identity = await resolveSpawnIdentity('reviewer', 'team-w', factory); | ||
| expect(calls).toBe(1); | ||
| expect(identity.workerId).toBe('reviewer'); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2354,10 +2354,13 @@ export async function resolveSpawnIdentity( | |
| executorRegistry.resolveWorkerLivenessByTransport(agent), | ||
| ): Promise<SpawnIdentity> { | ||
| // 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. | ||
| // by id; skip the canonical-lookup query and return a fresh canonical. | ||
| // Preserve `name` as the human-readable workerId — the DB-level UUID is | ||
| // minted later by findOrCreateAgent (agent-registry.ts). Returning a | ||
| // fresh UUID here would clobber custom_name with a UUID and break every | ||
| // `--to <role>` resolver tier. | ||
| 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() }; | ||
| return { kind: 'canonical', workerId: name, sessionUuid: uuidFactory() }; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| } | ||
| const { getConnection } = await import('../lib/db.js'); | ||
| const sql = await getConnection(); | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example command on the following line (line 244) still uses
--to tracer. Since the agent is now spawned astraceon line 243, the target name in thesendcommand should be updated to--to traceto ensure the example remains correct and functional.