Skip to content

refactor(desktop): converge the three definition→instance mappings (Phase 1B.3.5)#1645

Merged
wesbillman merged 1 commit into
mainfrom
feat/definition-instance-mapping
Jul 8, 2026
Merged

refactor(desktop): converge the three definition→instance mappings (Phase 1B.3.5)#1645
wesbillman merged 1 commit into
mainfrom
feat/definition-instance-mapping

Conversation

@wesbillman

Copy link
Copy Markdown
Collaborator

What (Phase 1B.3.5 — mapping convergence)

The three surfaces that start an agent from a definition — (A) library quick-start (usePersonaActions.handleSubmit), (B) library start-existing (useManagedAgentActions.handleStartPersona), (C) profile start (UserProfilePanel.createManagedAgentForPersona) — each built CreateManagedAgentInput independently and had drifted. This converges them on one mapping, desktop/src/features/agents/lib/instanceInputForDefinition.ts, per the decision table pressure-tested in-channel:

Row Divergence Decided behavior
1 C started on a fallback runtime with only a toast when the configured runtime was missing; A/B refused Refuse with an actionable error, everywhere (Wes's call). resolveStartRuntimeForDefinition
2 harnessOverride: A always true, B formula, C omitted B's formula — the only one aligned with the backend contract (create_time_agent_command_override stores None when picked == inherited; harness_override: false keeps the definition authoritative on fallback)
3 Avatar: A resolved, B/C raw resolveManagedAgentAvatarUrl with an injectable upload fn (helper stays unit-testable; plain URLs pass through, base64 data URIs upload)
4 B/C seeded envVars: persona.envVars into create Never seed definition env. Post-#1640 record.env_vars is overrides-only; a stale seed (definition env edited before first spawn) no longer matches the live value, so the retain() self-heal would classify it as a deliberate override and silently mask the edit — the exact bug class #1640 killed, re-manufactured at create
5 Feedback: library notices / action notices / toasts Deliberately not converged — surface-appropriate feedback is a feature
6 Runtime list: B refetch-aware, C raw query.data ?? [] (spurious refusal when unfetched) Refetch-aware everywhere. availableRuntimesForStart

Subtraction: B's private getAvailableRuntimesForStart + filterAvailableRuntimes deleted as dead. Net src delta excluding the new helper+tests: 28 insertions, 97 deletions.

Behavior changes (user-visible, disclosed)

  • Profile start with a missing configured runtime now refuses with the actionable error instead of silently starting on a different runtime (decided by Wes in-channel).
  • Profile start on an unfetched runtime list now fetches instead of spuriously refusing.

envVars downstream-reader audit (agreed at scope time)

Sole frontend reader of input.envVars on the create path: tauri.ts:1148 (envVars: input.envVars ?? {}) — safe default when omitted. CreateAgentDialog's envVars is the standalone-create path (genuine user overrides) and is untouched. channelAgents.ts: no envVars readers. Reviewer re-verifies independently.

Pins

Frozen shapes (CreateManagedAgentInput type untouched), zero spec edits (desktop/tests/e2e/ diff = 0 lines), cascade untouched (submitProfilePersonaDialog not in the diff), budget 6 files.

Validation

  • tsc clean, biome clean
  • Unit: 2092/2092 (10 new tests pin rows 1/2/3/4/6, incl. the create input never contains definition env vars regression)
  • Scoped e2e: 59-test agent/persona smoke slice green, zero spec edits

The three surfaces that start an agent from a definition (library
quick-start, library start-existing, profile start) each built their
CreateManagedAgentInput independently and had drifted on five axes.
Converge them on one mapping in instanceInputForDefinition.ts:

- availableRuntimesForStart: refetch-aware runtime acquisition (profile
  previously resolved against a possibly-unfetched list and could
  spuriously refuse).
- resolveStartRuntimeForDefinition: refuse with an actionable error when
  the definition's configured runtime is unavailable, at every surface
  (profile previously started on a fallback runtime with only a toast).
- buildInstanceInputForDefinition: harnessOverride via the
  backend-aligned formula (create_time_agent_command_override stores
  None when picked == inherited; harness_override: false keeps the
  definition authoritative on fallback); avatar through
  resolveManagedAgentAvatarUrl with an injectable upload fn; definition
  env vars never seeded into the create input (record.env_vars is
  overrides-only since #1640 — seeding would manufacture pseudo-
  overrides that mask definition env edits made before first spawn).

Feedback surfaces deliberately stay per-site. B's private
getAvailableRuntimesForStart/filterAvailableRuntimes are deleted as
dead. 10 unit tests pin the decided rows, including the no-definition-
env-ever regression.

Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co>
Signed-off-by: Wes <wesbillman@users.noreply.github.com>
@wesbillman

Copy link
Copy Markdown
Collaborator Author

Review — APPROVE (via comment; formal approve blocked, same authed account)

Reviewed against the six rows and pins locked in-channel.

Pins: CreateManagedAgentInput / types.ts / tauri.ts / channelAgents.ts untouched (diff = 0 lines) ✓; desktop/tests/e2e/ diff = 0 lines ✓; submitProfilePersonaDialog / UserProfilePanelPersonaSubmit.ts absent from the diff — cascade untouched ✓; 6 files, src delta excluding helper+tests is net-subtractive (28+/97−) ✓.

Helper (instanceInputForDefinition.ts): three functions map 1:1 onto rows 6 / 1 / 2–4 as agreed. Row-2 formula matches the backend contract (create_time_agent_command_override); row-4 comment documents the overrides-only invariant correctly; row-3 upload is injectable with the tauri default via the now-exported UploadMediaBytes type — the only change to managedAgentAvatar.ts is that one-word export. ✓

Per-site reads:

  • A (usePersonaActions.handleSubmit): keeps its find-or-error (row 1 semantics unchanged), drops hardcoded harnessOverride: true in favor of the helper formula — which evaluates identically in A's context (persona just created with the chosen runtime). Zero behavior change, as claimed. ✓
  • B (handleStartPersona): slims to the three helper calls; private getAvailableRuntimesForStart + filterAvailableRuntimes deleted — their logic moved verbatim into availableRuntimesForStart. Refuse semantics preserved. ✓
  • C (createManagedAgentForPersona): gains refetch-aware runtimes + refuse-on-override, the two disclosed behavior changes per Wes's row-1 call. Stops seeding envVars. ✓

envVars independent re-verify (my agreed slot): grepped the branch — channelAgents.ts has zero envVars readers; tauri.ts:1148 input.envVars ?? {} is the sole create-path consumer, safe default on omission; CreateAgentDialog envVars is the standalone-create path, untouched. Matches the PR body's audit exactly.

Unit tests: 10 tests pin every decided row, including the named regression (row 4: create input never contains definition env vars) and both row-6 cases (cached vs unfetched). ✓

Manual gate (fresh detached checkout of 361f51fcf): full unit 2092/2092, scoped e2e (edit-agent + agents + persona-env-vars + persona-sync + human-edit-agent-content) 26/26 — independently matching the builder's numbers.

Two non-blocking observations (no change requested):

  1. With refuse-on-override in the helper, resolveStartRuntimeForDefinition can only ever return an empty warnings array today (every warning-producing path throws). B's notices = [...warnings] and C's toast loop are inert plumbing — harmless, and they future-proof non-fatal warnings, but worth knowing they're currently unreachable.
  2. C's useCallback dep changed from availableRuntimesQuery.data to the whole availableRuntimesQuery object; query object identity can change per render, so the callback memo is weaker. Correctness unaffected.

LGTM — queues at Wes's gate once CI settles.

@wesbillman wesbillman merged commit 9683ef8 into main Jul 8, 2026
25 checks passed
@wesbillman wesbillman deleted the feat/definition-instance-mapping branch July 8, 2026 17:08
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