Commit Graph
2 Commits
Author SHA1 Message Date
Brennan Benson d6d2795da5 chore(worktree): include create timing and spare outcome in the workspace create events (#24483)
* chore(worktree): include create timing and spare outcome in the workspace-created event

The workspace_created and workspace_create_failed events gain optional,
numbers-and-enums-only fields built from what the create already measured:
total and per-phase durations, the prepared-checkout hit/miss and miss
reason, the execution host (local/WSL/SSH), a worktree count bucket, how
many other creates were in flight, whether the repo has a post-checkout
hook (file existence only, probed after the create returns), and for a
failure the phase it died in plus elapsed time. No new git process runs;
consent and opt-out are unchanged.

* fix(worktree): attribute failed_phase by error, label WSL-path repos, skip the hook check with telemetry off

- failed_phase now names the outermost timed step the thrown error (or its cause) left, so a
  caught failure or a concurrent sibling step can no longer be misattributed; the old-relay SSH
  error keeps its cause so it still reads as git_worktree_add.
- execution_host follows the same rule Git routing uses, so a \\wsl.localhost repo reads wsl.
- The post-checkout hook check does not read the repo when telemetry is disabled.
- Privacy page mentions the miss reason code and the failed step.

* test(worktree): pin the old-relay SSH add error to git_worktree_add through its cause

* fix(worktree): name the create event field sets for their role, and type the old-relay test's caught error

* fix(worktree): send create events from runtime creates and record what the spare checkout did

Runtime creates (CLI, agents, phone app, paired clients, orchestration, server
automations) reuse prepared checkouts like the app's own creates, but recorded
no timing and sent no events. Both entry points now start one shared sender
(workspace-create-telemetry.ts), so every create sends exactly one event with
the same fields, plus create_entry_point (app | runtime).

Spare-checkout fields:
- concurrent_preparations: peak prepared-checkout builds and background
  discards running during the create, excluding the one it used; the window
  closes before the create's own re-arm starts.
- prepared_checkout_claim / prepared_checkout_discard phases, so on a miss
  git_worktree_add minus the prepared_checkout_* phases is the plain checkout.
- prepared_checkout_reset (none | base_moved | retargeted) replaces the
  retargeted flag; prepared_checkout_origin (prefetch | rearm) on hits.
- workspace_create_failed carries the spare outcome and its wait.
- repo_index_size_bucket from one stat of .git/index in the existing
  post-create probe (telemetry on, local/WSL only, 2 s cap).

* fix(worktree): add spare build and idle time, the re-arm prefetch origin, and a tracked-file count

- prepared_checkout_build_ms / prepared_checkout_idle_ms on hits: from arming
  the spare to ready, and how long it sat ready before the claim (0 when the
  create waited). readyAt is recorded in the pool's existing ready handler.
- prepared_checkout_origin gains rearm_then_prefetch: an automatic re-arm that
  the dialog prefetch then asked for too, so rearm means the re-arm alone.
- repo_file_count_bucket replaces the index byte size: the entry count from
  the 12-byte index header, which is the same in every index version; left
  out for a split or sparse index.
- The shared sender never lets a failed send change the create's result or
  error; it logs instead and still ends the create's concurrency membership.
- Tests pin the runtime SSH create's timing hand-off and the throwing-send
  cases on both entry points.

* fix(worktree): leave out the file count under any sparse checkout and time spare builds monotonically

- The repo probe also reads .git/config.worktree, where git sparse-checkout
  --sparse-index writes index.sparse, and omits the file count whenever
  sparse checkout or a sparse index is on in either file, with Git's boolean
  spellings. core.hooksPath there is honoured too.
- prepared_checkout_build_ms / _idle_ms use performance.now(), like every
  other duration; the build is timed from its own start (buildStartedAt).
- The origin field comment names all three values.

* fix(worktree): keep the spare's build time on its first build and count worktrees by lock reason

- prepared_checkout_build_ms runs from the first build's start (including any
  wait for the base fetch it is built on) to its first ready; a later tip
  refresh no longer restarts it, though it still counts as new preparation
  work. prepared_checkout_idle_ms runs from the latest ready (build or
  refresh) to the claim.
- The worktree count reads each .git/worktrees entry's locked file and leaves
  out entries whose lock reason names an Orca preparation, the way the
  listing does, instead of subtracting this process's spares. That covers
  spares from other processes, crash leftovers and spares being discarded,
  and cannot run one low while a spare's admin dir does not exist yet. It has
  its own 1.5 s cap inside the probe.
2026-10-02 12:46:13 -07:00
Neil a5796ec8eb refactor(runtime): split OrcaRuntimeService and compatibility tests (#17605)
* refactor(runtime): split OrcaRuntimeService into focused modules

* test(runtime): cover admission tiers and strict worktree reconciliation

* fix(runtime): preserve owner and structured session visibility

* fix(runtime): port post-extraction compatibility fixes

* fix(runtime): preserve skill-share cancellation barrier

* test(runtime): update identity inventory after extraction

* fix(runtime): preserve hook transport environment cleanup

* fix(runtime): consolidate idle probe imports

* test(runtime): retire split file process allowlist entry

* fix(runtime): route child process types through shared boundary

* test(runtime): preserve worktree host metadata precedence

* fix(runtime): update extracted test seams

* fix(runtime): gate the split's ts-nocheck set and restore the stop-confirmed contract

Audit follow-ups for the OrcaRuntimeService split:

- Freeze the 171 @ts-nocheck files behind a ratchet so no new file can disable
  type checking. The split's linear mixin chain cannot express forward
  references yet, so the existing suppressions are grandfathered; the baseline
  may only shrink.
- Drop the stray @ts-nocheck at the end of orca-runtime-get-status.ts. It sat
  after the first statement, where TypeScript ignores it, so the module was
  already checked.
- Restore `retireRejectedPty(ptyId, stopConfirmed: boolean)` as a required
  argument. The split widened it to optional and patched the resulting error
  with `stopConfirmed === true`; an omitted argument would have silently taken
  the unverified-stop path instead of failing to compile.
- Guard that every orca-runtime-tests fragment is imported by the compatibility
  entrypoint. The fragments are .spec.ts, which no Vitest include glob matches,
  so one left out of the list would silently stop running.

* fix(runtime): restore four behaviors the OrcaRuntimeService split dropped

Audit findings against the refactor's true base (ad5ba2572e):

- retirePtyAgentLaunchAuthority collected pane keys after deleting the
  restored-authority receipt instead of before it. collectPaneKeysForPty reads
  that receipt, so a receipt-only pane lost its key and never had its agent-hook
  compatibility authority retired. on-pty-exit.ts already carried a comment
  naming this exact invariant.
- The PTY-exit path kept orchestrationMailboxNotifications.retirePty but lost
  the loop that schedules a debounced mail-pointer repoint for the dead pty's
  terminal handle and any run bound to its panes. Restores the schedule call
  count to 7, matching base.
- subscribeToPtyExit lost isPtyKnownExited's leaf fallback and its
  post-registration lifecycle-generation recheck. leavesByPtyId is rebuilt from
  the renderer graph independently of ptysById, so a leaf can outlive its pty
  record; without the fallback a caller waiting on an already-dead pty never
  gets released.
- The chain root declared `[key: string]: unknown`, which base had nowhere. It
  leaked through the exported runtime type into every consumer, so any misspelled
  member access typechecked as unknown instead of erroring, and it accounted for
  957 of the suppressed errors. Removing it costs zero type errors.

* fix(runtime): restore escalation prose and unscoped automation publication

Two more behaviors the split dropped, each with a regression test that fails
against the pre-fix code:

- The worker-exit escalation stopped deriving its title through
  buildOrchestrationTaskDisplayMetadata and inlined `task.spec` instead. That
  ignored an explicit task_title, dropped the single-line normalization and the
  80-character bound, and turned the no-spec case into a quoted, duplicated id.
  A multi-paragraph spec landed verbatim in the coordinator's banner. The
  existing 11 tests all use short single-line specs, where the derived title and
  the raw spec are identical, so none of them could see it.
  Also reverts an added `if (!handle) return` guard: the dispatch lookup is
  deliberately keyed on the pane as well, because a reminted handle no longer
  matches the row while the pane identity outlives the remint.
- updateAutomation stopped going through automationChangePublications and
  published `source` unconditionally while gating the fallback on a non-null
  destination. A destination the store can no longer name then published only
  the stale source, so subscribers scoped elsewhere kept rendering a row that
  had left them — the exact case the helper documents. The helper had been left
  with zero callers; all three sites use it again.

* fix(skills): stop swallowing lookup errors and hard-erroring on non-ssh hosts

Follow-ups from auditing the skill install path against the refactor's base:

- resolveWorktree wrapped showManagedWorktree in `.catch(() => null)`, so a
  transient git or IO failure surfaced to the user as
  skill-install-workspace-not-found with the real cause discarded. Errors
  propagate again; a genuine id mismatch still returns null.
- resolveSkillSshTarget threw skill-install-workspace-host-unavailable when the
  execution host was neither local nor ssh, on both the repo and folder
  branches. Base gated these on connectionId, so a runtime-owned repo simply
  was not an SSH install and fell through to the local path. Both return null
  again, and the error code the split invented is now unreferenced.
- listManagedSkillInstalls awaited the receipt walk and the worktree resolve in
  sequence. They are independent and either can hit disk, WSL, or an SSH scan,
  so Promise.all is restored.

Deliberately unchanged: resolving the worktree through listResolvedWorktrees
rather than showManagedWorktree, which disambiguates a worktree id colliding
across hosts and is covered by its own test, and the SSH-folder
skill-install-ssh-dispatch-required throw, which matches the repo branch.

* fix(runtime): merge duplicate worktree-logic imports

The #17448 port added a third import from ../ipc/worktree-logic, which the
code-quality oxlint config rejects under --deny-warnings. Plain oxlint does not
flag it, so it only surfaced in CI's static analysis job.

* ci: run the ts-nocheck ratchet in PR checks

pr-workflow-lint-parity requires every leaf command in `pnpm lint` to have a
matching step in pr.yml. The ratchet was wired into lint but not the workflow,
so PR CI would not have enforced it.

* Merge remote-tracking branch 'origin/main' and retry the paired-host launch evaluate

main advanced 9 commits; none touch the orca-runtime.ts this branch splits, so
nothing needed porting.

CI failed twice on `Execution context was destroyed` thrown from
headless-paired-runtime-host's first `evaluate` after launch — a different spec
each run, which is the signature of the flake #17780 describes rather than a
regression. That commit added retryTransientMainEvaluate and adopted it in five
helpers but not this call site, even though its docblock names exactly this
case: the first evaluate after electron.launch() resolves, before the app is
ready. Wrapped it the same way.
2026-08-31 19:34:55 -07:00