From 028bc162fc0055be6dda1f7640cf3ac9182bf84b Mon Sep 17 00:00:00 2001 From: Rickylabs Date: Wed, 12 Aug 2026 13:14:17 +0200 Subject: [PATCH 1/6] docs(harness): plan #1459 for separate PLAN-EVAL Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TKxrWGp5uxEHQ2NyZiZSMF --- .../slices/plan-1459.md | 106 +++++++++++++++ .../slices/research-1459.md | 121 ++++++++++++++++++ 2 files changed, 227 insertions(+) create mode 100644 .llm/runs/release-0.0.6-features--orchestration/slices/plan-1459.md create mode 100644 .llm/runs/release-0.0.6-features--orchestration/slices/research-1459.md diff --git a/.llm/runs/release-0.0.6-features--orchestration/slices/plan-1459.md b/.llm/runs/release-0.0.6-features--orchestration/slices/plan-1459.md new file mode 100644 index 0000000000..e110988870 --- /dev/null +++ b/.llm/runs/release-0.0.6-features--orchestration/slices/plan-1459.md @@ -0,0 +1,106 @@ +# Plan — #1459 `DeferComponent` is not hydrated, so partial-miss regions never refresh + +Lane: 0.0.6 runtime reopen. Control PR #1555. Research: `slices/research-1459.md`. +Branch: `fix/1459-defer-island-hydration`. **Goes to PLAN-EVAL before implementation** via the +`openhands` + `status:plan-eval` label pair. + +## What the research changed + +The issue frames this as "the component is not hydrated". The actual finding is narrower and harder: +**`DeferIsland.tsx` is not an island at all** — there is no island registration for it anywhere +(`DeferIsland.tsx:111` is a *named* export; the scaffold calls `fresh()` with no options at +`vite.config.ts.template:41`; `createNetScriptVitePlugin` has zero island logic across all 438 lines). +Islands in this repo reach the client by **directory convention** or by being **copied into the +consumer app** (`fresh-ui`), and the one JSR-shipped island that works — `QueryIsland` — works only +because the *consumer's own* island file imports it (`query-island.tsx:26-36`). + +And a **second, independent defect** sits in the same component: `DeferIsland.tsx:222` sets +`f-client-nav={!(isPartialRequest && !hasCachedData)}`, whose condition is **exactly** `partial-miss`. +So in precisely the failing case, `f-client-nav` is `false` while `f-partial` is set (`:221`). +Hydration alone may therefore still produce a full document navigation instead of the `/partials/**` +request. These two are **technically inseparable**: fixing hydration without this leaves the issue's +observable symptom unfixed. + +## Scope decision — split, and move one criterion with a written reason + +The issue's acceptance asks for a regression test that *"build[s] the Fresh client bundle, navigate[s] +into a page with a cache-miss deferred layer, assert[s] the partial endpoint is requested, and +assert[s] the named boundary swaps exactly once."* + +This repo has **no browser driver** (Playwright appears only in docs and in the CLI *scaffolding* for +consumer projects) and **no gate that builds or inspects a client bundle**. That criterion therefore +needs **net-new test capability**, which is its own piece of work. + +**Decision: it moves to a follow-up issue with a written reason (#1090 pattern), and this PR does not +tick it.** A p1 runtime blocker — a documented pattern that paints a skeleton which never resolves — +should not wait on net-new browser-test infrastructure. Per the honesty rule, the criterion travels +with a reason rather than being ticked on weaker evidence. + +What this PR **does** prove is stated below and is deliberately not dressed up as the missing test. + +## Locked decisions + +- **D1 — the island must reach the client without depending on a consumer-owned file.** Preference + order, to be settled by the implementer against verified feasibility: + 1. register the island from `createNetScriptVitePlugin` (the only option requiring no consumer + file) — **feasibility is unverified**; the research could not confirm `@fresh/plugin-vite` + supports registering an island from a JSR specifier, and proved only that *this repo passes no + such option*. **Verify this first.** + 2. if (1) is not supported, the `fresh-ui` **copy-mode** precedent (`registry.generated.ts:334-340`). + A consumer-side shim that a consumer can delete — silently reintroducing the bug — is the **least** + preferred and must be argued for if chosen. +- **D2 — fix `f-client-nav` (`DeferIsland.tsx:222`).** Its current condition disables client nav in + exactly the `partial-miss` case it needs to be enabled for. **Verify Fresh's actual + `f-client-nav`/`f-partial` form-interception semantics before changing it** — the research flagged + this as inference, not fact. If Fresh does intercept `f-partial` regardless of `f-client-nav`, say + so and leave the line alone with evidence. +- **D3 — address the partial-swap case.** The coordinator form renders as a **sibling outside** the + `` (`DeferPage.tsx:257-275`), so a region swap leaves the form's DOM in place while a + page-level swap re-renders an inert form. The acceptance explicitly requires the coordinator to work + *after a page partial swap*; a fix that only works on first paint does not satisfy it. +- **D4 — no change to `decideDeferClientAction`'s policy logic** (`policy.ts:177-208`). It is correct + and already unit-covered; it is simply never executed. If the fix requires changing it, stop and + report. +- **D5 — out of scope:** the `streams`/`shouldStream` sibling path (`runtime/mod.tsx:178`), the dead + `debug` prop (`DeferIsland.tsx:54`), and #1457/#1548's surfaces. + +## Tests — what this PR can honestly prove + +1. **Island marker in server output.** Reuse the existing JSX-tree harness + (`define-page/tests/search-params.test.tsx:93-99,115-134`) to assert the rendered tree carries + whatever marker the chosen D1 mechanism produces. Fails if registration regresses. +2. **`f-client-nav` condition** (D2) asserted directly for the four combinations of + `isPartialRequest` × `hasCachedData`, pinning that `partial-miss` enables client nav. This is the + guard that would have caught the second defect. +3. **`buildDeferFormState` / decision plumbing** extended so `partial-miss → submit` is exercised + with the real inputs `DeferPage` passes (`DeferPage.tsx:267-268`), not synthetic ones. +4. If D1 lands via the Vite plugin, a unit test that the plugin **emits the island registration**. + +**None of these prove the client bundle contains the island or that a browser issues the partial +request.** That is the moved criterion. Say so plainly in the PR body; do not let tests 1–4 be read as +satisfying it. + +## Gates + +Scoped check/lint/fmt over `packages/fresh`, `deno task quality:gate` (**verify it covers +`packages/fresh`; it demonstrably omits several packages — #1542**), `deno task --cwd packages/fresh +test`, `deno task doc:lint`. If D1 changes the CLI registry or scaffold template, add the +`packages/cli` scoped wrappers and expect `scaffold-static` to exercise it in CI. + +## Risks + +- **R1** — D1's preferred option has **unverified feasibility**. If it fails, the fallback changes the + distribution model (copy-mode), which is a materially different PR. That branch point is why this + plan goes to PLAN-EVAL. +- **R2** — the `f-client-nav` claim is inference. Acting on it without verifying Fresh's semantics + risks changing a correct line. +- **R3** — copy-mode (D1.2) means framework fixes stop reaching already-generated apps; that is a + doctrine-relevant trade, not just an implementation detail. + +## Acceptance mapping (#1459) + +| Criterion | This PR | Evidence | +| --- | --- | --- | +| Coordinator registered/hydrated as a real Fresh island, including after a partial swap | **yes** (D1 + D3) | server-output marker test; plugin-emission test | +| A cache miss triggers the configured partial request and the region settles | **partially** — mechanism fixed (D2), not observed in a browser | `f-client-nav` condition tests | +| Regression test builds the client bundle, navigates, asserts the partial request and a single swap | **no — moved to a follow-up with a written reason** | needs net-new browser/bundle capability this repo lacks | diff --git a/.llm/runs/release-0.0.6-features--orchestration/slices/research-1459.md b/.llm/runs/release-0.0.6-features--orchestration/slices/research-1459.md new file mode 100644 index 0000000000..8379c20733 --- /dev/null +++ b/.llm/runs/release-0.0.6-features--orchestration/slices/research-1459.md @@ -0,0 +1,121 @@ +# Research — #1459 `DeferComponent` is not hydrated + +Delegated read-only sub-agent (Claude · Opus 5, `drift.md` D-1), bounded brief — 27 tool calls, +~3 min, within budget. Static analysis only; nothing built, run, or reproduced. + +## The reported defect is confirmed, and its cause is simpler than "hydration is broken" + +**`DeferIsland.tsx` is not an island at all.** There is no island registration for it anywhere in the +repo. + +- It is a **named** export, not a default export + (`packages/fresh/src/application/defer/DeferIsland.tsx:111`), re-exported as an ordinary module + symbol (`defer/mod.ts:12`, reachable as `@netscript/fresh/defer`). +- **The mechanism in this repo is directory convention**, resolved by the stock Fresh Vite plugin. + The scaffold calls `fresh()` with **no options** + (`packages/cli/src/kernel/assets/app/vite.config.ts.template:41`), and `createNetScriptVitePlugin` + contains **zero** island logic — no `islands` match across all 438 lines of + `packages/fresh/src/application/vite/vite.ts`. `define-fresh-app.ts:121-135` registers fs-routes + only and never passes a `loadIsland`. +- Every island that actually works in a scaffolded app lives in a **scanned directory**: the app + template's `(_islands)/` folders, and `@netscript/fresh-ui` islands, which are **copied into the + consumer app** (`packages/fresh-ui/registry/islands/`, registered as copy templates at + `registry.generated.ts:334-340`) rather than imported from the package. +- The contrast that proves the pattern: `QueryIsland` + (`packages/fresh/src/application/query/query-island.tsx:39`) ships from the same JSR package and + **does** reach the client — because its documented contract is that the **consumer's own** island + file imports it (`:26-36`), giving it a scanned entry point. `DeferComponent` has no such entry. + +So the component is reached **only** through a server-render path +(`DeferPage.tsx:8` ← `builders/define-page/runtime/mod.tsx:4`), and nothing in a scanned directory +transitively imports it. Both candidate causes hold and compound. + +## A second, independent defect inside the same issue + +`DeferIsland.tsx:222` sets: + +```tsx +f-client-nav={!(isPartialRequest && !hasCachedData)} +``` + +`isPartialRequest && !hasCachedData` **is exactly the `partial-miss` condition**. So in precisely the +case this issue is about, `f-client-nav` is `false` while `f-partial` is set (`:221`). If Fresh only +intercepts `f-partial` on forms under active client navigation, `requestSubmit()` would trigger a +**full document navigation**, not the `/partials/**` request the issue requires. + +**This is not verified** — Fresh core's form/partial interception was not read, so it is inference +from the attribute values. But it means **hydration alone may not fix the issue**, and any fix must +test this line rather than assume it. + +## Why no existing test catches this + +`decideDeferClientAction` (`policy.ts:177`, `partial-miss` branch at `:178-182`) has **exactly one +caller** — the client effect at `DeferIsland.tsx:188`. There is no server-side caller. In the shipped +product it is therefore **never executed at runtime on either side**; its unit coverage is the only +thing exercising it. + +The input plumbing is correct end-to-end (`runtime/mod.tsx:220` → `DeferPage.tsx:170,267-268`), which +is why this looks healthy from the server side. `DeferIsland.test.ts:1` imports only +`buildDeferFormState` and `sanitizeDeferSearchParams` and never renders the component. + +## Partial-swap behaviour + +From `DeferPage.tsx:257-275`: the coordinator form is rendered **as a sibling outside** the +`` — `
`. + +- A swap of the region's own partial replaces the content but **leaves the form's DOM node in + place**, so even a working island would not naturally re-run its effect (deps at `:201-214` are all + props). +- A **page-level** partial swap re-renders the whole `
` including a freshly server-rendered, + inert form — with no island marker, Fresh has nothing to hydrate into it. + +Nothing in the defer code addresses island persistence or re-hydration across `` swaps: no +`key`, no persistence attribute, no remount handling. The docs describe the transport and make no +persistence claim (`docs/site/web-layer/defer-streaming-ui.md:238-245`). + +## The regression test the issue asks for requires net-new capability + +The issue's acceptance says a test should *"build the Fresh client bundle, navigate into a page with +a cache-miss deferred layer, assert the partial endpoint is requested, and assert the named boundary +swaps exactly once."* + +What exists: + +- Pure-function unit tests (`DeferIsland.test.ts`) — never render the component. +- JSX-tree prop assertions (`define-page/tests/search-params.test.tsx:93-99,115-134`) — could assert + an island marker in **server** output, but cannot prove a client bundle. +- A live Fresh dev server over HTTP (`probe-project-boundary-dev.ts:43` with `FetchHttpAdapter`) — + the closest existing "drive a real app". + +What does **not** exist: + +- **No browser driver.** Playwright appears only in docs and in the CLI *scaffolding* Playwright + config for consumer projects (`init-agent.ts`) — not test infra this repo runs. +- **No gate builds or inspects a client bundle** anywhere in `packages/cli/e2e`. + +So the acceptance's regression test needs net-new capability at whichever level it targets. + +## Options (research made no recommendation) + +| Option | Trade-off | +| --- | --- | +| Consumer-side island shim from the scaffold (`routes/(_islands)/DeferIsland.tsx` re-exporting `DeferComponent`) | matches the `QueryIsland`/`fresh-ui` precedent, but every existing scaffolded app needs regeneration, and a consumer who deletes the file silently reintroduces the bug | +| Copy-mode distribution like `fresh-ui` (`registry.generated.ts`) | consistent with `ui:add`, but forks the island per project so framework fixes stop reaching existing apps | +| Declare the island from `createNetScriptVitePlugin` | the only option needing no consumer file — but **feasibility unconfirmed**, depends on a `@fresh/plugin-vite` capability not verified | +| Drop the island; resolve server-side on `ctx.isPartial && !component` | removes the hydration dependency and the `f-client-nav` hazard, but collapses the defer model — the policy branches (`policy.ts:184-208`) have no server equivalent | +| Inline `