Skip to content

Add agent creation to the workbench composer picker - #693

Open
pratikbuilds wants to merge 4 commits into
corbitsdev:mainfrom
pratikbuilds:feat/composer-create-agent
Open

Add agent creation to the workbench composer picker#693
pratikbuilds wants to merge 4 commits into
corbitsdev:mainfrom
pratikbuilds:feat/composer-create-agent

Conversation

@pratikbuilds

@pratikbuilds pratikbuilds commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

The new-workbench composer's agent picker offers Create agent, including when no agents exist. It opens the existing creation modal, preserves the composer draft, and selects the created agent. Empty descriptions use the agent name so the selection stays readable.

The durable-conversation adapter also serializes reactor startup reads and metadata writes with connector updates. Without this, startup can read metadata.json after a connector write truncates it but before that write completes, producing a JSON parse error and leaving the heartbeat run unfinished. The adapter retains the published storage implementation and the existing heartbeat timeout.

Validation:

  • Full bun run check passes with WORKBENCH_CHECK_CONCURRENCY=2, including database suites and structural checks.
  • 31 picker/modal tests pass, covering populated, empty, and null descriptions plus cancellation.
  • The metadata race regression reproduces the failure before the fix and passes 10 consecutive runs after it.
  • The real heartbeat end-to-end suite passes against local Postgres and noop inference.
  • Web and sidecar typechecks, focused lint, and the web build pass.
  • A browser walkthrough verifies the create-and-return interaction with fixture API responses; the full local stack also verifies login and opening the modal through the live picker.

@pratikbuilds

Copy link
Copy Markdown
Contributor Author

The heartbeat failure comes from concurrent access to the agent’s metadata.json, separate from the composer UI change.

An inbound message updates connector thread state and writes that file. Meanwhile, reactor startup loads it to restore pending operations, token usage, and connector state. The write can briefly leave the file empty between truncation and completion. A startup read during that gap fails to parse JSON.

CI captured Context store load failed: SyntaxError: JSON Parse error in @intx/storage-isogit’s loadMetadata(). The run recorded RunStarted and StepStarted, but no terminal event before the 30-second deadline. The timeout is the downstream symptom.

The durable-conversation adapter already queues connector updates, restores, and mirrors. Reactor storage access bypassed that queue. The fix routes reactor load() and writeMetadata() through the same queue, so they cannot overlap a connector metadata write. Other storage methods still delegate to the underlying store. The published storage package and heartbeat timeout are unchanged; genuinely corrupt metadata still produces an error.

For review:

  • apps/sidecar/src/conversation-state.ts: the storage adapter adds the two queued operations.
  • apps/sidecar/src/conversation-state.test.ts: the regression pauses a real metadata write after truncation, then starts a concurrent startup read. It fails before the fix and passes after it.

Validation: the forced-overlap regression passed 10 consecutive runs, the real heartbeat E2E passed locally, and full bun run check passed with WORKBENCH_CHECK_CONCURRENCY=2. These are local results, not a claim that the new GitHub CI run has completed.

@pratikbuilds

Copy link
Copy Markdown
Contributor Author
Screen.Recording.2026-09-08.at.9.13.50.PM.mov

@TheGreatAxios

Copy link
Copy Markdown
Contributor

@pratikbuilds question/thought - should we have further differentiation on workflows and agents? i.e Daily Digest or Morning Brief are more automated workflows than individual agents unless you are putting a singular agent in charge of just that

@TheGreatAxios

TheGreatAxios commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Recommendation: approve with nits.

Updated after a factual second pass. The metadata serialization fix is supported by the inspected paths: external load and writeMetadata operations join the existing queue through the proxy at apps/sidecar/src/conversation-state.ts:834, while internal operations use baseStorage, avoiding nested queue waits in those paths. The web tests at apps/web/test/new-workbench-picker.test.tsx:494 cover optimistic labeling and draft preservation; they do not establish refetch, tenant-switch, or focus behavior.

Follow-ups

  1. Normalize empty descriptions consistently. The existing agentDisplayName helper at apps/web/src/pages/new-workbench-picker.tsx:98 preserves description: "", which the API schema permits at packages/chat-ui/src/api.ts:191. The new normalization at apps/web/src/pages/new-workbench-picker.tsx:686 repairs only the optimistic cache entry. Normalize the display helper as well so refetching an empty description does not undo the optimistic fallback. The underlying helper behavior predates this PR.

  2. Cover tenant changes during creation. The callback at apps/web/src/pages/new-workbench-picker.tsx:694 lacks a current-tenant guard. A late result can append a stale-tenant ID; existing pruning means this is not proof of a persistent wrong-tenant chip. A tenant-switch-during-create test would clarify and protect the intended behavior.

Corrections to the original review

  • The filesystem spy does not depend on Bun-specific filesystem aliasing. createNodeIsogitRuntime() at apps/sidecar/src/conversation-state.ts:163 supplies node:fs, and the adapter resolves its promise methods dynamically. The test uses Bun APIs, but the claimed Node interception failure was incorrect and is withdrawn.
  • Splitting the composer feature and sidecar fix into separate PRs is organizational advice, not a correctness requirement. They already occupy separate commits and can be reverted independently.
  • Exact Cancel/Escape focus landing and long-chip rendering were not browser-verified. The earlier definitive focus-drop claim is withdrawn.
  • Adding an existing agent and creating a new agent are distinct actions; their different labels are not inherently inconsistent.

This assessment supports the queue fix in the inspected paths; it is not a claim that every possible storage API or UI interaction was exercised.

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.

2 participants