test: migrate e2e from Playwright runner to Cucumber - #7
Merged
Conversation
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
marked this pull request as ready for review
March 20, 2026 14:21
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.
This was referenced May 27, 2026
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.
This was referenced Jun 11, 2026
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace
@playwright/testrunner with Cucumber.js + raw Playwright. Same Chromium browser, same test coverage (9 scenarios, 42 steps), lower runner overhead.Architecture
Key concepts
Gherkin (
.featurefiles)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.Andis 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
.featurefile is matched to a function by its string pattern:{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")function(), not arrow functions — arrow functions break Cucumber'sthisbinding.featurefileWorld (
support/world.ts)The World is the
thiscontext shared by all steps within a single scenario:setWorldConstructor(KoluWorld)tells Cucumber which class to useGiven I note the canvas dimensionssaves tothis.savedCanvas, thenThen the canvas should be smallerreads it)setDefaultTimeout(60_000)is Cucumber's per-step timeout, independent from Playwright's locator timeoutsHooks (
support/hooks.ts)Lifecycle in execution order:
Browser is expensive → created once. Each scenario gets a fresh context (like incognito window: clean cookies, storage, page). The module-level
let browserbridgesBeforeAll→Beforesince hooks can't return values.Why migrate?
.featurefiles can drive both API tests (direct HTTP) and UI tests (Playwright) with different step definition setsTest plan
just test-devjust pc(pre-commit) passes