feat(workspace): F2 dispatch-seam freeze — inert branch hooks + ContextVar + op-vocab (#168)

[pzhang15]

Overview

Phase 0 / F2 — the dispatch-seam freeze (#168, epic #166, roadmap #165).

This is a foundation freeze, not a feature. It adds dead-until-Lane-A hook seams to the one VFS chokepoint (workspace/dispatcher.py) so the write-layer lanes (B/C) and the commit lane (D) plug in by providing objects the hooks call — never by reopening dispatch(). With no branch bound, behavior is byte-for-byte unchanged: every hook is gated on branch is not None, and nothing binds a branch yet (that's Lane A / #171), so all new branches are unreachable today.

Stacked on F1 (#206). F2 consumes F1's BaseResource.fork substrate, which is not yet on main (main is not an ancestor of feat/phase0-f1-fork). So this PR is based on feat/phase0-f1-fork and should be retargeted to main once F1 (#206) merges. The diff shown here is F2-only.

Detailed changes

observe/context.py — the _active_branch ContextVar trio

Mirrors the existing _revisions trio exactly:

• _active_branch: ContextVar[object | None] (default None) — typed object | None; the real Branch class is Lane A's and is deliberately not imported (would be a cycle).
• push_branch(branch) -> Token, reset_branch(token), branch_for() -> object | None.

Task-isolated, same as _revisions.

workspace/branch_vocab.py (new) — the frozen op-vocabulary

The single source of truth all lanes import: READ_OPS and WRITE_OPS. Leaf module (no workspace imports → no cycle). dispatcher.py's _DISPATCH_READ_OPS / _DISPATCH_WRITE_OPS now re-point here (the frozensets are byte-identical to the originals, so existing call sites are unchanged).

workspace/dispatcher.py — inert hooks in dispatch()

• branch = branch_for() near the top (None today).
• _needs_staging(mount) — the share-by-ref classifier: type(mount.resource).fork is BaseResource.fork. A resource that did not override BaseResource.fork shares its live backend by reference across a fork and must be staged. Deliberately NOT keyed on is_remote — a Redis mount is is_remote=False yet still shares by ref. (RAM/cache/Disk override fork → isolate → no staging; Redis/S3/Slack/GDrive inherit the default → need staging.) A comment notes F4 ([P0] F4: Write-layer protocol + resource strategy seam + store flags #170) may swap this predicate's internals but not its call site.
• a write-layer-first read hook (branch.staged_read(mount, path, **kw)) and a write-divert hook (branch.divert_write(mount, op, path, **kw)), both inside if branch is not None and _needs_staging(mount): and gated by op-vocab. Both return (result, IOResult) like dispatch. These method names are the contract Lane A implements.
• a policy_for(mount) stub returning None (documented for F3/F4/Lane D to fill).

Everything below the guard is the existing dispatch logic, unchanged.

commands/builtin/cross_mount.py

#168 mentioned fixing this dead module's "op-name mismatch." It was deleted on main by Lane F (#186) — but F1 predates that deletion, so the file is still present on this stack's base. F2 does not touch it: its dispatch calls already use canonical op names (read_bytes / write_bytes / unlink), so there is no mismatch on this base, and the deletion is Lane F's to carry. It drops out when this stack rebases onto post-#186 main.

Tests

• tests/observe/test_context.py — branch trio: branch_for() is None by default; push → returns it → reset restores previous; nested push/reset.
• tests/workspace/test_branch_vocab.py — vocab contents, disjointness, and that the dispatcher re-exports the same set objects.
• tests/workspace/test_dispatcher.py — _needs_staging across RAM (False), Redis (True, is_remote=False), S3-style remote (True), plus policy_for default. (Redis's real __init__ connects to a live server, so the test builds the real RedisResource type via object.__new__ — the gate only inspects type(resource).fork, keeping the test hermetic.)
• Byte-for-byte proof: the full existing suite (excl. tests/fuse, which needs native libfuse) passes with no new failures; the only failures are pre-existing/environmental (test_tac.py — no system tac on macOS; one redis test — no redis-server).

Definition of Done (#168)

• With no branch bound, the full existing test suite passes byte-for-byte.
• push_branch / reset_branch / branch_for added to observe/context.py, mirroring _revisions, with a unit test.
• A Redis mount (is_remote=False, share-by-ref) is classified as needing staging — explicit _needs_staging test across RAM/Redis/S3.
• Documented invariant: after this merge, dispatcher.py needs no further edits from any other lane (stated in the Dispatcher docstring + here).
• pre-commit run --all-files clean and relevant tests green.

The invariant

dispatcher.py is now read-only for all downstream lanes. Lanes A–J extend dispatch behavior solely by providing the bound branch object's staged_read / divert_write methods (Lane A) — they never reopen dispatch(). The seam call sites are frozen; F4 (#170), a sibling Phase-0 sub-issue, may refine the _needs_staging predicate internals and flesh out policy_for, but not the call sites.

🤖 Generated with Claude Code

[pzhang15]

Self-review — F2 dispatch-seam freeze

Did a thorough pass over this PR (against the head commit, on the feat/phase0-f1-fork base). The core of the freeze is sound; flagging a few things for discussion before we declare dispatcher.py read-only for lanes A–J. Grouped by severity, with the substantive design questions first.

✅ Confirmed sound (no action)

• The seam is genuinely inert / byte-for-byte holds. With no branch bound, the addition is one ContextVar read plus a short-circuited guard — _needs_staging isn't even evaluated (branch is not None short-circuits first), and everything below (cache lookup, execute_op, write invalidation, return shape) is unchanged. This isn't just asserted in prose: the existing dispatch/cache regression suite (test_dispatch_write.py, test_cache_mount.py, test_native_dispatch.py, …) runs every routed op with branch=None and would fail on any perturbation of the path below the guard. No import cycles (branch_vocab.py is a pure leaf; context.py/base.py don't import workspace).
• Style/convention axis is clean — imports at top, no nested functions, paths stay PathSpec, no top-of-file docstrings, no per-line comments. branch: object (cycle avoidance) and the missing -> Token on push_branch both match the existing push_revisions house style.

🟡 Design / contract completeness (the ones worth deciding now)

1. The op-vocab makes the frozen contract incomplete for metadata + directory ops. The seam only diverts READ_OPS / WRITE_OPS. But stat, readdir, and find also flow through dispatch() (Dispatcher.stat/readdir, Workspace.stat/readdir, and cross_mount dispatch("find", …)) and are in neither set — so on a share-by-ref mount with a branch bound, they fall through to the live backend. Once staging is live, a file written via divert_write won't stat, and readdir/find will hide staged creations and still list staged-unlinked files. Concretely, cp -r already mixes the two: cross_mount.py does dispatch("find") (→ live tree) then dispatch("read_bytes") (→ staged_read) — the two ops disagree about what exists.

Separately, mkdir / rmdir / rename don't even traverse dispatch() — they run via mount.execute_cmd straight on the accessor — so they structurally cannot be caught by this seam at all.

The freeze's invariant is "extend solely via staged_read/divert_write, never reopen dispatch()." As written, that invariant isn't satisfiable for any feature that needs staged files visible to stat/readdir/find. Two ways to close it, both cheap and in-scope for this freeze:

• add a third bucket (META_OPS → branch.staged_meta(mount, op, path, **kwargs)) so metadata ops can be reconciled without reopening dispatch(), or
• explicitly document in the Dispatcher docstring that stat/readdir/find (and the execute_cmd dir-mutation path) are intentionally not staged in this arc, and why that's safe (e.g. content-only staging, metadata always live) — and note that CoW of dir ops will need the execute_cmd/IOResult write-set path, not this seam.

2. policy_for() is a stub with no call site. Unlike staged_read/divert_write (which have inert call sites in dispatch()), policy_for is an uncalled module function returning None. It's fine as a deferred stub, but "freezes the call shape so they need not reopen the dispatcher" is loose wording — an uncalled function doesn't anchor a call site the way the routing hooks do. (To be clear: this does not force a future dispatch() edit — policy is plausibly resolved at commit time, outside the per-op path, per the docstring itself.) Worth confirming the mount-only signature is the shape Lane D/F3 will actually need; if they need op/path/branch context, the stub freezes the wrong shape.

3. _needs_staging keys on method-override identity, not behavior. type(mount.resource).fork is BaseResource.fork tests "did the resource override fork()", which the docstring describes as "shares its backend by reference." Those are equivalent for today's three backends but diverge for any resource that overrides fork() yet still shares its live client (returns self after bookkeeping, or a shallow copy reusing the same aioboto3/redis handle) — it'd be classified as isolating and its staged writes would silently hit live. No resource does this today and the predicate's internals are explicitly licensed to change in F4 (#170), so this is low-severity — but (a) tighten the docstring to say it tests override-identity (an approximation), not actual share-by-ref, and (b) consider making it fail-closed later (an explicit isolates_on_fork opt-out flag) so an unknown/custom fork defaults to staging rather than leaking.

🟡 Tests

4. The active routing block is never executed by any test. The new behavior the freeze exists to lock — branch.staged_read / branch.divert_write / fall-through for non-vocab ops — is asserted only in the docstring/comment. Tests cover the context trio, vocab contents/identity, _needs_staging classification, and policy_for default, but nothing binds a branch and routes an op through dispatch(). For a PR whose deliverable is a frozen contract, that contract should be an executable regression guard. Suggest adding a _FakeBranch (records calls, returns (result, IOResult)) and asserting, via push_branch/reset_branch: read_bytes → staged_read(mount, path); write → divert_write(mount, op, path); a non-vocab op (stat) bypasses both → live; and a RAM mount (_needs_staging False) bypasses the branch even for a read. No Lane A code required (branch is typed object).

5. (nit) test_dispatcher.py imports RedisResource/S3Resource at module top without importorskip, so collection hard-fails without the redis/aioboto3 extras — and that also takes down the two extra-free cases (_needs_staging RAM, policy_for) in the same file. This matches the prevailing suite convention (module-top imports gated by skipif) and passes in CI where --all-extras is installed, so it's only an ergonomics nit for extra-light local runs. Optional: importorskip the optional backends, or split the RAM/policy_for cases into an import-clean module.

⚪ Nits

6. Inside dispatch(), the new block uses bare READ_OPS/WRITE_OPS while the pre-existing cache/invalidation lines use the _DISPATCH_READ_OPS/_DISPATCH_WRITE_OPS aliases for the same objects — one concept, two names in one method. Pick one (simplest: the seam uses _DISPATCH_* like its neighbors).

7. policy_for is public but _needs_staging is private — both are seam stubs. If policy_for is meant for downstream lanes to import (parallel to the public READ_OPS/WRITE_OPS), the asymmetry is intentional and fine; otherwise _policy_for. Worth a confirming note either way.

8. The "dispatcher.py is read-only / never the call site" invariant is restated three times in one file (class docstring, _needs_staging docstring, and an 8-line inline block in dispatch()). State it once (the class docstring is the natural home) and trim the inline block to the load-bearing one-liner.

---

Automated self-review via Claude Code — multi-lens pass with an adversarial verification stage. Findings are prioritized; the design questions (1–3) are the ones I'd most like a second opinion on before merge.