From 4ee94aa4deb08e2d7bd620447c93d067a79f141e Mon Sep 17 00:00:00 2001 From: Rickylabs Date: Wed, 12 Aug 2026 13:14:20 +0200 Subject: [PATCH 1/3] docs(harness): plan #1548 for separate PLAN-EVAL Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TKxrWGp5uxEHQ2NyZiZSMF --- .../slices/plan-1548.md | 110 ++++++++++++++++++ .../slices/research-1548.md | 97 +++++++++++++++ 2 files changed, 207 insertions(+) create mode 100644 .llm/runs/release-0.0.6-features--orchestration/slices/plan-1548.md create mode 100644 .llm/runs/release-0.0.6-features--orchestration/slices/research-1548.md diff --git a/.llm/runs/release-0.0.6-features--orchestration/slices/plan-1548.md b/.llm/runs/release-0.0.6-features--orchestration/slices/plan-1548.md new file mode 100644 index 0000000000..b0ce3520a8 --- /dev/null +++ b/.llm/runs/release-0.0.6-features--orchestration/slices/plan-1548.md @@ -0,0 +1,110 @@ +# Plan — #1548 browser stream resolver cannot see Aspire VITE service references + +Lane: 0.0.6 runtime reopen. Control PR #1555. Research: `slices/research-1548.md`. +Branch: `fix/1548-vite-browser-stream-discovery`. **Goes to PLAN-EVAL before implementation** via the +`openhands` + `status:plan-eval` label pair (automatic policy, D-4/D-5). + +## Contract + +`createNetScriptStreamDB({ streamPath, schema })` with **no `baseUrl`** must resolve +`VITE_services__streams__http__0` (or `VITE_STREAMS_URL`) in a Fresh/Vite browser bundle and connect, +instead of throwing "Durable streams URL not found…". The consumer-side +`baseUrl: import.meta.env.VITE_… ?? …` workaround must become unnecessary. + +## Root cause (verified from repo code, not from claims about Vite) + +Two independent reasons the value cannot be statically substituted — +`packages/plugin-streams-core/src/application/stream-url-resolver.ts`: + +1. `import.meta` is passed **as a value** into `readImportMetaEnvironment` (`:60`), which reaches + `meta.env` through a parameter binding (`:77-78`). The literal expression + `import.meta.env.VITE_services__streams__http__0` never appears in source. +2. Even given a bag, both keys are read by **computed index** (`:64,:68`). + +**The decisive corroboration is internal, which is why this does not rest on an assertion about +Vite.** This repo's own Vite plugin injects env by emitting `define` entries keyed exactly as +`` `import.meta.env.${mapping.target}` `` (`packages/fresh/src/application/vite/vite.ts:195,311-335`). +That is textual static-expression substitution by construction — it cannot reach a value-passed +`import.meta` or a computed index. The failure is fully explained by this repo's own machinery. + +Aggravating factor: the browser branch is wrapped in a silent +`try { … } catch { return undefined }` (`:59,69-71`), so the diagnostic surfaces as a generic +"not found" rather than anything pointing at env reading. + +## Locked decisions + +- **D1 — read the two browser keys as literal static member expressions**, inline in the module: + `import.meta.env.VITE_services__streams__http__0` and `import.meta.env.VITE_STREAMS_URL`. No value + passing of `import.meta`, no computed index for these two keys. This is safe because the key + strings are **fully known at compile time** — `STREAMS_RESOURCE_NAME` is a constant + (`packages/plugin-streams-core/src/domain/constants.ts:5`). The generic + `serviceName`/`protocol`/`index` parameterisation is what made the keys computed; for the browser + path it buys nothing and costs substitutability. +- **D2 — split the pure lookup from the `import.meta` access**, mirroring the SDK precedent + `packages/sdk/src/discovery/browser-env.ts:35-54,65`, whose pure + `getBrowserServiceUrlFromEnv(env, …)` is unit-tested at + `packages/sdk/tests/discovery/env-ordering_test.ts:24,41,56`. The pure function takes an injected + env bag; only a thin inline shim touches `import.meta.env.*`. +- **D3 — no published-surface growth.** The pure function stays **internal**: not added to + `packages/plugin-streams-core/mod.ts`, which exports only `buildStreamUrl`, `getStreamsAuth`, + `getStreamsUrl` (`mod.ts:19`). Tests import it by src path, as this repo's tests already do. +- **D4 — resolution precedence is unchanged**: `DURABLE_STREAMS_URL` → `services__streams__http__0` + → browser `VITE_*` (full key before shorthand) → throw. This slice changes **how** the browser + branch reads, not **what** wins. +- **D5 — no Vite `transform` hook, and no scaffold-template change in this slice.** The transform + option would couple the framework to a bundler's module graph; the `envMappings` route + (`vite.ts:76-83,183-199`) only helps once the read is substitutable, so it is a possible follow-up, + not part of the fix. Adding it here would widen a p1 into CLI-template territory. +- **D6 — the silent `catch` (`:59,69-71`) may be narrowed so a genuine throw is distinguishable from + "absent", but must not change which value wins.** Explicitly bounded so it does not become a + diagnostics refactor. + +## The testability trap this plan has to solve + +A unit test cannot easily prove "Vite substituted the expression" without running a real Vite build. +So the tests must attack the defect from both sides: + +1. **Precedence tests on the pure lookup (D2)** with an injected bag — full key wins over shorthand; + shorthand used when the full key is absent; neither present → `undefined`. Mirrors + `env-ordering_test.ts`. +2. **A source-shape guard** asserting the module contains the **literal** substitutable expressions + and that the browser path does **not** pass `import.meta` across a function boundary or index + `env` by a computed key. This is deliberately a shape assertion: the defect *is* the shape, and + without this guard a future refactor back to a helper reintroduces the bug **silently and with all + behavioural tests still green** — exactly the regression class #1405's reason tests were built to + catch. +3. **A resolution test through `getStreamsUrl()`** proving the browser branch is reachable and + ordered correctly. + +The plan does **not** claim a unit test proves Vite substitution. If PLAN-EVAL judges the source-shape +guard insufficient, the fallback is a built-fixture test, which is materially more expensive and +should be argued for explicitly rather than assumed. + +## Slices + +- **S1** — D1 + D2 + D3 + D4 with tests 1–3. +- **S2** — D6 only if it stays trivial; otherwise dropped and recorded. + +## Gates + +Scoped check/lint/fmt over `packages/plugin-streams-core`, `deno task quality:gate` (**verify it +covers this package — it demonstrably omits several, tracked as #1542; if not covered, run an +explicit target scan and say so**), `deno task --cwd packages/plugin-streams-core test`, +`deno task doc:lint`. No `e2e:cli` — this slice does not touch scaffold output. + +## Risks + +- **R1** — the fix is only provable end-to-end in a real Vite build; the source-shape guard is a + proxy. Stated openly above rather than papered over. +- **R2** — `getStreamsAuth()` is Deno-only (`:138-141`) and returns `{}` in a browser. Out of scope + here, but if the browser path needs auth this fix alone will not deliver a working connection. + **Worth PLAN-EVAL's attention:** it may mean the issue's "connect directly to Streams" acceptance is + not fully satisfiable by URL resolution alone. +- **R3** — the AppHost actually injecting both VITE variables was **not verified** by research. + +## Acceptance mapping (#1548) + +| Expectation | Satisfied by | Evidence | +| --- | --- | --- | +| Zero-config browser factory resolves Aspire's canonical Vite reference | D1 + D2 + D4 | precedence tests + source-shape guard + resolution test | +| Consumer `baseUrl` workaround becomes unnecessary | D1 | the documented override remains supported, no longer required | diff --git a/.llm/runs/release-0.0.6-features--orchestration/slices/research-1548.md b/.llm/runs/release-0.0.6-features--orchestration/slices/research-1548.md new file mode 100644 index 0000000000..a78001fe2c --- /dev/null +++ b/.llm/runs/release-0.0.6-features--orchestration/slices/research-1548.md @@ -0,0 +1,97 @@ +# Research — #1548 Fresh Streams browser resolver cannot see Aspire VITE service references + +Delegated read-only sub-agent (Claude · Opus 5, `drift.md` D-1), bounded brief. Finished **within +budget** — 15 tool calls, ~2 min — after the earlier #1398 pass had to be stopped at 72 calls. Every +claim carries a `path:line` citation; the unverified list is part of the finding. + +## The read path today + +`packages/plugin-streams-core/src/application/stream-url-resolver.ts`: + +- `getBrowserServiceEndpoint()` (`:54-72`) calls `readImportMetaEnvironment(import.meta)` at `:60` — + **`import.meta` is passed as a value argument**. +- `readImportMetaEnvironment` (`:74-79`) reaches `meta.env` through a **parameter binding**, guarded + by `'env' in meta` and `isEnvironmentRecord` (`:81-88`). +- Both keys are then read by **computed index** — `env[fullKey]`, `env[shortKey]` (`:64,:68`) — for + `VITE_services__streams__http__0` and `VITE_STREAMS_URL`. +- The whole browser branch is wrapped in `try { … } catch { return undefined }` (`:59,69-71`), so + **every failure is silent**. That is why the symptom is a generic "URL not found" rather than + anything pointing at env reading. + +The thrown message (`:128-132`) is a **verbatim match** with the issue text. + +**Precedence** in `getStreamsUrl()` (`:99-133`): `DURABLE_STREAMS_URL` (Deno-gated) → +`services__streams__http__0` (Deno-gated) → **browser `VITE_*` branch** → throw. There is no default +or `localhost` fallback. The browser branch is third and is the **only** branch reachable in a +browser bundle. + +## The reported cause: code shape confirmed, build semantics deliberately not asserted + +Two independent reasons the value is unreachable by static substitution — the issue names the first, +the research found the second: + +1. The literal expression `import.meta.env.VITE_services__streams__http__0` **never appears in + source**; `import.meta` crosses a function boundary as a value (`:60` → `:77-78`). +2. Even given a bag, the keys are read by **computed index** (`:64,:68`), not as static member + expressions. + +**Repo-internal corroboration, which is stronger than a claim about Vite:** this codebase's own +first-party Vite plugin injects env by emitting `define` entries keyed exactly as +`` `import.meta.env.${mapping.target}` `` (`packages/fresh/src/application/vite/vite.ts:195`, applied +at `:311-335`). That mechanism is textual/static-expression substitution **by construction** — it +cannot reach a value-passed `import.meta` nor a computed index. So the failure is explicable purely +from this repo's own substitution machinery, without asserting anything about Vite's internals. + +The sub-agent explicitly **did not** verify Vite's runtime behaviour for a bare `import.meta`, nor +how Fresh's Vite pipeline treats a JSR dependency module, and said so rather than asserting it. + +## A first-party Vite plugin already exists — this is not net-new infrastructure + +`createNetScriptVitePlugin()` (`packages/fresh/src/application/vite/vite.ts:282`, `enforce: 'pre'`, +`:307-309`) already carries a typed env-injection API: `NetScriptViteEnvMapping` (`:76-83`), +`envMappings`/`env` options (`:99-102`), and `buildDefineEntries()` (`:183-199`). It has tests +(`vite.test.ts`). + +But the scaffold template +(`packages/cli/src/kernel/assets/app/vite.config.ts.template`) wires the plugin **without** +`envMappings` or `env` — it passes only `appRoot`, `workspaceRoot`, `aliasEntries`, `watchPaths`, +`routeManifest`. The define machinery is present and currently unused by generated apps. + +## The SDK already solved this shape + +`packages/sdk/src/discovery/browser-env.ts` splits a **pure** `getBrowserServiceUrlFromEnv(env, …)` +(`:35-54`) from the `import.meta` access (`:65`), and that pure function **is unit-tested** — +`packages/sdk/tests/discovery/env-ordering_test.ts:24,41,56` cover browser-full-key precedence, +shorthand fallback, and server fallback with an **injected env bag**. + +That is exactly the asymmetry here: the injectable function is testable and tested; the +`import.meta`-reading function is neither. + +## Test coverage: none + +**There are no unit tests for `stream-url-resolver.ts`'s URL resolution at all.** The nine test files +in `packages/plugin-streams-core/tests/` never reference `getStreamsUrl`. The only repo-wide +references pass an explicit `baseUrl` and bypass resolution entirely +(`plugins/streams/services/src/sse-contract_conformance_test.ts:108`, +`packages/fresh/src/runtime/streams/create-stream-db_test.ts:22`). Nothing simulates a browser or +Vite environment. + +## Options, with published-surface impact (no recommendation made by research) + +| Option | Surface impact | Note | +| --- | --- | --- | +| Literal `import.meta.env.VITE_*` member expressions inline | **none** — `getBrowserServiceEndpoint` is not exported (`mod.ts:19` exports only `buildStreamUrl`, `getStreamsAuth`, `getStreamsUrl`) | key strings are fully known: `STREAMS_RESOURCE_NAME` is a compile-time constant (`constants.ts:5`) | +| Split reader from lookup, SDK-style | additive export **if** the pure fn is exported | makes the browser path unit-testable without a Vite build | +| Add `envMappings` to the scaffold template | changes generated output + CLI template, not the package API | only helps if the read pattern is one `define` can reach — i.e. must pair with the literal-read change | +| New `transform` hook rewriting the JSR module | none | most fragile; couples the framework to a bundler's module graph | +| Documented required `baseUrl` override | none | zero-code docs fix, but pushes discovery onto every consumer and contradicts the resolver's stated purpose (`:42-53`) | + +## Explicitly not verified + +- Vite's actual behaviour for a value-passed `import.meta`, for computed indexing of + `import.meta.env`, and for JSR dependency modules under Fresh's Vite pipeline. +- That the Aspire AppHost in fact injects both VITE variables into the dashboard process — the naming + helper exists (`packages/aspire/src/application/build-vite-env-var-name.ts`) but the injection path + was not traced. +- Any runtime reproduction; no gates, tests, or builds were run. +- Whether `vite.test.ts` covers `buildDefineEntries`. From 7d1c445519a14cb126298ad3ad6c9745cf452e23 Mon Sep 17 00:00:00 2001 From: Rickylabs Date: Wed, 12 Aug 2026 13:42:06 +0200 Subject: [PATCH 2/3] docs(harness): carry the amended plan and implementation brief onto the slice branch Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TKxrWGp5uxEHQ2NyZiZSMF --- .../slices/implement-1548.md | 108 ++++++++++++++++++ .../slices/plan-1548.md | 33 ++++++ 2 files changed, 141 insertions(+) create mode 100644 .llm/runs/release-0.0.6-features--orchestration/slices/implement-1548.md diff --git a/.llm/runs/release-0.0.6-features--orchestration/slices/implement-1548.md b/.llm/runs/release-0.0.6-features--orchestration/slices/implement-1548.md new file mode 100644 index 0000000000..ee3bbd3913 --- /dev/null +++ b/.llm/runs/release-0.0.6-features--orchestration/slices/implement-1548.md @@ -0,0 +1,108 @@ +use harness + +# Slice brief — #1548 browser stream resolver cannot see Aspire VITE service references + +**Codex · GPT-5.6 Sol · medium** (`normal_implementation`). The plan has **passed PLAN-EVAL with +conditions**; implement it, do not re-decide it. + +| Field | Value | +| --- | --- | +| Issue | #1548 (`priority:p1`) | +| PR | **#1559** (already open, draft, at plan phase — commit onto its branch) | +| Worktree | `/home/codex/repos/ns006-1548` | +| Branch | `fix/1548-vite-browser-stream-discovery` | +| Run dir | `.llm/runs/release-0.0.6-features--orchestration/` | + +**Read first:** `slices/plan-1548.md` — **including the "Amendments after PLAN-EVAL" section at the +bottom, which is binding** — and `slices/research-1548.md`. If they are not on your branch, read them +with `git show chore/release-0.0.6-runtime-reopen:`. + +## SKILL + +- `netscript-doctrine` — `packages/plugin-streams-core` is framework code; `mod.ts` is published + surface. +- `deno-fresh` — Fresh/Vite build and `import.meta.env` substitution. +- `netscript-tools`, `netscript-pr`, `netscript-harness`. + +## The defect + +`packages/plugin-streams-core/src/application/stream-url-resolver.ts` cannot see +`VITE_services__streams__http__0` in a browser bundle, for **two** independent reasons: + +1. `import.meta` is passed **as a value** into `readImportMetaEnvironment` (`:60`), reached through a + parameter binding (`:77-78`) — the literal `import.meta.env.VITE_…` never appears in source. +2. Both keys are read by **computed index** (`:64,:68`). + +This repo's own Vite plugin substitutes by emitting `define` entries keyed exactly as +`` `import.meta.env.${target}` `` (`packages/fresh/src/application/vite/vite.ts:195,311-335`) — a +textual static-expression mechanism that by construction cannot reach either shape. + +## LOCKED decisions + +- **D1** — read the two browser keys as **literal static member expressions** inline: + `import.meta.env.VITE_services__streams__http__0` and `import.meta.env.VITE_STREAMS_URL`. No value + passing of `import.meta`, no computed index for these two. Safe because the keys are fully known at + compile time (`STREAMS_RESOURCE_NAME`, `domain/constants.ts:5`). +- **D2** — split the **pure, injectable** lookup from the impure reader. **A1 (binding):** the SDK + (`packages/sdk/src/discovery/browser-env.ts`) is a **structural** precedent only — it carries the + *same* substitutability defect. Borrow its shape; do **not** describe it, in code comments or the + PR, as having already solved this. +- **D3** — **no published-surface growth**: the pure function stays internal, not added to `mod.ts` + (which exports only `buildStreamUrl`, `getStreamsAuth`, `getStreamsUrl`). Tests import by src path. +- **D4** — precedence unchanged: `DURABLE_STREAMS_URL` → `services__streams__http__0` → browser + `VITE_*` (full key before shorthand) → throw. Change **how** the browser branch reads, not what wins. +- **D5** — **no** Vite `transform` hook and **no** scaffold-template change in this slice. +- **D6** — you may narrow the silent `catch` (`:59,69-71`) so a genuine throw is distinguishable from + "absent", but it must not change which value wins. Drop it if it grows. +- **A3** — `plugin-streams-core/deno.json` depends only on `@netscript/telemetry` and `@std/assert`; + there is no dependency on `packages/fresh`, so no cycle is introduced and the + `createNetScriptStreamDB` call site needs no change. + +## Required tests + +1. **Precedence on the pure lookup**, injected bag: full key wins over shorthand; shorthand used when + the full key is absent; neither → `undefined`. Mirrors + `packages/sdk/tests/discovery/env-ordering_test.ts:24,41,56`. +2. **Source-shape guard** — assert the module contains the literal substitutable expressions and that + the browser path does **not** pass `import.meta` across a function boundary or index `env` by a + computed key. This is deliberately a shape assertion: **the defect is the shape**, and without this + guard a refactor back to a helper reintroduces the bug silently with all behavioural tests green. +3. **Resolution through `getStreamsUrl()`** proving the browser branch is reachable and correctly + ordered. + +There are currently **no unit tests at all** for this file's URL resolution — the package's nine test +files never reference `getStreamsUrl`. You are adding the first. + +**Do not claim any test proves Vite substitution.** It does not; test 2 is an explicit surrogate. +Say so in the PR. + +## Gates + +```bash +deno run --allow-read --allow-run .llm/tools/run-deno-check.ts --root packages/plugin-streams-core --ext ts,tsx +deno run --allow-read --allow-run .llm/tools/run-deno-lint.ts --root packages/plugin-streams-core --ext ts,tsx +deno run --allow-read --allow-run .llm/tools/run-deno-fmt.ts --root packages/plugin-streams-core --ext ts,tsx +deno task --cwd packages/plugin-streams-core test +deno task doc:lint --root packages/plugin-streams-core --pretty +``` + +**A2 (binding):** `arch:check` at `deno.json:156` **confirmed does not include** +`packages/plugin-streams-core`. So run `deno task quality:gate` **and** an **explicit target scan over +`packages/plugin-streams-core/src`**, and state in the PR that the package-quality verdict rests on +the explicit scan, not the repo gate. Do not report a green repo gate as proof for this package. + +Use `deno task --cwd test`, never a bare `deno test ` (omits `--allow-env`, exits 1 on +`NotCapable`). No `e2e:cli` — this slice does not touch scaffold output. + +## Commit trail + +PR **#1559 already exists** as a draft at plan phase. Commit onto its branch, push by explicit +refspec, and post a `[PHASE: IMPL]` comment with commit hash and pasted real gate output. Update the +PR body's Definition of Done to match what shipped. `Closes #1548` is already in its Scope. Move the +label from `status:plan-eval` to `status:impl` when you push. + +## Reporting contract + +Report what changed, the exact test names and what each catches, verbatim gate output, and anything +you could not verify. **Do not flip the PR to ready** — that fires the automatic IMPL-EVAL and is the +orchestrator's trigger to pull. Merge authority is the orchestrator's. diff --git a/.llm/runs/release-0.0.6-features--orchestration/slices/plan-1548.md b/.llm/runs/release-0.0.6-features--orchestration/slices/plan-1548.md index b0ce3520a8..7459c16f9b 100644 --- a/.llm/runs/release-0.0.6-features--orchestration/slices/plan-1548.md +++ b/.llm/runs/release-0.0.6-features--orchestration/slices/plan-1548.md @@ -108,3 +108,36 @@ explicit target scan and say so**), `deno task --cwd packages/plugin-streams-cor | --- | --- | --- | | Zero-config browser factory resolves Aspire's canonical Vite reference | D1 + D2 + D4 | precedence tests + source-shape guard + resolution test | | Consumer `baseUrl` workaround becomes unnecessary | D1 | the documented override remains supported, no longer required | + +--- + +## Amendments after PLAN-EVAL (verdict PASS with conditions) + +**A1 — the SDK is a *structural* precedent, not a *fix* precedent. Do not over-claim it.** +PLAN-EVAL verified that `packages/sdk/src/discovery/browser-env.ts:65` has the **same +Vite-substitutability defect** — it reads `import.meta.env` via a cast inside try/catch, so it is +*also* not a literal static member expression. What this plan borrows from the SDK is **only** the +separation of a pure, injectable lookup from the impure reader, and the fact that the pure half is +unit-tested. It must not be described anywhere as "the SDK already solved this". Both the research +note and D2 are corrected by this amendment. + +Consequence worth recording: **the SDK likely has the same latent browser-discovery bug.** That is +out of scope here and is not being fixed blind, but it should be filed once this fix proves the +shape works. + +**A2 — the explicit target scan is a hard gate step, not a conditional.** PLAN-EVAL confirmed +`deno.json:156` (`arch:check`) does **not** include `packages/plugin-streams-core`. So "verify +whether it is covered" is settled: it is **not**. The implementation slice's gate list must contain +an explicit target scan over `packages/plugin-streams-core/src`, and the PR must state that the +package-quality verdict rests on that scan rather than on the repo gate. Same caveat this lane +already carried on #1528 and #1536, now confirmed for this package too. + +**A3 — no circular-dependency or size risk.** PLAN-EVAL verified `plugin-streams-core/deno.json` +depends only on `@netscript/telemetry` and `@std/assert` — no dependency on `packages/fresh`, so +fixing this in `plugin-streams-core` introduces no cycle, and the `createNetScriptStreamDB` call site +in `packages/fresh` needs no change. It also verified the planned ~120-150 LOC test file sits well +under the 409-LOC ceiling of the largest existing test in that package. + +**Unchanged:** R1 stands as disclosed — no unit test proves Vite's `define` substitution fires in a +real build; the source-shape guard is an explicit surrogate, and PLAN-EVAL accepted it as such rather +than requiring a built fixture. From ccfa5407eb2c0291a67f650baffb1c3163b514e2 Mon Sep 17 00:00:00 2001 From: Rickylabs Date: Wed, 12 Aug 2026 13:50:53 +0200 Subject: [PATCH 3/3] fix(streams): make browser discovery statically substitutable --- .../context-pack.md | 10 +++ .../slices/worklog-1548.md | 65 ++++++++++++++++ .../stream-browser-environment.d.ts | 6 ++ .../src/application/stream-url-resolver.ts | 53 +++++-------- .../application/stream-url-resolver_test.ts | 78 +++++++++++++++++++ 5 files changed, 180 insertions(+), 32 deletions(-) create mode 100644 .llm/runs/release-0.0.6-features--orchestration/slices/worklog-1548.md create mode 100644 packages/plugin-streams-core/src/application/stream-browser-environment.d.ts create mode 100644 packages/plugin-streams-core/tests/application/stream-url-resolver_test.ts diff --git a/.llm/runs/release-0.0.6-features--orchestration/context-pack.md b/.llm/runs/release-0.0.6-features--orchestration/context-pack.md index 77cd40bec4..22208aa818 100644 --- a/.llm/runs/release-0.0.6-features--orchestration/context-pack.md +++ b/.llm/runs/release-0.0.6-features--orchestration/context-pack.md @@ -1,5 +1,15 @@ # Context pack — 0.0.6 runtime / public-surface lane +## Reopened slice — #1548 / PR #1559 (2026-08-12) + +The lane was reopened for the P1 browser stream-discovery fix on +`fix/1548-vite-browser-stream-discovery`. PLAN-EVAL passed with binding amendments recorded at the +bottom of `slices/plan-1548.md`. The implementation replaces the value-passed/computed browser env +reads with literal Vite-substitutable member expressions, adds a source-internal pure lookup, and +adds five resolver tests. Gate evidence and the contributor path are in `slices/worklog-1548.md`. +The PR remains draft at `status:impl`; separate-session IMPL-EVAL and merge authority remain with the +orchestrator. + Closing summary. The lane is **complete**: both owned issues landed on `main`. Read this first if resuming or auditing; everything below is traceable to a named artifact. diff --git a/.llm/runs/release-0.0.6-features--orchestration/slices/worklog-1548.md b/.llm/runs/release-0.0.6-features--orchestration/slices/worklog-1548.md new file mode 100644 index 0000000000..9807d01669 --- /dev/null +++ b/.llm/runs/release-0.0.6-features--orchestration/slices/worklog-1548.md @@ -0,0 +1,65 @@ +# Worklog — #1548 browser stream resolver + +## Design + +- **Public surface:** unchanged. `mod.ts` continues to export only `buildStreamUrl`, + `getStreamsAuth`, and `getStreamsUrl` from this resolver; the injectable browser lookup is + source-internal and imported by tests through its `src/` path. +- **Domain vocabulary:** the two finite browser keys are + `VITE_services__streams__http__0` and `VITE_STREAMS_URL`; their narrow `ImportMeta` declaration + lives in `stream-browser-environment.d.ts`. +- **Ports / seams:** `getBrowserStreamsUrlFromEnv()` is the pure injected environment-bag seam. + `getBrowserServiceEndpoint()` is the impure edge that contains the literal static member reads. +- **Constants:** `STREAMS_RESOURCE_NAME` remains the server-side discovery constant. The browser + member names are intentionally literal because their source shape is the substitution contract. +- **Commit slice:** one S1 implementation slice changes the reader shape and adds five tests. The + scoped static gates, package tests, doc lint, repo quality gate, and explicit package source scan + prove it. D6's catch narrowing was dropped because it was unnecessary to fix precedence or shape. +- **Deferred scope:** no Vite transform, scaffold template, SDK fix, real Vite build fixture, or + browser-auth change. +- **Contributor path:** add browser discovery behavior in `stream-url-resolver.ts`; add finite key + typing in `stream-browser-environment.d.ts`; preserve the source-shape guard in + `stream-url-resolver_test.ts`. + +## Implementation + +- Replaced value-passed `import.meta` and computed browser-key indexing with the two binding literal + member expressions. +- Split the pure injected-bag precedence lookup from the impure browser reader. +- Kept server/browser precedence and the public export map unchanged. +- Added a source-shape regression guard and a text-substituted module fixture that reaches the real + `getStreamsUrl()` path. The fixture is not claimed as a real Vite substitution test. + +## Tests added + +1. `browser streams URL lookup prefers the full Aspire key over shorthand` +2. `browser streams URL lookup falls back to the shorthand key` +3. `browser streams URL lookup returns undefined when both keys are absent` +4. `browser streams URL reader preserves Vite-substitutable static member expressions` +5. `getStreamsUrl reaches the browser reader and preserves browser key order` + +## Gate evidence + +All required commands exited 0 after the final source shape landed: + +| Gate | Result | +| --- | --- | +| scoped check | `filesSelected=45`, `failedBatches=0`, no findings | +| scoped lint | `filesSelected=45`, no findings | +| scoped format | `filesSelected=45`, `failedBatches=0`, no findings | +| package test task | `38 passed`, `0 failed` | +| doc lint | `totalErrors=0`, `totalMissingJSDoc=0` | +| `quality:gate` | exit 0; its configured roots do not include this package | +| explicit package source scan | `ok=true`, `findings=[]`, `allowCount=0` | + +The package-quality verdict rests on the explicit +`scan-code-quality.ts --root packages/plugin-streams-core/src` result, not the repo-level quality +gate, whose `arch:check` roots omit `packages/plugin-streams-core`. + +## Reconcile + +- PR #1559 still carries `Closes #1548`, remains draft, and already has exactly one lifecycle label: + `status:impl`. +- PLAN-EVAL PASS and its binding amendments are preserved. IMPL-EVAL remains for the orchestrator's + separate evaluator session; this implementation session did not flip the PR ready. +- No plan, doctrine, or debt drift was introduced. diff --git a/packages/plugin-streams-core/src/application/stream-browser-environment.d.ts b/packages/plugin-streams-core/src/application/stream-browser-environment.d.ts new file mode 100644 index 0000000000..0e359a2b60 --- /dev/null +++ b/packages/plugin-streams-core/src/application/stream-browser-environment.d.ts @@ -0,0 +1,6 @@ +interface ImportMeta { + readonly env: { + readonly VITE_services__streams__http__0?: string; + readonly VITE_STREAMS_URL?: string; + }; +} diff --git a/packages/plugin-streams-core/src/application/stream-url-resolver.ts b/packages/plugin-streams-core/src/application/stream-url-resolver.ts index 93f1d80d5c..a4bd199522 100644 --- a/packages/plugin-streams-core/src/application/stream-url-resolver.ts +++ b/packages/plugin-streams-core/src/application/stream-url-resolver.ts @@ -1,3 +1,5 @@ +/// + import { STREAMS_RESOURCE_NAME, STREAMS_URL_PREFIX } from '../domain/constants.ts'; interface EnvReadState { @@ -42,49 +44,36 @@ function getServerServiceEndpoint( /** * Read a service URL from the browser environment. * - * Mirrors the lookup performed by `@netscript/sdk/discovery` so that browser - * consumers (Fresh islands, the Vite-built playground) can resolve the streams - * URL without taking a framework-level dependency on the SDK. + * Browser consumers (Fresh islands, the Vite-built playground) can resolve the + * streams URL without taking a framework-level dependency on the SDK. * * Aspire's `WithConfiguredViteHttpReferences` injects two env vars per * service reference, both surfaced by Vite as `import.meta.env.VITE_*`: * 1. `VITE_services__{name}__{protocol}__{index}` — isomorphic full format. * 2. `VITE_{NORMALISED}_URL` — convenience shorthand. */ -function getBrowserServiceEndpoint( - serviceName: string, - protocol: 'http' | 'https' = 'http', - index = 0, -): string | undefined { +function getBrowserServiceEndpoint(): string | undefined { try { - const env = readImportMetaEnvironment(import.meta); - if (!env) return undefined; - - const fullKey = `VITE_services__${serviceName}__${protocol}__${index}`; - const fullUrl = env[fullKey]; - if (fullUrl) return fullUrl; - - const shortKey = `VITE_${serviceName.toUpperCase().replace(/-/g, '_')}_URL`; - return env[shortKey]; + return getBrowserStreamsUrlFromEnv({ + VITE_services__streams__http__0: import.meta.env.VITE_services__streams__http__0, + VITE_STREAMS_URL: import.meta.env.VITE_STREAMS_URL, + }); } catch { return undefined; } } -function readImportMetaEnvironment( - meta: ImportMeta, -): Readonly> | undefined { - if (!('env' in meta) || !isEnvironmentRecord(meta.env)) return undefined; - return meta.env; -} - -function isEnvironmentRecord( - value: unknown, -): value is Readonly> { - return typeof value === 'object' && value !== null && - Object.values(value).every((entry: unknown) => - entry === undefined || typeof entry === 'string' - ); +/** Resolve the browser streams URL from an injected environment bag. */ +export function getBrowserStreamsUrlFromEnv( + env: + | Readonly<{ + readonly VITE_services__streams__http__0?: string; + readonly VITE_STREAMS_URL?: string; + }> + | undefined, +): string | undefined { + if (!env) return undefined; + return env.VITE_services__streams__http__0 || env.VITE_STREAMS_URL; } /** @@ -111,7 +100,7 @@ export function getStreamsUrl(): string { return serverDiscovered; } - const browserDiscovered = getBrowserServiceEndpoint(STREAMS_RESOURCE_NAME, 'http'); + const browserDiscovered = getBrowserServiceEndpoint(); if (browserDiscovered) { return browserDiscovered; } diff --git a/packages/plugin-streams-core/tests/application/stream-url-resolver_test.ts b/packages/plugin-streams-core/tests/application/stream-url-resolver_test.ts new file mode 100644 index 0000000000..148600efd0 --- /dev/null +++ b/packages/plugin-streams-core/tests/application/stream-url-resolver_test.ts @@ -0,0 +1,78 @@ +import { assertEquals, assertNotMatch, assertStringIncludes } from '@std/assert'; +import { getBrowserStreamsUrlFromEnv } from '../../src/application/stream-url-resolver.ts'; + +const RESOLVER_URL = new URL( + '../../src/application/stream-url-resolver.ts', + import.meta.url, +); +const CONSTANTS_URL = new URL('../../src/domain/constants.ts', import.meta.url); + +Deno.test('browser streams URL lookup prefers the full Aspire key over shorthand', () => { + assertEquals( + getBrowserStreamsUrlFromEnv({ + VITE_services__streams__http__0: 'http://browser-full.example', + VITE_STREAMS_URL: 'http://browser-short.example', + }), + 'http://browser-full.example', + ); +}); + +Deno.test('browser streams URL lookup falls back to the shorthand key', () => { + assertEquals( + getBrowserStreamsUrlFromEnv({ + VITE_STREAMS_URL: 'http://browser-short.example', + }), + 'http://browser-short.example', + ); +}); + +Deno.test('browser streams URL lookup returns undefined when both keys are absent', () => { + assertEquals(getBrowserStreamsUrlFromEnv({}), undefined); +}); + +Deno.test('browser streams URL reader preserves Vite-substitutable static member expressions', async () => { + const source = await Deno.readTextFile(RESOLVER_URL); + + assertStringIncludes( + source, + 'import.meta.env.VITE_services__streams__http__0', + ); + assertStringIncludes(source, 'import.meta.env.VITE_STREAMS_URL'); + assertNotMatch(source, /\w+\(\s*import\.meta\s*\)/); + assertNotMatch(source, /\benv\s*\[/); +}); + +Deno.test('getStreamsUrl reaches the browser reader and preserves browser key order', async () => { + const keys = ['DURABLE_STREAMS_URL', 'services__streams__http__0'] as const; + const previous = new Map(keys.map((key) => [key, Deno.env.get(key)])); + const tempDir = await Deno.makeTempDir(); + + try { + for (const key of keys) Deno.env.delete(key); + + const source = (await Deno.readTextFile(RESOLVER_URL)) + .replace("'../domain/constants.ts'", JSON.stringify(CONSTANTS_URL.href)) + .replaceAll( + 'import.meta.env.VITE_services__streams__http__0', + JSON.stringify('http://browser-full.example'), + ) + .replaceAll( + 'import.meta.env.VITE_STREAMS_URL', + JSON.stringify('http://browser-short.example'), + ); + const fixtureUrl = new URL(`file://${tempDir}/stream-url-resolver.ts`); + await Deno.writeTextFile(fixtureUrl, source); + + const fixture: { getStreamsUrl(): string } = await import( + `${fixtureUrl.href}?test=${crypto.randomUUID()}` + ); + assertEquals(fixture.getStreamsUrl(), 'http://browser-full.example'); + } finally { + await Deno.remove(tempDir, { recursive: true }); + for (const key of keys) { + const value = previous.get(key); + if (value === undefined) Deno.env.delete(key); + else Deno.env.set(key, value); + } + } +});