🤖 feat: allow agentId on workspace tasks (default exec) - #3957
🤖 feat: allow agentId on workspace tasks (default exec)#3957ThomasK33 wants to merge 16 commits into
Conversation
The task tool with kind: "workspace" now accepts an optional agentId (e.g. "plan") selecting the agent mode for the launched turn. Omitted agentId preserves existing behavior: exec for new workspaces, resumed identity for mode: "existing" follow-ups to descendant agent workspaces. Because resolveAgentForStream silently falls back to exec for top-level workspaces, createWorkspaceTurn validates explicit ids fail-fast via a shared helper (syntax -> existence -> ui-selectable -> not disabled, mirroring the UI agent picker): pre-create against the owner checkout and post-create against the target checkout for mode: "new", and against the target checkout for mode: "existing". Explicit ids are per-turn overrides only; no record/metadata schema changes. subagent_type stays rejected for workspace tasks (issue path narrowed to the field itself).
|
@codex review |
This comment has been minimized.
This comment has been minimized.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e01195b864
ℹ️ 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".
- Resolve agent discovery paths via resolveWorkspaceRootPath so Docker targets use the container-side runtime path (P1) - Include Agent Plugins roots in validation when the experiment is on (P2) - Skip post-create target re-validation while the created checkout is not reachable yet (deferred-provisioning runtimes) instead of stranding the launch; owner-side validation still gates obviously-bad ids (P1) - Reject explicit agentId for descendant agent workspace targets: stream resolution pins children to persisted identity, so an override could never actually run (P1) - Dispatch existing-target overrides with skipAiSettingsPersistence so the target's saved agent/settings stay untouched; new workspaces still persist the requested agent as their default (P1) - Pass the validated checkout as definitionContext to AI-settings resolution so agent frontmatter ai defaults apply (P1)
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 931084c6c2
ℹ️ 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".
Round 2 Codex findings: unreachable target checkouts (deferred provisioning, stopped containers) now use a consistent policy instead of skipping (mode new) or strict-failing with a misleading error (mode existing): built-in agents validate against embedded definitions and launch safely; non-built-in agents fail with an explicit reachability error, preventing a silent exec fallback at stream time.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce92a4aef7
ℹ️ 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".
…record Round 3 Codex findings: (1) post-create validation failures now write the workspace-turn record first and settle it as an error, so the created workspace stays owner-owned and a mode="existing" retry passes the ownership check instead of hitting invalid_scope; (2) unreachable-target built-in ids are validated against the owner's checkout (same project), so a project-local shadow of a built-in id — including its ui/disabled state — is respected instead of trusting embedded membership alone.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab12c42326
ℹ️ 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".
…heckouts Round 3 P2: gate unreachable-target validation on the winning definition's scope resolved via the owner context, not built-in membership — host-side definitions (built-in, global, plugin) resolve independently of the target checkout, while project-scoped definitions keep failing closed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3dbef2ed51
ℹ️ 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".
Round 4 Codex findings: - NodeAgentDefinitionContext carries includeAgentPlugins so AI-settings resolution reads plugin agents' frontmatter ai defaults and base chains (wired for both workspace turns and sub-agent creation) - A trunkBranch different from the owner's branch makes owner-side agent resolution advisory: target-branch-only agents launch after strict post-create target validation instead of failing "unknown agentId" - The same divergence makes unreachable targets fail closed for every id (the target branch can shadow even built-ins); the settled record keeps the workspace retryable via mode="existing"
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08bab92eb3
ℹ️ 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".
…ble errors Round 5 Codex findings: agent validation contexts now use createRuntimeContextForWorkspace — the exact helper the stream uses — so subproject workspaces validate against the same discovery path that will stream (subproject-only agents pass; root/sub mismatches cannot slip through). Post-create validation failures on disposable workspaces no longer advertise a mode="existing" retry, since disposable settlement cleans the workspace up.
…compare Round 6 Codex findings: existing targets have unknown checkout provenance, so explicit agent overrides fail closed while the checkout is unreachable (default identity keeps working); the trunkBranch divergence check compares via sanitizeBranchNameForWorkspace so feature/foo is not treated as divergent from its own workspace feature-foo; and synchronous post-create validation failures strip attentionPolicy from the settled record to avoid a duplicate terminal wake on top of the synchronous tool error.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2b5618da6
ℹ️ 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".
|
@codex review Round-7 fixes pushed in b267445:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b267445ab7
ℹ️ 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".
This comment has been minimized.
This comment has been minimized.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4566a2c898
ℹ️ 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".
This comment has been minimized.
This comment has been minimized.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de00023f45
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b8ebd741c
ℹ️ 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".
This comment has been minimized.
This comment has been minimized.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e572dac6ec
ℹ️ 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".
…shadows; strict fail-closed on eligibility errors
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2e23b8c22
ℹ️ 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".
…kes; forward strictness to resumed turns
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
This comment has been minimized.
This comment has been minimized.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f24c86fc9
ℹ️ 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 (message.role !== "user") { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
When an existing-workspace turn with an explicit override finishes normally while one of its background bashes remains monitored, a later monitor match is a new synthetic turn, not a continuation of the completed delegated turn. This scan ignores all assistant messages—including the terminal finishReason: "stop" response—so the old workspace-task user row is still selected and its per-turn agent/model/strictness overrides are reused. The late wake can therefore run under the previous override instead of the target's persisted defaults, or fail and be consumed if that agent has since disappeared; stop scanning when a terminal assistant closes the delegated turn, like the open-correlation logic does.
Useful? React with 👍 / 👎.
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
1 participant
Summary
The
tasktool withkind: "workspace"now accepts an optionalagentId(e.g.plan) selecting the agent mode for the launched turn, with fail-fast validation of unknown, internal (ui.hidden), and disabled agents before any workspace is created or any turn is dispatched. OmittedagentIdpreserves today's behavior exactly.Background
Calling
taskwithkind: "workspace"+agentId: "plan"previously failed schema validation ("Workspace tasks do not accept agentId or subagent_type"), making it impossible for an agent to launch new workspaces in plan mode. Plan mode needs no pre-setup (planFilePathis derived from workspace metadata), so a first turn withagentId: "plan"just works once the id reachessendMessage.Implementation
toolDefinitions.ts): thekind === "workspace"branch keeps rejecting only the deprecatedsubagent_typealias (message narrowed, issue path moved to["subagent_type"]);agentIdis now allowed and documented in the tool description.tools/task.ts): forwardsagentIdintotaskService.createWorkspaceTurn. The plan-mode-parent gate is untouched — a parent already in plan mode still cannot spawn workspace tasks.taskService.ts): newvalidateWorkspaceTurnAgentIdhelper (syntax →resolveAgentFrontmatterexistence →resolveAgentVisibility().selectable→!isAgentEffectivelyDisabled, the same rule set as the UI agent picker, so custom user-visible agents work without a hardcoded allowlist). Validation is fail-fast becauseresolveAgentForStreamsilently falls back to exec for top-level workspaces, which would hide the caller's mistake:mode: "new": two-stage — owner checkout pre-create (bad ids create no workspace), target checkout post-create (project-local agent definitions can diverge across branches; on failure no turn is dispatched and the error names the created workspace, which is left as failed evidence since no safe pre-record cleanup path exists).mode: "existing": validated against the target workspace's checkout; an explicit id is a per-turn override only — it wins over the resumed descendant-agent identity but never mutates persisted identity (no record/metadata schema changes).mode: "existing") without persistingattentionPolicy, so the synchronous error is never duplicated by a terminal wake. Owner-side prechecks and unreachable-target vouching are fatal only when the owner's checked-out branch is git-proven to be the target's base. As the final backstop, explicit overrides dispatch withstrictAgentResolution: stream-time resolution (which runs after init completes) fails the stream loudly if the requested agent cannot be resolved or is disabled, instead of silently falling back to exec.model/thinkingoverrides and follow-up target-settings precedence are unchanged.Validation
Beyond unit tests and
make static-check, live dogfooding against an isolated dev-server sandbox (tempXUM_ROOT), driving the realtasktool through a real model in the web UI:agentId: "plan"→ launched workspace's first turn ran in plan mode: plan file written under~/.xum/plans/…,propose_plancompleted, plan proposal card with Implement button; persisted last-used agentplan.agentId→ turn resolved toexec(default unchanged).agentId: "compact"→Task.createWorkspaceTurn: agentId is not selectable for workspace turns (compact), no workspace created.agentId: "doesnotexist"→Task.createWorkspaceTurn: unknown agentId (doesnotexist), no workspace created (sandbox config verified).taskService.test.tsshows 3 pre-existing failures that reproduce identically on a clean-HEAD probe worktree (unrelated to this change).Risks
Low. The default path (
agentIdomitted) is byte-for-byte the previous behavior, pinned by existing tests. The new validation only runs for explicit ids and fails closed. The riskiest surface is the post-create re-validation inmode: "new", which can leave a created-but-undispatched workspace behind on divergence — rare, surfaced with a clear error naming the workspace.📋 Implementation Plan
Allow
agentIdon workspace tasks (defaultexec)Context
Calling the
tasktool withkind: "workspace"+agentId: "plan"fails schema validation:"Workspace tasks do not accept agentId or subagent_type". This makes it impossible to tell an agentto launch new workspaces in plan mode.
Verified facts:
refineTaskToolAgentArgsinsrc/common/utils/tools/toolDefinitions.ts(~366–411) rejectsagentId/subagent_typewheneverkind === "workspace".src/node/services/taskService.tscreateWorkspaceTurn(~3674):let workspaceTurnAgentId = "exec";→ passed toworkspaceService.sendMessage(..., { agentId, ... })(~3882–3894).resolveAgentForStream(src/node/services/agentResolution.ts:267–279) silently falls back to exec for top-level workspaces. So fail-fast validation must happen increateWorkspaceTurn, before workspace creation.resolveAgentFrontmatter(runtime, workspacePath, agentId)(existence; throwsAgent definition not found),resolveAgentVisibility(frontmatter.ui)→selectable(src/node/services/agentDefinitions/agentVisibility.ts:16–20, mirrors the UI picker filteruiSelectable), andisAgentEffectivelyDisabled. Built-ins:exec/planare selectable;explore,compact,desktop,dream,name_workspacehaveui.hidden: true. Custom user agents withoutui.hiddenare selectable.planFilePathis computed from workspace metadata instreamContextBuilder, so a first turn withagentId: "plan"just works (plan-file-locked edits,propose_planenabled).WorkspaceTurnTaskHandleRecord(src/node/services/taskHandleStore.ts:51–125) norbuildWorkspaceTurnMuxMetadata(taskService.ts:10843–10852) has an agent field.mode: "existing"follow-ups resume agent identity only for descendant agent-task workspaces viaresolveTaskAgentIdForResume(readsagentId/agentTypefrom the workspace config entry); follow-ups tomode: "new"-created normal workspaces always default to exec today.Behavior contract
kind: "workspace"accepts optionalagentId(normalized via existingTaskAgentIdSchema: trim + lowercase).src/node/services/tools/task.ts~415–417 and the existing rejection test stay untouched).agentId→ behavior unchanged:"exec"for new workspaces; resumed identity formode: "existing"follow-ups to descendant agent workspaces; exec for follow-ups to normal workspaces.agentIdonmode: "existing"is a per-turn override only — it changes that turn's agent, never mutates the target workspace's persisted identity (nothing is written; records store no agent field, and we intentionally do not add one).subagent_typestays rejected for workspace tasks (deprecated sub-agent alias); refinement error message narrows to"Workspace tasks do not accept subagent_type"with the zod issue path on["subagent_type"](not["agentId"]).validateWorkspaceTurnAgentId(runtime, workspacePath, agentId)so existence/selectable/disabled logic cannot drift):AgentIdSchema→Erron invalid syntax.resolveAgentFrontmatter(existence) →resolveAgentVisibility(...).selectable(rejects internal agents likecompact) → notisAgentEffectivelyDisabled— same rule set as the UI agent picker, so custom user-visible agents work without a hardcoded allowlist.mode: "existing": validate against the target workspace path (the workspace whose turn will run), before dispatch.mode: "new": two-stage. Pre-create validation against the owner workspace path catches obvious bad ids (doesnotexist,compact) with no workspace created. AfterworkspaceService.create, re-validate against the target workspace path beforesendMessage(project-local agent definitions can diverge across branches/worktrees — owner-path resolution is not an invariant); the finalized id feeds AI-settings resolution. If target validation fails, do not dispatch the turn — surface a clear handle/tool error; decide at implementation time whether to leave the created workspace as failed evidence or clean up (only if existing lifecycle code already supports safe cleanup).resolveAgentForStreamwould silently run exec, hiding the caller's mistake.workspaceTurnAgentId(after explicit override) is the id used wherever agent-default model/thinking settings are resolved increateWorkspaceTurn, so a plan-mode workspace gets plan-agent defaults. Precedence preserved: explicit toolmodel/thinking> existing target settings on follow-ups > agent defaults; parent pro-mode inheritance logic untouched (covered by existing tests).Changes (net product LoC: ~ +45–60)
src/common/utils/tools/toolDefinitions.tsrefineTaskToolAgentArgs: in thekind === "workspace"branch, stop rejectingagentId; keep rejectingsubagent_typewith narrowed message and issue path["subagent_type"].buildTaskToolDescription: document that workspace tasks accept optionalagentId(agent mode for the launched turn, e.g.plan), defaultexec; internal agents are not eligible.src/node/services/tools/task.ts(~419–442)kind === "workspace"branch, forward the providedagentIdintotaskService.createWorkspaceTurn({...}).src/node/services/taskService.tsagentId?: stringtoWorkspaceTurnCreateArgs(~520–544).validateWorkspaceTurnAgentId(runtime, workspacePath, agentId): syntax (AgentIdSchema) → existence (resolveAgentFrontmatter) → selectable (resolveAgentVisibility) → not disabled (isAgentEffectivelyDisabled), mirroring the sub-agent path at ~4202–4207.createWorkspaceTurn: whenargs.agentIdis provided, run the helper (owner path pre-create formode: "new", target path formode: "existing"; target-path re-validation after create formode: "new") and setworkspaceTurnAgentIdfrom it — override wins over resume identity. Otherwise keep existing defaults.workspaceTurnAgentIdinto the "agent defaults on create" AI-settings resolution instead of implicit exec.WorkspaceTurnTaskHandleRecordandWorkspaceTurnMuxMetadatadeliberately stay agent-free (per-turn override semantics; avoids schema churn and accidental identity mutation).Tests
src/common/utils/tools/toolDefinitions.test.tsagentId: "plan".subagent_typefor workspace tasks.src/node/services/tools/task.test.tsagentIdtocreateWorkspaceTurnfor workspace kind.src/node/services/taskService.test.tscreateWorkspaceTurnwithagentId: "plan"→sendMessagecalled withagentId: "plan".agentIdoverrides resume identity onmode: "existing"follow-up to a descendant agent workspace.agentIdonmode: "existing"follow-up still resumes the persisted identity (pin current behavior).Err, no workspace created (mode: "new"pre-create stage).compact) →Err.agentId: "exec", ~line 962) stays green.Dogfooding & quality gates
Gate A (after implementation, before dogfooding):
bun test src/common/utils/tools/toolDefinitions.test.ts src/node/services/tools/task.test.ts src/node/services/taskService.test.tsmake typecheckthenmake static-check.Gate B (live dogfood, evidence required):
dev-server-sandboxskill to launch an isolated dev-server instance (tempXUM_ROOT, free port).tasktool with{"kind":"workspace","agentId":"plan","prompt":"Plan a small change...","title":"Plan dogfood"}(drive viabun run debug send-message <workspace-id>oragent-browseragainst the web UI).propose_plan/plan-file behavior active.agentIdstill starts in exec.agentId: "compact"andagentId: "doesnotexist"return clear tool errors and create no workspace.agent-browserscreenshots of the tool call arguments/result and the spawned workspace's mode indicator, plus a screen recording (agent-browser video recording) of the plan-mode launch flow; attach viaattach_file.Acceptance criteria
taskwithkind: "workspace",agentId: "plan"succeeds; launched workspace's first turn runs in plan mode with plan-agent default AI settings.agentIdpreserves today's behavior exactly (existing tests pass unmodified except the narrowed rejection message test).Errbefore any workspace is created — no silent exec fallback.agentIdon existing-workspace follow-ups overrides for that turn only; no persisted identity changes.make static-checkgreen; Gate B evidence captured.Generated with
xum• Model:anthropic:claude-fable-5• Thinking:xhigh• Cost:$37.19