Skip to content

test: migrate e2e from Playwright runner to Cucumber - #7

Merged
srid merged 4 commits into
masterfrom
claude/intelligent-tharp
Mar 20, 2026
Merged

test: migrate e2e from Playwright runner to Cucumber#7
srid merged 4 commits into
masterfrom
claude/intelligent-tharp

Conversation

@srid

@srid srid commented Mar 20, 2026

Copy link
Copy Markdown
Member

Summary

Replace @playwright/test runner with Cucumber.js + raw Playwright. Same Chromium browser, same test coverage (9 scenarios, 42 steps), lower runner overhead.

Architecture

.feature files       →  WHAT to test (plain English / Gherkin)
step_definitions/    →  HOW to test (Playwright calls)
support/world.ts     →  shared state + helpers per scenario
support/hooks.ts     →  setup/teardown (browser, server, screenshots)
cucumber.js          →  config (which files to load, output format)

Key concepts

Gherkin (.feature files)

  • Feature: — top-level grouping, one per file. Text after it is documentation.
  • Scenario: — one test case. Each runs independently (fresh browser context).
  • Given/When/Then/And — steps that get pattern-matched to step definitions. And is an alias for whichever keyword came before it. Convention: Given = setup, When = action, Then = assertion.
  • Background: — steps that run before every scenario in the file (shared setup).

Step definitions (step_definitions/*.ts)

Each step in a .feature file is matched to a function by its string pattern:

// {string} matches quoted text, {int} matches bare numbers
When('I resize the viewport to {int}x{int}', async function (this: KoluWorld, w: number, h: number) {
  await this.resizeViewport(w, h);
});
  • {string} captures "quoted text" → passed as string parameter
  • {int} captures bare numbers → passed as number parameter
  • (s) in patterns like {int} time(s) makes the "s" optional (matches "1 time" and "2 times")
  • Must use function(), not arrow functions — arrow functions break Cucumber's this binding
  • Steps are global — defined once, usable from any .feature file

World (support/world.ts)

The World is the this context shared by all steps within a single scenario:

  • Cucumber creates a fresh instance per scenario — no state leaks between tests
  • setWorldConstructor(KoluWorld) tells Cucumber which class to use
  • Steps communicate within a scenario through World properties (e.g., Given I note the canvas dimensions saves to this.savedCanvas, then Then the canvas should be smaller reads it)
  • setDefaultTimeout(60_000) is Cucumber's per-step timeout, independent from Playwright's locator timeouts

Hooks (support/hooks.ts)

Lifecycle in execution order:

BeforeAll  (once)          → start server, launch Chromium
  Before   (per scenario)  → new browser context + page, wire error listener
    Given/When/Then        → steps run using this.page
  After    (per scenario)  → screenshot if failed, close context
AfterAll   (once)          → close browser, kill server

Browser is expensive → created once. Each scenario gets a fresh context (like incognito window: clean cookies, storage, page). The module-level let browser bridges BeforeAllBefore since hooks can't return values.

Why migrate?

  • Test-first development: Gherkin scenarios are readable specs — write the scenario before writing the feature
  • Future dual-profile: same .feature files can drive both API tests (direct HTTP) and UI tests (Playwright) with different step definition sets
  • Lower overhead: ~9s vs Playwright runner's heavier worker/project/reporter initialization

Test plan

  • All 9 scenarios (42 steps) pass with just test-dev
  • just pc (pre-commit) passes

srid added 3 commits March 20, 2026 09:16
Replace @playwright/test runner with @cucumber/cucumber + raw Playwright.
Same Chromium browser, same test coverage (9 scenarios, 42 steps), lower
runner overhead.

- Add .feature files (Gherkin) for smoke and terminal scenarios
- KoluWorld class absorbs the old DSL helpers
- Hooks manage browser lifecycle, server spawn, screenshot-on-failure
- Update justfile recipes (test, test-dev); remove test-ui
- Add tests/README.md documenting the setup and Playwright comparison
- Remove unused execSync import
- Rename PLAYWRIGHT_REUSE_SERVER -> REUSE_SERVER
- Extract duplicated WS monkey-patch into wsInterceptScript()
- Rename savedCanvas2 -> previousCanvas for clarity
@srid
srid marked this pull request as ready for review March 20, 2026 14:21
@srid
srid merged commit 99deec7 into master Mar 20, 2026
5 checks passed
@srid
srid deleted the claude/intelligent-tharp branch March 20, 2026 14:31
srid added a commit that referenced this pull request May 19, 2026
… with gap signal

Last two reviewer findings rolled in. The PR now resolves every item
from the audit comment.

**Reviewer #5 — helper socket race + no auth.**
- 0700 HELPER_DIR + 0600 daemon.token (32 random bytes per daemon).
- `daemon.lock` acquired via O_CREAT|O_EXCL; PID written to it.
  If the lock exists but the holder is dead (`kill -0` → ESRCH),
  unlink and retry. If alive, refuse to start. Stops the racing
  second daemon that used to unlink the live socket and orphan
  the in-flight client's PTYs.
- Every connection starts in auth-pending state. The relay's first
  line MUST be `{"auth":"<token>"}\n`; mismatch / malformed /
  oversized auth → socket destroyed before any PTY-op surface is
  exposed. The relay reads the token from the same per-user
  HELPER_DIR (the daemon's own user owns it), so a hostile process
  on the same box can't connect blind.
- 5s auth timeout fires if the client never sends the frame.
- Shutdown unlinks the lock, token, pid, and socket together.

**Reviewer #7 — replay buffer gap signal.** Ring buffer is now byte-
bounded (`RING_BUFFER_MAX_BYTES = 4 MiB`) instead of event-bounded
(4096 events). High-output reattach over an SSH drop no longer
silently sheds whole megabytes. When `attach(sinceSeq)` finds that
the buffer's earliest event is past `sinceSeq + 1`, the daemon emits
a one-off `replayGap` event before the replay sequence. The
controller resets the headless xterm, pushes `\x1b[2J\x1b[H` to the
client, and lets the live stream rebuild the screen — no corrupted
scrollback.

Schema bumped: `HelperReplayGapEventSchema` added to the wire union;
`manager.replay()` now returns `{events, gap}`.

`PtyReg.onReplayGap` wired in `host/remote.ts` so the controller-side
xterm resets when the daemon signals a gap. Tests updated to the new
replay return shape.
srid added a commit that referenced this pull request May 27, 2026
Codex review findings #6 and #7:

- `build:agent` in `package.json` pointed at `tsconfig.agent.json`
  which doesn't exist (and the agent runs through `tsx`, no `tsc` emit
  step anywhere in the flow). Removed.
- `packages/surface/README.md` said to pass `implementSurface(...).router`
  directly to `serveOverStdio` — that's the exact double-prefix
  footgun the regression test pins against. Updated the snippet's
  comment and added a "Router wrapping (footgun)" paragraph explaining
  why `implement(contract).router({...fragment.router})` is required.
- `packages/surface/example/remote-process-monitor/README.md` still
  described the old v1 closure-copy model (`AGENT_PATH`,
  `nix copy --to ssh://`). Replaced with the `.drv`-copy + remote
  realise flow that the code actually does (`KOLU_AGENT_DRV` env,
  `nix copy --derivation`, `nix-store --realise` on the remote),
  including the single-binary `nix run .#process-monitor-monitor`
  entrypoint and the `trusted-users` requirement on the remote.
srid added a commit that referenced this pull request May 30, 2026
Constraints #7 (session backup) and #8 (preferences storm) split out as
GitHub issues #1040 and #1041. Scorecard + constraints list now link them,
and a 'Decision — salvage, in progress' note records that #5 shipped while
#6, #1-3, and #4 are being implemented onto this PR.
srid added a commit that referenced this pull request May 30, 2026
…t shipped

Export/Import session (#1046, sessionTransfer.ts) is the SavedSession
backup/restore hatch constraint #7 asked for — #1040 closed. Also record the
always-visible build-id readout shipping in 67567d0 (#1047 closed).
srid added a commit that referenced this pull request Jun 2, 2026
Paulg-method rewrite of the 14 panel-flagged passages. 11 applied
after an adversarial skeptic confirmed each cleared the reader's
stumble while preserving meaning, facts, the physics metaphors, and
voice. 4 of the 11 are the skeptic's own endorsed fix (the rewriter's
version was rejected and replaced).

Headline fixes:
- #1 "the invariant" now named ("the code's analogue of that
  frame-invariant interval") — was undefined-on-a-definite-article.
- #6 "structural review has." -> "has become that." — elliptical
  gapping after a negative had inverted the meaning.
- #4 lede "volatilities that got bound" -> "fast-moving and
  slow-moving parts coupled" — one reader had read "bound" as the
  opposite (good) meaning.
- #12 moved "space-like" off the observer onto the slice/reading.
- #3, #7, #8, #9, #10, #11, #14: see optimization log.

3 deliberate keeps (over-fix guard): #5 (the physics is fine as-is),
#13 (gloss would steal the next paragraph's reveal), #2 (deferred to
pass 2 — pass-1 rewrite contradicted the post's thesis).

Behaviour gate: `just website::build` PASS.
srid added a commit that referenced this pull request Jun 6, 2026
#7 chord-arbitration (Family B, candidate): who owns a chord when a PTY
tool stacks under the app (Ctrl+B/J); tested but hand-shifted.
#8 capability-gate (Family B, extract-now): probe→degrade→notify scattered
at 5+ sites; tryFeature seam.
#9 terminal-probes (Family B, extract-now·partly-built): read volatile
xterm/WebGL internals via null-safe thunks; harden the leaks.
srid added a commit that referenced this pull request Jun 20, 2026
…e toggle

The Code-tab Markdown toggle remounted + re-sanitized the whole document on every
flip — active() returned only the active branch. FileView now keeps each toggle
mode alive once visited (hidden with display:none, not unmounted), so a flip is a
visibility change, not a remount: the marked -> DOMPurify -> tree-walks ->
image-resolve -> Shiki -> innerHTML pipeline runs 0x/toggle (was 1x). A per-slot
heldFile snapshot freezes a hidden mode and adopts the latest when shown, so
reload-on-edit is intact with NO render(file) API change, and an edit never
re-renders both modes at once. Single-form appliances (image/video/iframe) are
both()-gated out, unchanged.

Keeping both comment surfaces mounted exposed a latent fragility: the CSS Custom
Highlight registry is one global map keyed by name and applyHighlights REPLACES
the named highlight, so two surfaces under the shared "kolu-comment" name
clobbered each other. Each overlay instance now owns a per-instance highlight
name + style element; a hidden surface's ranges simply don't lay out and repaint
when it's shown again.

Proven by an e2e (the rendered preview element survives a Source/Rendered
round-trip; the comment-highlight survival e2e passes against per-instance names).

Ships item #7 of the performance map; full investigation in
docs/perf-investigations/markdown-image-resolver-and-toggle.md.
srid added a commit that referenced this pull request Jun 20, 2026
**The Code tab's Markdown Source ⇄ Rendered toggle rebuilt the entire
document on every flip** — `FileView` unmounted the inactive mode and
remounted the other, re-running the whole marked → DOMPurify →
tree-walks → image-resolve → Shiki → `innerHTML` pipeline. It now keeps
both modes alive once opened and flips *visibility*, so toggling is a
no-op for the renderer instead of a full re-sanitize.

It started as `/be "stabilize the markdown image resolver reference"`
(perf-map item #7) — but that literal ask is a **measured no-op**, so no
resolver patch ships. Stabilizing the inline-arrow reference eliminates
*zero* `sanitizeHtml` runs, confirmed three independent ways: a runtime
reproduction on the repo's real Solid build (fresh arrow vs stable
callback ⇒ byte-identical memo counts), `FileView`'s own
remount-on-snapshot design, and the Solid compiler (the inline-arrow
prop is **static**, never a reactive dependency). The reproduction
surfaced the *real* cost — the toggle remount — which is what this
fixes.

### The fix landed cleaner than scoped

- **No `render(file)` API change.** A per-slot `heldFile` memo freezes a
hidden mode's snapshot and adopts the latest the instant it's shown: a
toggle with no edit reuses the same snapshot (no re-render), an edit to
the _visible_ mode still re-renders it (reload-on-edit intact), an edit
to a _hidden_ mode defers until shown (no double-render).
- **`both()`-gated** — single-form appliances (image / video / iframe)
stay on the existing `active()` path, untouched.
- **Comment surfaces decoupled.** Two kept-alive `CommentTextSurface`s
would have contended for the single global `kolu-comment` CSS Custom
Highlight (`applyHighlights` _replaces_ it per call). Each overlay
instance now owns a **per-instance** highlight name + style — removing a
latent single-surface fragility.

### The win, measured

| | Before | After |
| --- | --- | --- |
| Render pipeline per toggle | **1×** | **0×** |
| 50-image doc, 3 round-trips (reproduction, real Solid build) | 150
image-resolutions + 3 pipelines | **0** |
| Rendered preview element across a round-trip (real app, e2e) |
remounted | **preserved** |

### Review hardening (the gauntlet earned its keep)

Keeping *both* comment surfaces mounted at once exposed a real bug the
lens/codex review caught and fixed:

- **Cross-surface comment ownership** (codex, major) — a path-only
comment filter fed each of the two now-live overlays the *other*
surface's comments, racing the shared scroll-request flow. Fixed by
making the overlay **surface-aware** (filter by `comment.surface`, gate
the scroll request on `surface`), plus a load-time **`backfillSurface`**
that routes legacy surface-less Markdown comments to `source` (the only
commentable surface before the rendered preview existed) — so every
comment has exactly one owner. Covered by a new unit test + a regression
e2e (*"Tray jump returns to the Source Markdown surface"*).
- Per-instance highlight name derived from `createUniqueId` (lens);
`aria-hidden` on the inactive slot to match `RightPanel` (lens); the
`visited` latch simplified to one memo (simplify).

### Tests

New e2e (*"Toggling Source and Rendered keeps the rendered preview
alive"*): a marker on the rendered preview survives the round-trip,
which a remount would erase. Full `code-tab.feature`: **115/115**;
comment unit tests **9/9**. CI green on both platforms.

Full write-up + reproduction:
[`docs/perf-investigations/markdown-image-resolver-and-toggle.md`](https://github.com/juspay/kolu/blob/master/docs/perf-investigations/markdown-image-resolver-and-toggle.md).
Closes perf-map item #7.

_Generated by [`/be`](https://github.com/srid/agency) on Claude Code
(model `claude-opus-4-8`)._
srid added a commit that referenced this pull request Jun 30, 2026
…view gauntlet)

Hardens the S1+S2 cutover against the 4-reviewer architecture-first-principles
+ perfection-review debate (debates/pr1626-review). Every residual was in the
shell around the fold; the fold spine was credited sound.

- #2/#7 — the restore target is now a fold-produced discriminated value:
  `RestoreTarget = none | { exact, command, agent } | { legacyMostRecent, command }`
  (`restoreTargetOf`, consumed by `resumeFormFor`), replacing the bare optional
  `resumeAgent` identity. Absence can no longer be read as "resume most-recent":
  quit-to-shell → `none` → bare shell by construction (#1492) for new records;
  migrated pre-1.29 records keep most-recent as a NAMED `legacyMostRecent`. The
  derivation moved out of the emit shell into the fold (closes #7). The client
  restore card + dormant tile require an actual `exact`/`legacyMostRecent` target.
- #5 — the authored collection no longer re-publishes on the ~150 ms observation
  firehose: the emit loop computes deltas once and gates each arm by its own
  (observed publish on an observed change, authored publish on an authored-fact
  change, disk autosave on a restore-relevant change).
- #1 — the recency frame phase is decided BY VALUE (compare the re-resolved agent
  identity to the saved restore target), deleting the 1.5 s wall-clock window and
  its restamp race.
- #8 — the schema-module docstring is rewritten to the Observation/AgentMemory/
  KoluAwareness world; the deleted AwarenessValue/AwarenessSink paragraphs are gone.

Deferral caveats made honest (not implemented): #3 the sleep drain stays deferred,
and the docs stop claiming the freeze "holds by construction" (a sub-second
launch-then-sleep now yields a false BARE shell under model B — fail-safe); #6
adds a fold.test.ts pin that a structurally-equal-but-new-ref git/pr still compares
equal at the autosave fence.

AgentIdentity + RestoreTarget move down to anyagent/schemas (the resume vocabulary
layer), re-exported through terminal-workspace. SCHEMA_VERSION migration reshaped
to synthesize the discriminated target. Docs (README, 5 Atlas notes, changelog)
reconciled to the shipped behavior.
srid added a commit that referenced this pull request Jun 30, 2026
## Awareness — the producer observes, kolu remembers (S1 + S2 → R9.0)

Implements the merged plan
[`awareness-derive-store.mdx`](docs/atlas/src/content/atlas/awareness-derive-store.mdx)
(#1621) as **one coordinated change** — S1 (the memoryless producer) and
S2 (kolu's fold) are coupled, so they ship together — then **hardened
across two rounds of a 4-reviewer architecture-first-principles +
perfection-review gauntlet** (`debates/pr1626-review`,
`debates/pr1626-rereview`). The reviewers credited the fold spine
**sound**; every residual was in the shell around it — the first round's
four must-fixes (below), then two relocations codex caught on re-review
(F1/F2).

### The cut

The per-terminal awareness engine used to **derive** awareness by
**mutating a host record it also read back as memory**, through an
`AwarenessSink` two homes (kolu + `pulam`) each had to wire. This splits
it:

- **S1 — memoryless producer.** `startAwareness` → `startSensors`: it
*emits* per-field `TerminalEvent`s and takes no seed, no sink, no
record. Deleted: the `AwarenessSink` interface, **both**
`makeAwarenessSink` impls (kolu's + pulam's `hooks.ts`),
`AwarenessRecord`, the apply-and-publish read-back.
- **S2 — kolu's fold.** A pure `fold(cur, o, ctx) → TerminalState`
(`{snapshot: TerminalSnapshot, memory: AgentMemory}`). The producer's
emit type **cannot spell** a memory fact, so the old "two narrowed
mutators" write-fence is now the **type**.
- **R9.0 — local cutover.** kolu runs the producer in-process and folds
the stream, publishing the snapshot half to `snapshots` and the memory
half to `kolu.authored`.

### Review-gauntlet hardening (the four must-fixes)

| # | Finding | Fix |
| --- | --- | --- |
| **#2 / #7** | The restore target was a bare optional `resumeAgent`
identity read alongside `lastAgentCommand`, so `(command set, identity
absent)` meant **both** "quit, restore nothing" *and* "no id captured,
resume most-recent" — and the derivation lived in the emit shell, not
the fold. | The fold now produces a **discriminated `RestoreTarget`** —
`none \| { exact, command, agent } \| { legacyMostRecent, command }`
(`restoreTargetOf`, consumed by `resumeFormFor`). Absence can no longer
be read as most-recent: **quit-to-shell → `none` → bare shell by
construction** (#1492) for new records; a migrated pre-1.29 record keeps
most-recent as a *named* `legacyMostRecent`. The client restore card +
dormant tile require an `exact`/`legacyMostRecent` target. |
| **#5** | The authored *collection* re-published on **every**
observation tick (~150 ms) — only the disk arm was fenced. | The emit
loop computes deltas once and gates each arm by **its own** delta:
snapshot publish on a snapshot change, **authored publish on an
authored-fact change**, disk autosave on a restore-relevant change. |
| **#1** | Recency leaned on a 1.5 s wall-clock window; a slow agent
resolution crossing it could restamp saved recency on restore. | The
frame phase is decided **by value** — compare the re-resolved agent
identity to the saved restore target. The timer is deleted. |
| **#8** | The schema module's docstring still taught the deleted
`AwarenessValue` / `AwarenessSink` world. | Rewritten to
`TerminalSnapshot` / `AgentMemory` / `TerminalState` / the emit type. |

`AgentIdentity` + `RestoreTarget` moved down to `anyagent/schemas` (the
resume-vocabulary layer), re-exported through `terminal-workspace` — the
fold *produces* the target, `resumeFormFor` *consumes* it, one owned
type. The serial lens → codex → simplify → code-police pass then
collapsed the vestigial `AgentSessionRef` into `AgentIdentity`, shared
one `resumableCommand` projection across the client surfaces,
value-gated the emit fences, and hardened the migration against corrupt
input.

### Second re-review — two relocations codex caught (the panel earned
its keep again)

A second 4-reviewer pass on the hardened HEAD credited the four
must-fixes **closed by construction** — but codex (1 of 4) found two
real defects relocated *inside* the new shapes, both verified against
the diff:

| # | Finding | Fix |
| --- | --- | --- |
| **F1** | `restoreTargetOf` paired `lastAgentCommand` with
`snapshot.agent` **without checking they agree on agent kind**, so a
stale-command/new-agent race could build `exact{ command:"opencode …",
agent:{kind:"claude-code"} }` — which `resumeAgentCommand` silently
downgrades to opencode's most-recent: the wrong-agent resume #2 makes
unspellable, re-expressed in the `exact` arm. | One constructor,
**`exactRestoreTarget(command, agent)`** (anyagent), builds `exact`
*only when* `agentKindFromCommand(command) === agent.kind`, else `null`.
**Both** production sites go through it — the fold (→ `none` on
mismatch) and the migration (→ `legacyMostRecent` on mismatch) — so
`resumeFormFor` always takes the same-agent path (exact-by-id, or
refuse), never the downgrade. |
| **F2** | The funnel `guardedEmit` swallowed a thrown emit **after**
the producer advanced its dedup baseline, so "emitted" ≠ "accepted": the
fold never got the value while the baseline advanced → a later equal
value deduped → silent permanent divergence. The guard sat at the wrong
seam. | Error handling moved to where the fallible work is — the host's
**publish boundary**.
`commitSnapshot`/`updateMemory`/`emitTerminalsDirty` (kolu) and pulam's
snapshots `upsert` each **fold-accept first, then guard their own
publish** (logged, self-heals on the next change). With the publishes
infallible, `emit` is infallible — and the wrong-seam funnel guard is
**removed**. |

### Type-naming cleanup

A follow-up **mechanical, compiler-checked rename** gives the awareness
types **one root per shape** — `TerminalSnapshot` (the whole value a
host emits) · `TerminalEvent` (a per-field delta) · `TerminalState` (the
`{ snapshot, memory }` composite kolu folds) · `Known<T>` (resolution
status). The `Terminal` prefix scopes the wire-facing family, and a
field repeats its type's root (`entry.observed` → `entry.snapshot`). The
served collection key bumps `awareness` → `snapshots`
(`TERMINAL_WORKSPACE_CONTRACT_VERSION` `2.0` → `3.0` — breaking, the
wire path changed). "Observation"/"awareness" survive only as
feature-word prose; the deleted `AwarenessSink`/`AwarenessValue` keep
their old names where the text describes what was removed. The
schema-module docstring, the [design
note](docs/atlas/src/content/atlas/awareness-derive-store.mdx) (types,
prose, and the foldflow/homes diagrams), and the unit-test fixtures all
move with the code.

### Honoring the design philosophy

- **Fail-fast / no knobs** — the write-fence is the emit type; the
restore decision is a discriminated value, not a runtime fallback. No
override path.
- **Volatility boundary** — producer/fold/`RestoreTarget` stay in the
`@kolu/*` packages; kolu and pulam are consumers, the dependency arrow
points out.
- **Reuse the source of truth** — `foldSnapshot` is shared by kolu's
fold and pulam's accumulator; `restoreTargetOf` / `exactRestoreTarget`
are the *one* place a restore target is decided; the migration composes
the existing backfill ladder.

### Deferred refinements (documented honestly, with pins)

- **Active drain-at-sleep (#3).** `beginSleep` freezes the last
fold-written target and does **not** drain a final settle, so a
sub-second launch-then-sleep freezes a stale `none` and wakes to a
**false bare shell**. The docs no longer claim the freeze "holds by
construction"; under model B the consequence is fail-*safe* (a bare
shell, never the wrong conversation). The active drain is an
async-`beginSleep` follow-up.
- **`git`/`pr` reference equality (#6).** `restoreRelevantEqual`
compares `git`/`pr` by reference (correct today — upstream
`gitInfoEqual`/`prResultEqual` dedup before emit). A `fold.test.ts` pin
asserts a structurally-equal-but-new-ref `git`/`pr` still compares equal
at the fence, so a future fold copy can't silently break it.
- **R9.3 scope** — the framed `TerminalFrame` *wire* stream and
cross-host keying stay R9.3; R9.0 keys by `TerminalId` and folds
in-process.

### Migration

`SCHEMA_VERSION` 1.28.0 → 1.29.0: `backfillSnapshotCutover` backfills
`pr: { kind: "absent" }` and **synthesizes the discriminated
`restoreTarget`** from what a pre-cutover record remembered — a valid
`agentSession` whose kind matches the command → `exact`, a command alone
(or a kind mismatch) → `legacyMostRecent`, neither → absent (`none`);
corrupt `agentSession` value-types fall through safely rather than
producing a parse-rejecting record.

### Tests & CI

The pure `fold` / `restoreTargetOf` / `exactRestoreTarget` /
`resumeFormFor` are unit-tested (the discriminated switch, the
kind-mismatch refusal, the ref-stability pin, the publish-boundary
infallibility). The sleep/wake, adopt, reconcile, metadata,
session-transfer, and restore-card tests track the reshaped persistence;
the e2e session-restore + sleeping-terminals fixtures were fixed to
stamp the now-required `pr`. All **unit tests** green across the touched
packages, `just check` (tsc + biome) clean, and the **Atlas sync gate
idempotent** — re-confirmed after the type-naming cleanup. **Full CI
green on both `x86_64-linux` and `aarch64-darwin`** (every recipe × both
platforms) on the naming commit, including `ci::e2e` on each.

_Generated by [`/be`](https://github.com/srid/agency) on Claude Code
(model `claude-opus-4-8`)._
srid added a commit that referenced this pull request Jul 7, 2026
… consumers

A runtime crash (surfaced by `nix run`, multi-host): `padiMap.entries` has TWO
whole-collection consumers — wire.ts's reconcile sub and HostSelectorStrip's chip
row. The police-batch fix (finding #7) added an `onError` to wire.ts's `.use()`
but left the strip's a bare `.use()`. The whole-collection dedup slot holds exactly
ONE handler and its registration is order-ASYMMETRIC: if the BARE consumer
registers FIRST (slot ← undefined), the HANDLER consumer registering SECOND throws
("whole-collection dedup slot already has an error handler") — and the gated strip
can mount before wire.ts's setup runs, so it did, crashing the app on load:

  Uncaught Error: whole-collection dedup slot already has an error handler —
  a second consumer needs per-consumer collection onError wiring (not yet built)

tsc + the unit suite were green because the throw fires only when BOTH consumers
mount in that order at runtime (the strip is gated behind multi-host, unexercised
by the mock-entry tests).

Fix: both consumers pass the SAME exported `onHostMembershipError` reference, so
whichever registers first bakes it and the second matches (the guard's documented
"identical handler shares fine" allowance) — order-independent, one toast for both.
Regression test pins the reverse-order trap (bare-first → handler throws) in
collectionDeltasGate.test.ts, the case the existing handler-first test missed.
srid added a commit that referenced this pull request Jul 7, 2026
…ackages

The remaining type-audit minors, batched disjoint-by-package (4 agents, no-git-ops
guard, one recovered cleanly from a mid-run API error). Combined green: tsc 49
projects; surface-map 22 / surface-remote 133 / padi 221 / server 186 / client 600.

surface-map+client: entries floored via the same floorOnLiveness as state() (the
  #1568 green-over-dead closed at the membership path); EntryConnectionState/MapRegistry
  parameterized over Prov (a local-only registry gets a compile error on copying,
  test-d pinned); Subscription.complete forwarded through delegateSubscription + wire +
  createPolledQuery (stays optional — non-factory sites have no typed-end to report);
  useIntentEditor collapsed to one clear-ability representation.
server: PadiSession made generic over Prov (local arm PadiSession-of-never can't be
  copying, test-d pinned — the split's last consumer); ProcessStartedAt seed uses the
  real serverStartedAt epoch, not the in-band 0.
padi: PadiIdentity union dropped the uninhabited undefined arm; HostLocation remote
  hostId min(1) + pin. (3 real-but-cross-package residuals documented for follow-up:
  PadiUrgency.awaiting, lastActivityAt-0 in terminal-workspace, SavedSession double-absence.)
surface-remote: getHandler returns the entry (membership by presence, not a colliding
  undefined handler); ResolveDrvError.failureCause tightened off network; ExitResult now
  a closed exit|signal|spawn-error union (honest signal, no code-null conflation);
  relayStream lead boxed; ClosedInfo gained transport-failed (ssh-255) + endpoint-down
  (no-process death) variants. ConnectionInfo flatten REFUTED (deliberate loose wire
  projection, safety inherited from the SessionState sum at source — a wire discriminated
  union would ripple to drishti for an unreachable, un-read contradiction).

drishti Phase-3 surface.md gate carries the API-facing surface-remote deltas (#2-#7).
Residual: the ClosedInfo endpoint-down PRODUCER (server padiBinding) migration is a small
paired follow-up. [never-defer]
srid added a commit that referenced this pull request Jul 14, 2026
…al exclusion

Code-police (rules + fact-check) — applied the clean findings:

- dry-rule-of-three: the byte-identical `store.set` "graph-owned (one writer)"
  throw lived in both `derived.cell` forms. Extracted `graphOwnedStore(get)` so the
  read-only facade + its one-writer guard has a single home.
- fact-check (test gap): the migrated fold gates on the COMPOSED record's
  `state === "active"` (was the raw registry's `entry.meta.state`), but no test
  built a non-active arm. Added a test proving a SLEEPING terminal whose agent reads
  `awaiting_user` is EXCLUDED — the behavioral-parity claim as an executable pin.

Skipped (recorded):
- invalid-states-unrepresentable (SiblingSource `engineTracked` boolean + dead
  `subscribe` → discriminated union): the lens debate already raised and DROPPED
  this by lowy+hickey consensus; not re-opened here.
- no-thin-wrapper-functions (`dropSnapshot`): kept as a documented named lifecycle
  seam, symmetric with `installSnapshot` (both forward one call) — the rule's
  "documentation-carrying named seam deliberately preferred" exception.
- fact-check (urgency fold recomposes the collection per poke): a real cost the
  reactive-bridge note anticipates (honest-cost #7 "measure before caring") and
  defers the fix (keyed reconciler) to SR8 — recorded in the PR body, not fixed here.
srid added a commit that referenced this pull request Jul 28, 2026
…cet vocabulary

`OsfactsReading` stood for both `Snapshot` and `HostSnapshot`, so eleven of its
eighteen fields were structurally always-empty at every call site and the
parser accepted either grammar in either output. It splits along the split Rust
already has.

- lowy F3 ≡ hickey #6 — one type for two verbs; now `SnapshotReading` /
  `HostReading` with `parseSnapshotOutput` / `parseHostOutput`, each refusing
  the other's row tags loudly instead of returning a silently empty field.
  `HostReading.memory` can finally be named `memory`, matching the JSON face
- lowy F4 ≡ hickey #7 — `mem` meant process RSS and host RAM in one flat union;
  the verb split takes the facet unions with it
- lowy F5 ≡ hickey #9 — the flag→wire-name map is not mechanical (`procs` names
  `proc`; `ports` names three), so every consumer re-derived tool knowledge by
  hand; `snapshotFacetNames` exports it. It states fact, not policy: which
  named facet counts as blindness stays with the consumer
- lowy F8 — `processIdentity` put the daemon supervisor's identity-gate policy
  inside the explicitly policy-free client, with zero consumers (verified by
  grep) and shaped for the OSF8 consumer this PR deliberately parked. Deleted,
  with the sync spawn that existed only to serve it
- lowy F1 ≡ hickey #1 — `facets.test.ts` pins the three unions to
  `osfacts/facets.json`, out of `describeDaemon` so the fast local loop catches
  the drift too
- lowy F10 — the zero-value reading was written out twice with nothing
  requiring the copies to agree
- lowy F12 — `isTcpPort` is no longer exported; it is the parser's own guard

Raised by the lowy ∥ hickey lens review. Not pushed or merged.
srid added a commit that referenced this pull request Aug 1, 2026
…own tag

- hickey #1 / lowy #7 — `readdir({withFileTypes:true})` has lstat semantics, so
  under pnpm most of `node_modules` rendered as clickable FILE leaves and a
  click answered EISDIR; stat the symlink entries so the row's shape matches
  what a click reads, and a broken link stays a leaf.
- lowy #5 — a `readdir` ENOENT was filed as `GIT_FAILED`, a tag meaning "a git
  subprocess failed" for a call that spawns none, and the documented typed
  `NOT_FOUND` survived only by regex on a twice-re-wrapped errno string. Add a
  `FILE_GONE` member, return it from `listDirectory` and `readFile`, map it in
  `unwrapGit`, and delete the now-redundant `servePadi` wrapper.
- hickey #12 — cross-reference `isDirectoryPath` as the consumer of the
  folder-key format `listDirectory` mints.

Raised by the lowy ∥ hickey lens review. Not pushed or merged.
srid added a commit that referenced this pull request Aug 1, 2026
- hickey #3 — `openLazyDirs` carried three facts and two broke. A row that has
  no node right now (a search projection hid it) no longer retires the record,
  so a filter keystroke stops erasing the user's expansion; and a key the host
  no longer declares lazy is pruned, so an eye-toggle round trip reports afresh
  instead of showing an arbitrarily old cached level with no refetch path.
- lowy #2 — a `lazyEpoch` prop clears the record when the host's loaded levels
  stop describing this tree (a repo / host switch), the wrapper's half of the
  invalidation the host already performs on its children cache.
- lowy #6 — `onExpandLazyDirectory` may return a promise; on rejection the
  wrapper forgets the expansion, so a transient read failure no longer wedges
  the folder open-and-empty for the mount.
- hickey #7 — the `tree.subscribe` callback runs under `safeApply`, so a throw
  can't escape into Pierre's emit loop and take other subscribers with it.
- hickey #9 — "which directories should be open" is spelled once
  (`desiredExpandedPaths`) and used at both the constructor and `toOpen`.
- hickey #8 — `expandPaths`' JSDoc no longer asserts an invariant
  `openLazyDirs` has made false.

Raised by the lowy ∥ hickey lens review. Not pushed or merged.
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