mirror of
https://github.com/stablyai/orca.git
synced 2026-10-13 08:02:52 +00:00
b922342199bb4c816072deb42cc58cf755420488
6
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Jinwoo Hong
|
2a68ea9496 |
refactor(terminal): one commit module for terminal topology closes, with a boundary check (#25329)
* refactor(terminal): move the topology revision and leaf-lookup helpers into terminal-topology
advanceTerminalTopologyRevision, findTerminalTabIdForLeaf and
hasHostAuthoritativeTerminalMembership move verbatim from the renderer-save
membership rebase into persistence/terminal-topology, the home of the commit
boundary. Importers are repointed; no behavior change.
* test(terminal): test-only guard for topology writes outside the commit boundary
The three session sinks now publish through one commitWorkspaceSessionPartition
helper, which hands the prior and published partition to the topology write
guard. Production never arms the guard, so each sink pays one global lookup.
The unit suite arms it in report mode: a sink write that changes class-(a)
topology (membership, root, bindings, titles, incarnations, sleeping records,
remote session ids, tombstones, default-applied, revision) outside a commit
scope is attributed to its writer's file by stack, and fails the test unless
the writer is on the unrouted-writers allowlist that later routing PRs shrink.
Renderer saves and test seeding are exempt. Deep-freeze is available but stays
off suite-wide until the in-place writers return new sessions.
* refactor(terminal): add the topology commit module with bindLeaf, closeLeaf and closeTab
terminal-topology-commit.ts is the boundary for class-(a) terminal topology.
bindLeaf forwards to persistPtyBinding, whose write now runs in a commit
scope; the spawn commits and the relay reattach bind through it. closeLeaf and
closeTab wrap the existing close mutation in a commit scope and one
persistence.terminal-topology span (kind, outcome, refusal reason; no ids),
and the runtime close goes through them. Session output is unchanged: tests
compare it byte for byte with the old writers for local, ssh: and folder
workspaces.
* chore(terminal): drop an unused lint suppression from the topology write guard
* fix(terminal): attribute topology guard writers relative to the repo root
The guard read a frame's file through its last /src/ segment, so a test under
tests/ (folder-upgrade-identity-persistence.unit.test.ts) had no source frame
and failed as an unknown writer. Frames are now taken relative to the repo
root the setup passes in, tests/ counts as test seeding, Windows backslashes
are normalized before the node_modules skip, and nested src/ paths keep their
full path. The two runtime funnel files skip only their funnel function, so
another writer in them still shows. The R9 allowlist key names the file whose
frame actually writes. The class-(a) diff and the attribution move into their
own files; the stack limit is restored in a finally.
* refactor(terminal): drop bindLeaf until binding reaches a sink; add the boundary ratchet
bindLeaf and the commit scope inside persistPtyBinding changed nothing: the
binding write never reaches a session sink, and a scope inside the Store method
would have admitted every direct caller once it did. Both return in B1-4; the
spawn commits and the relay reattach call persistPtyBinding directly again.
The runtime close now calls one closeLeafOrTab entry, so its callbacks keep
their contextual types. A census test is the primary enforcement: only
persistence/terminal-topology and the callers it lists may call
persistPtyBinding, the session setters or the three sinks, and every
unrouted-writer allowlist entry must name an existing file.
* fix(test): resolve the topology guard's repo root without the global URL
Under happy-dom the global URL is not Node's, so fileURLToPath(new URL(...))
threw in the setup file and failed every happy-dom test file.
* refactor(terminal): drop the runtime topology write guard; the boundary ratchet enforces
The AST boundary ratchet is the enforcement for B1. The stack-attributed
runtime guard, its class-(a) diff, the unrouted-writer allowlist, the vitest
setup and the freeze option are removed; it saw two writers in the whole unit
suite and its real value starts only once binding reaches a sink (B1-4).
Also: one closeLeafOrTab wraps the close in the span (no per-kind copies or
narrowed types), the span has one finish like persistence.pty-binding, the
sink helper is publishWorkspaceSessionPartition (it publishes; the commit
boundary is the module), the ratchet drops the private publishSession row and
checks that every listed caller file exists, and the close comparison keeps
its two meaningful cases with span cases chosen by name.
* refactor(terminal): trim the B1-1 commit module and ratchet to what they enforce
- Point the acknowledged-tab-retirement audit fixture at the moved
advanceTerminalTopologyRevision; its old import no longer resolved.
- Drop publishWorkspaceSessionPartition: it was the removed guard's
interception point, so the three session sinks return to origin/main.
- One traced(kind, mutate) wrapper in the commit file replaces the span
factory; closeLeafOrTab is one call.
- The ratchet walks src/main with the shared scanSourceTree, drops the
loading-store-internal rows and the redundant file-exists test; exact-set
equality already fails on a missing file.
- The commit test is a pure unit test of the span outcomes: no Store
harness, electron mock or self-comparing close.
* refactor(terminal): census the runtime session controller's write and drop stage ids from comments
The controller's setter was named set, which the boundary census could not
list without matching every Map.set, so a new OrcaRuntime mixin could write
sessions through it unseen. Rename it setForWorktree and census it.
Comments now describe state instead of citing plan stage ids.
* refactor(terminal): census writer references and trace refusals by callback
- traced() takes refusalOf instead of assuming an Error refusal, and only
mutate() sits in the try, so a span outcome of threw means the write threw.
- The boundary ratchet counts references, not just direct calls: non-null
calls, bracket keys, aliases, destructures, .call/.bind and parenthesized
callees all count; declared names and type positions do not.
- Census terminalSurfaceCloseMutation (boundary-only) and the partition sinks
setLocalWorkspaceSession / setHostWorkspaceSession.
* test(terminal): count writer uses in extends clauses and instantiations, skip type-only imports and local declarations
The census skipped ExpressionWithTypeArguments as a type, which also holds
`extends f(x)` and `x<T>` value expressions. Type-only import/export
specifiers and declared names (variables, parameters, accessors, enum
members) no longer count as uses. The audit fixture is listed in the table
instead of a separate exemption.
* test(terminal): count quoted and assignment-pattern destructures of layout writers
* test(terminal): count every mention of a layout writer except its definition
Telling definitions from uses per syntax kind kept missing nested and
for-of destructures. Exempt only the writer's own function or class-member
definition; any other mention (including object-literal keys) counts, so the
census errs toward a loud false alarm rather than a silent miss. Quoted names
count only in member-name position.
* test(terminal): count every string literal naming a layout writer
Member-name positions missed wrapped keys like store[('name')] and
store['name' as const]. Counting every string literal outside types is
shorter and errs toward a loud false alarm.
* test(terminal): exempt only class members and functions as writer definitions
Object-literal methods and accessors were exempt while equivalent arrow
properties counted; all object-literal keys now count alike.
* test(terminal): parse files with unicode escapes in the writer census
A name spelled with a \u escape never appears verbatim, so the text
prefilter skipped it.
* test(terminal): parse any file with an escape in the writer census
\x, identity and line-continuation escapes also decode to a writer name
without it appearing verbatim.
|
||
 Brennan Benson
|
6835b9b4e3 |
fix(mobile): paired clients re-derive a kept terminal after a cold restore (#23109)
* fix(mobile): paired clients re-derive a kept terminal after a cold restore A renderer frame published before a cold-restored terminal's PTY registered was fenced to an empty tab list and recorded as accepted, and the renderer never resends unchanged content. When registerPty binds a surface the accepted frame fenced out, re-merge that frame so the fence reads current state. * test(mobile): drive the live desktop window through the runtime's desktop seam * test(mobile): the re-derive path never flushes the store synchronously * test(mobile): a re-derived frame must not bring back a surface the host retired after accept * fix(mobile): a re-derived frame changes membership only for the registering surface The replay re-ran the whole accepted frame, so a surface the host retired after accept (a phone close whose remote PTY is still exiting, or a closed chat tab) came back. Every other surface now keeps the host's current decision; the removal repair is extracted from the terminal retirement helper so non-terminal tabs are removed the same way. * test(mobile): a re-derived frame must not drop or disown a phone-created terminal the desktop has not published * fix(mobile): a re-derived frame does not infer renderer retirements from its older frame * revert(mobile): drop the replay of a fenced renderer frame Reverts the production parts of |
||
  
|
0e3b71f605 |
fix(session): give an SSH workspace one owning partition so its tabs stop round-tripping as deletions (#19572)
* fix(session): give an SSH workspace one owning partition so its tabs stop round-tripping as deletions `workspaceSessionPartitionHostId` answered differently depending on who asked: the renderer mapped an SSH worktree's session to the `local` blob, the main-process runtime read-modify-wrote `ssh:<targetId>`. One workspace's session lived in two stores and no reader reunited them, so whatever landed on the unread side did not read as unknown — it round-tripped as absence. The remote-workspace upload is a `replace-session` patch, which turned that absence into deletion on the host, and the next pull applied the deletion locally and re-poisoned the snapshot. Collapse the two answers into one: every non-'local' host owns its partition. Boot hydration and the export fallback now read the SSH partition, and rows a shipping build left in `local` are folded back in once, gap-filling only — an empty tab row is a gap, never proof that anything was closed. Folder workspaces deliberately keep their existing 'local' routing: boot discovers SSH partitions from the repo catalog, so an SSH target that owns only a folder workspace has no partition any reader enumerates. They are still adopted back out of an SSH partition when a repo does name the host. Fixes #12721 Supersedes #12722 Co-authored-by: Robert Nisipeanu <github@nisipeanu.com> Co-authored-by: Jinwoo-H <Jinwoo-H@users.noreply.github.com> * test(session): pin the old-client empty-publish skew direction * fix(session): adopt every workspace the host partition names, not only tabbed ones Review caught that gating adoption on `host.tabsByWorktree[key].length > 0` traded the #12721 deletion for a narrower one. The write path routes EVERY worktree-scoped field to the owning partition, so an SSH workspace with open editor files or browser tabs and no terminals had all of it dropped on every restart — and unlike terminal state it cannot be recovered from the host snapshot, which carries terminal fields only, so an unsaved `dirtyDraftContent` was destroyed outright. The defect was not a missing field. It was a hand-maintained field list deciding what the read recovers while the write used the ownership table, so the two could disagree. Adoption now walks `WORKSPACE_SESSION_FIELD_OWNERSHIP` with an exhaustive switch, and a new ownership kind is a compile-time decision rather than a silent omission. Session keys are normalized through the shared `normalizeWorkspaceSessionKeyToWorkspaceId` so host-qualified visit recency (`ssh:target|worktreeId`) reaches its workspace, and the regression is pinned by feeding the shipping split's own output back through the real boot read rather than a hand-built fixture. Co-authored-by: Robert Nisipeanu <github@nisipeanu.com> Co-authored-by: Jinwoo-H <Jinwoo-H@users.noreply.github.com> * fix(session): stop adoption overwriting rows it was never told about Three losses, one cause: the reader walks its own description of the partition layout while the writer walks another, so the two agree on which ownership kinds exist and not on what a kind means. - An empty host row replaced a populated base row, destroying an unsaved dirtyDraftContent the header comment says must never be destroyed. The host holding nothing is not evidence the base is wrong. - A contested bare id was adopted as if local and ssh:<target> were one workspace written twice, which is exactly the id where that premise is false. The read already reached that verdict and adoption could not ask for it, so it is passed in; contested keys are gap-filled, never replaced. mergeWorkspaceSessionsWithHostShadow now reports the real contested set, which primaryHostBySessionKey never was. - Tab-, pane- and file-keyed rows are adopted through the split's own indexes, so unified-only tabs come back and the pane key is parsed once. - A bare lastVisitedAtByWorktreeId key only fills a gap; the split has a dedicated branch for that field and the reader had none. * test(session): pin the tombstone/gap boundary the two readings meet at An explicit empty tabsByWorktree row means the user closed the last terminal; adoption reads an empty base row as a gap to fill. Same value, opposite readings, so the boundary is asserted rather than argued: the tombstone lands in the owning partition, restores as a present empty row rather than a deleted key, is declined by the real seeding predicate, is published as an empty list, and the legacy-transition resurrection happens once and cannot recur. * docs(reliability): record the adoption guards and the tombstone boundary in the gate * test(e2e): read the SSH restart assertions from the partition that owns them ssh-cold-activation-restore asserted persistence through session.get() with no host, which is the local partition an SSH worktree's rows no longer live in. The invariant it means to check is that the state is persisted where the boot read will find it, so it now unions local and ssh:<targetId> and stays correct on both layouts. Confirmed the product invariant separately rather than by the edit: the behavioural half of both tests - the full app restart, the active worktree, the eager terminal remount and the PTY-owner reclaim against a real Docker OpenSSH host - runs after this check and passes. 2 passed in 48.9s. * test(e2e): read ssh-restart-tab-accumulation from the owning partition too Same layout-coupled read as ssh-cold-activation-restore: the pre-quit flush asserted through session.get() with no host. Verified against a real Docker OpenSSH target - both repeated quit/relaunch cycles keep exactly the restored SSH tabs, no accumulation and no loss. 2 passed in 52.9s. * fix(lint): clear the casting gate on the partition adoption main tightened typescript/consistent-type-assertions to assertionStyle: never, which the rebase brings onto these added lines. Most of the round-trip fixtures did not need a cast at all -- three were hiding wrong-shaped literals (a browser workspace keyed 'name', a unified tab keyed 'type', a layout keyed 'direction'), now written as the types they stand for. The adoption reads narrow through an isRecord predicate instead of casting, which also stops a null entry throwing out of Object.keys. What is left is dynamic-field writes and unknown-typed IPC returns, each with its own SAFETY rationale. * fix(session): give an SSH folder workspace one owning partition boot can find The partition owner rule already names `ssh:<targetId>` for a repo-backed worktree, but `getFolderWorkspacePartitionHostId` still answered 'local' for a folder workspace while main's `RuntimeWorkspaceSessionController.getPreferredHostId` answered `ssh:<targetId>` for the same key. That is #12723 unfixed for folder workspaces, and once the renderer started writing `ssh:*` at all it got worse: a save's field-level patch carries only the rows routed to that partition, so a `tabsByWorktree` write without the folder row erased the row main had put there. The reason the renderer could not route there was real - boot discovered SSH partitions from the repo catalog, which cannot name a target whose only workspace is a folder. So persistence now answers that directly over `session:list-host-ids`, and boot reads the partitions that exist rather than the ones a catalog implies. Removing a folder workspace prunes its rows from the owning partition too, or the census would adopt them back on the next launch as a workspace the user already deleted. Adoption now decides from the repo catalog instead of from co-presence. Two partitions holding one bare `repoId::path` is not evidence of a collision - that is the exact shape the repair exists for - so the verdict comes from `resolveWorktreeExecutionHost`: a repo id registered on more than one host is contested and may only be gap-filled, and one the catalog positively resolves to a different host is residue this partition does not own and is not adopted at all. Without the second rule a stale partition sorting first won the read and was then written into the live one. Nothing is deleted either way; the rows stay where they are. Finally, a workspace adopted out of a partition now routes back to that partition. Routing used to re-derive an owner from the catalog, so a boot whose repos had not hydrated moved the rows it had just reunited back into 'local' and re-stranded them. Contested ids are withheld from that override, because routing the whole bare id to one host is the loss the gap-fill prevents. The publish path resolves each workspace's owner once for the whole publish, shared with the projection, so the per-target catalog attribution does not repeat it per connected host. * fix(session): drop a deleted workspace from every partition, not just the local blob Adversarial review of the previous commit found three ways the partition census - which now reads whatever persistence holds rather than what the repo catalog implies - keeps rows alive that nothing should keep alive. `deleteProjectGroup` pruned only the local blob, so every folder workspace under a deleted group left its rows in `ssh:<targetId>`; the next boot adopted them back, named that partition their owner and wrote them there again, forever. `removeFolderWorkspace` had the same hole for a workspace whose partition its host expression could not name: main never persists a folder workspace's `executionHostId`, and `RuntimeWorkspaceSessionController` can infer a connection from the group's repos that the workspace row itself does not carry. Deriving the partition at delete time is the wrong question - a deleted workspace owns nothing anywhere - so both paths now remove it from every partition. The third is on the read side. A contested id is deliberately withheld from the read-source override so the write cannot carry one host's rows into another's partition, but the routing that then re-derives an owner answers 'local' for an id the catalog cannot name. Adopting such a row moved it out of the partition that owns it and into the blob: the two-store split this change exists to remove. A contested id the assembled session holds no row for is therefore not adopted at all. Gap-filling stays available for a contested id the session already names, since that row's own partition is what the write follows. Declining to adopt leaves a row invisible for one boot; it never deletes one. Also: the folder-key guard in both catalog attributions was dead, because `getRepoIdFromWorktreeId` hands back the whole key rather than nothing when there is no `::`. The verdict was right and the resolution wasted; it now skips by shape. And the two type assertions the changed-code casting gate rejected are gone rather than suppressed. * fix(session): park the rows a partition read declines instead of letting the next write erase them A partition write replaces each field with exactly what the unified session routed there. So a row the read left out of that session is erased from its own partition the moment any sibling workspace writes the same one - and with SSH partitions now the owning store, that row is then in no partition at all. Three separate decisions produce such rows: residue the catalog attributes to another host, a contested id withheld so the write cannot carry one host's rows into another's partition, and a workspace the base already holds the live copy of. Declining to show a row was quietly deleting it. The machinery for this already exists. `attachHostSessionShadow` writes a contested runtime co-claimant's parked rows straight back into its own slice before the write, so the primary's write cannot erase them; the ssh partitions simply were not among the slices the contention split arbitrates. The read now parks everything it is not returning to an ssh partition into that same shadow, and the existing re-attach puts it back. Leak, never kill - docs/reference/ssh-execution- boundary.md - and a row no partition holds is unrecoverable. Second, the contested branch of the tab adoption read `Object.hasOwn` as "the base has tabs here". An empty list satisfies it, so whenever a legacy id happened to be contested, #12721's empty local row won over the host's real one - the exact reading the module's own header, and the gate invariant it is pinned by, say is wrong. An empty row is the gap this repair fills, so it is now treated as one. * test(session): pin the empty-base-row gap for a contested id Mutation testing found the assertion missing: reverting the gate to `Object.hasOwn` left all 39 assertions passing, which makes the fix that reads an empty base tab row as a gap unguarded. The #12721 shape does not stop being a gap because the id happens to be contested. --------- Co-authored-by: Robert Nisipeanu <github@nisipeanu.com> Co-authored-by: Jinwoo-H <Jinwoo-H@users.noreply.github.com> |
||
|
Jinwoo Hong
|
573537ecd4 |
feat(cli): make terminal close the canonical workspace teardown (#18073)
* fix(runtime): recover stale session owners and await retirement * fix(runtime): preserve session hydration and smoke compatibility * test(runtime): cover empty and unindexed session owners * feat(cli): make terminal close the canonical workspace teardown * fix(preload): align ssh termination result type * test(runtime): assert folder hydration owner * fix(runtime): fence legacy terminal stop by worktree host * fix(preload): reconcile ssh result import with main * fix(runtime): keep same-id sibling hosts out of workspace close The stale-owner fallback in the session controller re-routed any worktree whose catalog partition had no tabs to whichever other partition held tabs. Only `runtime:` environment ids rotate across relay restarts; `repoId::path` legitimately repeats across hosts, so an SSH workspace close could retire the local copy's tabs and resume records, or flip owners mid-close and strand the SSH PTY. Restrict the fallback to runtime hosts, and pin the session partition once per workspace close so record clearing targets the partition that owned the tabs. * test(runtime): give the cross-host close fixture a real resume record * fix(preload): take main's ssh-bridge import order so the merge stays duplicate-free |
||
|
Neil
|
c61ca56a9b |
fix(ssh): resolve the worktree's execution host instead of guessing from one repo row (#17909)
* fix(host-routing): resolve the execution host before reading a connection Three issues in one defect class: a resolver reads one spelling of one arbitrarily chosen row instead of resolving the worktree's execution host, so something local answers a question about a remote. returned that row's connectionId. With duplicate repo rows for one repo id it could pair a runtime owner with a client-owned SSH connection. It now resolves through the same ambiguity-aware index getRuntimeEnvironmentIdForWorktree uses, prefers the repo row for the host the worktree names, and derives the connection from the resolved host. Conflicting rows return `undefined` (this module's documented "cannot determine the host"), never `null`. `store.getRepo(worktree.repoId)?.connectionId ?? null`. `getRepo` is host-blind and the same repo id can exist on local, SSH and runtime hosts, so a remote worktree could spawn its PTY on the client with the remote cwd. resolveWorktreeLaunchHost picks the row for the worktree's host and reads the connection off that host; conflicting rows are unresolved, not local. session-partition owner maps that contradict each other. Both now compute through one shared function whose argument records the divergence. No behaviour change on either side: converging needs a read-both migration, since both partitions hold real data written by shipping builds. * fix(host-routing): keep nested SSH connections resolvable under a runtime host getRepoSshConnectionId read only the resolved execution host, so a repo row owned by a runtime that reaches a nested SSH target (connectionId: ssh-*, executionHostId: runtime:*) resolved to no connection — answering 'local' for a remote worktree, the same defect #17909 fixed in the other direction. * fix(host-routing): resolve both sides of the execution host through one rule The renderer resolver leaked between two different SSH hosts: a worktree on `ssh:m4air` whose only indexed repo row belonged to `openclaw` answered 'openclaw', because the host-scoped lookup missing fell through to an id-only one. Main's resolver, in the same change, answered 'm4air' — two resolvers, one right and one wrong, on identical input. Both sides now adapt one shared rule (`worktree-execution-host-resolution.ts`): the worktree's own host outranks every repo row, and a row on a different host is never evidence about this one. The renderer's WeakMap index becomes the memoizing adapter it always was; `resolveWorktreeLaunchHost` becomes main's mapping of unresolved onto its throw. Settles the rule the change previously answered two ways. `getRepoSshConnectionId` and `getSshTargetIdForExecutionHost` disagreed for a runtime host carrying a nested `connectionId`; they now compose, so the execution host is the single authority. On a `runtime:*` row that field is a paired HUB's private SSH target, spread through by `repoWithFetchedOwner` and unaddressable from this client — the project-first successor of the row nulls it for exactly that reason. That also fixes the `kind !== 'ssh'` fallback, which fired for `local`: a row declaring itself local handed out an SSH connection. |
||
|
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 (
|