fix(fresh): give managed form redirects an explicit navigation strategy

[rickylabs]

Summary

Adds an explicit managed-form navigation policy so redirecting forms can request a full document transition without leaking Fresh's raw f-client-nav transport attribute. The default remains inherited/client navigation, preserving managed validation-state revival.

Scope

• Archetype / area: Archetype 4 public DSL / packages/fresh
• Closes fix(fresh): make managed form redirects safe under inherited client navigation #1569

Slices

• S0 Harness research, mechanism verification, and plan — a7d470509
• S1 Public strategy, resolver, docs, SSR/state/browser coverage — fd20378b1
• S2 Wire the real-browser regression into required CI — 717cef36d

Mechanism evidence

• Plain Preact 10.29.2 omits a hyphenated JSX boolean false: <form ></form>; a spread string renders f-client-nav="false".
• Actual Fresh 2.3.3 is different: its server hook recognizes f-client-nav and stringifies Boolean(value), so both boolean and string false render as the literal "false" (@fresh/core@2.3.3/src/runtime/server/preact_hooks.ts:185-197).
• The reviver finds the closest ancestor/form carrying the attribute and enables client navigation unless getAttribute("f-client-nav") === "false" (@fresh/core@2.3.3/src/runtime/client/partials.ts:41-45).
• Therefore the existing boolean type is not a lie in the actual Fresh SSR runtime. The real defect is that consumers must know the transport attribute. The new policy maps { navigation: 'document' } directly to the literal string, making the outcome renderer-independent.

Public shape

• FormNavigationMode = 'client' | 'document'
• FormNavigationStrategy = { readonly navigation: FormNavigationMode }
• FormProps.strategy? and FormEnhancementOptions.strategy?
• FormCollectionStrategy shares the navigation vocabulary; legacy clientNav? remains as a deprecated compatibility input.
• EnhancedFormProps['f-client-nav'] widens additively from boolean to boolean | 'false'.

The resolver stays in the existing components/enhancement.tsx strategy seam. No builder, route, defer, AI, or internal package paths changed.

Red evidence

Focused pre-implementation product red:

Form document navigation strategy renders the literal Fresh opt-out ... FAILED (8ms)
AssertionError: Expected document strategy to render the literal Fresh opt-out in <form ... strategy="[object Object]">...
FAILED | 0 passed | 1 failed | 56 filtered out (2s)
error: Test failed

Controlled browser mutation (document temporarily resolved to client navigation):

browser: document form redirect beats inherited body client nav without runtime errors ... FAILED (30s)
AssertionError: Values are not equal.

-   true
+   false

FAILED | 0 passed | 1 failed (30s)
error: Test failed

The resolver was restored immediately and the same browser test passed.

Validation

• deno run --allow-read --allow-run .llm/tools/run-deno-check.ts --root packages/fresh --ext ts,tsx — exit 0

  {"selection":{"filesSelected":192,"batches":2,"failedBatches":0},"summary":{"totalOccurrences":0,"uniqueOccurrences":0,"uniqueCodes":0,"uniquePaths":0},"groups":[]}

• deno run --allow-read --allow-run .llm/tools/run-deno-lint.ts --root packages/fresh --ext ts,tsx — exit 0

  {"selection":{"filesSelected":192,"batches":1},"summary":{"totalOccurrences":0,"uniqueOccurrences":0,"uniqueCodes":0,"uniquePaths":0},"groups":[]}

• deno run --allow-read --allow-run .llm/tools/run-deno-fmt.ts --root packages/fresh --ext ts,tsx — exit 0

  {"command":"deno fmt --check","mode":"check","summary":{"filesSelected":192,"batches":1,"failedBatches":0,"findings":0,"ignoredFindings":0},"findings":[]}

• deno task --cwd packages/fresh test --reporter=dot — exit 0

  ok | 231 passed | 0 failed (31s)
  

• deno task --cwd packages/fresh test:browser — exit 0

  Task test:browser deno test --allow-all ./tests/form-navigation_browser.ts
  running 1 test from ./tests/form-navigation_browser.ts
  browser: document form redirect beats inherited body client nav without runtime errors ... ok (15s)
  
  ok | 1 passed | 0 failed (15s)
  

• Browser automation: Option 1. The required .github/workflows/ci.yml check-test job installs @playwright/cli@0.1.17 in runner temp, provisions that package's matching Chromium revision, and invokes the exact test:browser task above. The browser regression is no longer manual-only coverage. GitHub execution is deferred only because this lane must keep the PR draft.
• Workflow YAML parse — exit 0: workflow YAML parse: ok. actionlint is not installed in this environment.
• deno task quality:gate — exit 0. It reports known doctrine warnings in untouched repository paths.
• deno task quality:scan --root packages/fresh/src --pretty — exit 0; this explicit target is the package verdict:

  {"ok":true,"mode":"repository","scanned":["packages/fresh/src"],"findings":[],"allowCount":1}

• deno task --cwd packages/fresh publish:dry-run — exit 0

  Success Dry run complete
  

• deno task doc:lint --root packages/fresh --pretty — wrapper exit 0; the form entrypoint has 0 findings. The package baseline remains 44 pre-existing findings in route/query/streams, outside this slice.
• deno.lock — unchanged; no Deno workspace dependency was added.
• e2e:cli — intentionally not run per slice boundary.

Exact tests:

• Form document navigation strategy renders the literal Fresh opt-out
• Form default navigation strategy preserves inherited client navigation
• resolveFormNavigationProps maps typed client and document strategies
• applyCollectionStrategy accepts the shared document navigation policy
• browser: document form redirect beats inherited body client nav without runtime errors

Acceptance evidence — mirroring not applicable. Issue #1569 states its acceptance as plain
bullets, not markdown checkboxes, so it carries no close-gated boxes and there is nothing for
mirror-acceptance-evidence.ts to tick. The prior fenced block mapped box-index 1-5 against
boxes that do not exist and failed validation. Evidence for each stated criterion is recorded in
the [PHASE: IMPL] comments and in the fallback IMPL-EVAL verdict, which walks all six criteria
individually at head 717cef36d.

Harness

• Run dir: .llm/runs/fix-1569-form-redirect-nav-strategy--codex/
• Phase: impl
• Automatic label-driven evaluation is pending and orchestrator-owned. No local evaluator or OpenHands trigger was launched.
• PR remains draft with status:impl.

Drift / Debt

• Drift: the issue's boolean premise applies to plain Preact, but Fresh's SSR hook serializes boolean false correctly. Recorded in drift.md.
• Debt: none created.
• Baseline: 44 pre-existing doc-lint findings outside the form entrypoint.

Definition of Done

• Public typed navigation policy is exported and documented.
• Document navigation defeats inherited body client navigation in a real browser.
• Default navigation remains compatible and validation state revives without document reload.
• The browser regression is invoked automatically by the required CI check-test lane.
• Requested package, browser, quality, and published-surface gates are recorded truthfully.
• No prohibited path or lockfile changed.
• Resynced onto current main — merge to f2a78abf0, then 717cef36d.
• Separate-session IMPL-EVAL records PASS — authorized native Opus 5 fallback at the exact immutable head 717cef36d returned PASS with no blocking finding, after the cloud run was cancelled for producing no verdict beyond 20 minutes. It independently proved RED by reverting form.tsx (SSR strategy="[object Object]" and browser documentAttr === 'false' both failing) and verified the Fresh 2.3.3 reviver contract at partials.ts:41-44/:258. Advisories C1/C3 filed as fix(fresh): managed form silently drops navigation:'document' when mode:'client' #1609; C2 recorded for the 0.0.6 release notes.

[rickylabs]

[PHASE: IMPL]

Implemented and pushed the typed managed-form navigation strategy in fd20378b190632ccb672bf614f2528df14c35a89.

Outcome

• Added public client | document navigation vocabulary and resolved it through the existing enhancement strategy seam.
• document emits literal f-client-nav="false"; the default emits no local attribute and preserves inherited client navigation.
• Added real Fresh/Vite/Chromium coverage for an inherited <body f-client-nav>, invalid POST state revival without reload, and successful 303 document redirect with no runtime errors.
• Preserved the legacy clientNav collection input as deprecated compatibility surface.
• Updated managed-form documentation.

Red evidence

Form document navigation strategy renders the literal Fresh opt-out ... FAILED (8ms)
AssertionError: Expected document strategy to render the literal Fresh opt-out in <form ... strategy="[object Object]">...
FAILED | 0 passed | 1 failed | 56 filtered out (2s)
error: Test failed

Controlled browser mutation:

browser: document form redirect beats inherited body client nav without runtime errors ... FAILED (30s)
AssertionError: Values are not equal.
-   true
+   false
FAILED | 0 passed | 1 failed (30s)
error: Test failed

Gate output

check: 192 files, 2 batches, 0 failed batches, 0 findings
lint: 192 files, 1 batch, 0 findings
fmt: 192 files, 1 batch, 0 failed batches, 0 findings

Task test deno test --allow-all ./src ./tests '--reporter=dot'
ok | 231 passed | 0 failed (31s)

Task test:browser deno test --allow-all ./tests/form-navigation_browser.ts '--fail-fast'
browser: document form redirect beats inherited body client nav without runtime errors ... ok (26s)
ok | 1 passed | 0 failed (26s)

quality:gate: exit 0
quality:scan --root packages/fresh/src: {"ok":true,"scanned":["packages/fresh/src"],"findings":[],"allowCount":1}
publish:dry-run: Success Dry run complete
doc:lint: wrapper exit 0; form entrypoint 0 findings; package baseline 44 existing route/query/streams findings
deno.lock: unchanged

Handoff

Harness run: .llm/runs/fix-1569-form-redirect-nav-strategy--codex/.

The PR remains draft with status:impl. Automatic label-driven evaluation is pending; no local evaluator, Fable session, OpenHands trigger, ready-state transition, merge, or canary action was taken.

[rickylabs]

[PHASE: IMPL] — review finding follow-up

Closed the browser-test discovery gap with Option 1 in 717cef36d8e2eaea27234e61ab3ac809680cbfc0.

The required check-test job now installs @playwright/cli@0.1.17 in runner temp, adds its local binaries to GITHUB_PATH, provisions the package's matching Chromium revision, and invokes:

deno task --cwd packages/fresh test:browser

Verbatim output from that exact task:

Task test:browser deno test --allow-all ./tests/form-navigation_browser.ts
running 1 test from ./tests/form-navigation_browser.ts
browser: document form redirect beats inherited body client nav without runtime errors ... ok (15s)

ok | 1 passed | 0 failed (15s)

Workflow YAML parsing also passed:

workflow YAML parse: ok

actionlint was unavailable locally, so no actionlint claim is made. The GitHub job has not been forced while the PR is draft; it will execute through the existing required non-draft check-test lane.

deno.lock is unchanged. No Deno workspace dependency was added.

The PR remains draft at status:impl; no evaluator, OpenHands trigger, ready-state transition, merge, or canary action was initiated.

[rickylabs]

[PHASE: IMPL] slice review — finding closed

Resynced onto current main by merge (f2a78abf0), and the review finding I raised is fully resolved at 717cef36d.

The finding was that packages/fresh/tests/form-navigation_browser.ts does not match Deno's *_test.ts discovery pattern, and the new test:browser task was invoked by nothing — not .github/, not a root task, not .llm/tools/. The browser assertion passed when run by hand and would then never have run again, leaving the #1569 acceptance box "a browser test covers successful POST + redirect under an ancestor/body client-nav opt-in" satisfied by a test that could not catch the regression it was written for.

You took the preferred option rather than the disclosure escape hatch: the required CI check-test lane now provisions Chromium and invokes deno task --cwd packages/fresh test:browser. That makes the box mean what it says.

The implementation itself I reviewed and accept: client | document resolved through the existing enhancement strategy seam rather than a parallel prop, document emitting the literal f-client-nav="false" (the string, which is what the reviver actually reads — a boolean false was the whole defect), the default emitting no local attribute so inherited navigation is preserved, and the deprecated clientNav input kept for compatibility. Both RED proofs are genuine — the unit assertion failing on strategy="[object Object]" and the real Chromium mutation failing true vs false.

Added a Separate-session IMPL-EVAL records PASS box: the Definition-of-Done had every box checked and no evaluation box, so close-gate would have passed before any evaluator ran. Same correction I made on #1602.

deno.lock unchanged; the diff stays inside application/form/** plus the browser fixture and the CI lane. Flipping to ready — automatic IMPL-EVAL fires once at this head.

[rickylabs]

@openhands-agent model=openrouter/deepseek/deepseek-v4-flash-0731 output=pr-comment iterations=800 phase=impl head=717cef36d8e2eaea27234e61ab3ac809680cbfc0

Trusted base SHA: 6619603
Evaluated head SHA: 717cef3

use harness

SKILL

• netscript-harness — apply the formal IMPL-EVAL protocol and verdict vocabulary.
• openhands-handoff — publish one machine-readable OpenHands verdict.
• netscript-tools — run the smallest decisive repository-native gates without mutating source.
• netscript-doctrine — apply package/plugin doctrine when the changed surface requires it.

Act as the formal IMPL-EVAL session for this pull request. Do not edit files, create commits, push,
or repair findings. The trigger metadata supplies the trusted base SHA and immutable head SHA: read
the evaluator protocol, verdict definitions, and selected profiles from that base commit, then
evaluate the PR body, linked issues, run artifacts, final diff, review threads, and architecture
debt at the immutable head. Verify the approved plan or recorded PLAN-EVAL: N/A, design
checkpoint, acceptance criteria, static/runtime/consumer gates, public surface, lock hygiene, and
false-done states. For documentation changes, also read every changed document fully and hand-test
representative executable claims.

Return concise, severity-ranked findings with exact evidence and required action. End with exactly
one supported verdict line using OPENHANDS_VERDICT: PASS, OPENHANDS_VERDICT: FAIL_FIX,
OPENHANDS_VERDICT: FAIL_RESCOPE, OPENHANDS_VERDICT: FAIL_DEBT, or
OPENHANDS_VERDICT: FAIL_PLAN. Write the same verdict to OPENHANDS_SUMMARY_PATH.

[rickylabs]

OpenHands Agent — Did not run

OPENHANDS_VERDICT: NONE

Model: openrouter/deepseek/deepseek-v4-flash-0731
Provider: OPENROUTER
Job status: cancelled
Verdict source: none
Summary source: none
Housekeeping: ack=success trace=success commit-artifacts=skipped replies=skipped draft-pr=skipped
Trace: .llm/tmp/run/openhands/pr-1600/run-31623281943-1 (in the Actions artifact)
Artifact: openhands-agent-31623281943-1

OpenHands Agent Summary

The agent step did not run (outcome: cancelled).

This is a workflow failure, not a task verdict.

Diagnostics

• Bootstrap: success
• Agent: cancelled
• Model: openrouter/deepseek/deepseek-v4-flash-0731
• Provider: OPENROUTER
• Output mode: pr-comment
• Run: https://github.com/rickylabs/netscript/actions/runs/31623281943

See the uploaded Actions artifact for bootstrap.log and agent.log when present.

Run: https://github.com/rickylabs/netscript/actions/runs/31623281943

[rickylabs]

[PHASE: FALLBACK IMPL-EVAL] [VERDICT: PASS]

The cloud evaluator run produced no verdict beyond 20 minutes and was cancelled by the owner. Per that authorization this is one fresh native Opus 5 fallback — read-only, in a clean detached worktree at the immutable evaluated head 717cef36d8e2eaea27234e61ab3ac809680cbfc0. No labels were cycled, no second paid evaluator was launched, no Fable. This is the final comment for this phase.

At head 717cef36d the document strategy emits the literal f-client-nav="false" that Fresh 2.3.3's reviver actually checks, the default emits no attribute at all, and both the SSR and real-Chromium assertions were independently proven red by reverting form.tsx and green after restore. No blocking finding.

Verified

• The defect's root, at the framework source. Fresh 2.3.3's client reviver (jsr:@fresh/core@2.3.3/src/runtime/client/partials.ts:41-44) does el.closest('[f-client-nav]') then getAttribute(CLIENT_NAV_ATTR) !== "false", and the submit handler calls checkClientNavEnabled(el) on the form element itself (partials.ts:258). So a form carrying the literal "false" is its own closest match and short-circuits before any partial interception — which is exactly why a boolean was insufficient and why the string defeats an inherited <body f-client-nav>. enhancement.tsx:62-72 produces precisely that: navigation === 'document' ? 'false' : true.
• The emitted value is the string, not a boolean or an omission. form.test.tsx:80-105 asserts the rendered HTML contains f-client-nav="false" and that strategy= never reaches the DOM; :176-185 pins the resolver map (undefined → {}, client → true, document → 'false').
• The default is unchanged. resolveFormNavigationProps(undefined) returns {} spread last, so nothing is written; form.test.tsx:107-133 asserts no f-client-nav= at all, and the browser test asserts clientAttr === null under <body f-client-nav> (form-navigation_browser.ts:85).
• POST validation errors still revive without a document reload. The browser test sets a page-global sentinel, submits the default-strategy form, waits for the managed role="alert", and asserts the sentinel survived (:58-61,86) — a surviving JS global is proof no document navigation occurred. The fixture returns managed invalid state from a real app.post('/validate') (fixtures/form-navigation-browser/app.tsx:65-80).
• RED proved by execution, not taken from the PR body. Reverting only form.tsx to its pre-fix parent: form.test.tsx --filter navigation → FAILED | 2 passed | 1 failed with strategy="[object Object]" in the message, and test:browser → FAILED | 0 passed | 1 failed at :84 (the documentAttr === 'false' assertion). Restored; git status --porcelain empty and HEAD back at 717cef36d.
• The browser regression is genuinely wired and genuinely ran. deno.json:26 adds test:browser for the non-discoverable ./tests/form-navigation_browser.ts; .github/workflows/ci.yml:236-247 adds two steps to the required check-test job, and job 94209841064 (8m30s, pass) records success Install managed-form browser test runtime and success Managed form browser regression. The provisioning was checked to be valid rather than assumed: @playwright/cli@0.1.17 installs playwright, playwright-cli, and playwright-core into node_modules/.bin, so both the playwright install --with-deps chromium step and the test's Deno.Command('playwright-cli') resolve — and a missing binary would throw from Deno.Command, so the test cannot silently no-op.
• Gates run in the evaluation worktree: check → exit 0, 193 files, 0 occurrences; lint → exit 0, 0 occurrences; deno task --cwd packages/fresh test → ok | 236 passed | 0 failed; test:browser → ok | 1 passed | 0 failed (real Vite + Fresh + Chromium); deno doc --lint ./src/application/form/mod.ts → exit 0. Counts differ from the PR body's 192/231 because the branch merged main at f2a78abf0; the direction is additive.
• Boundaries and hygiene. No application/defer/** or application/builders/** path in the changed set; git diff for deno.lock and packages/fresh/deno.lock is empty; grep for isPartial across the diff returns nothing, so no cache read or seed is suppressed on partial requests.

Findings

C1 — FormCollectionStrategy silently drops navigation: 'document' when mode: 'client'. Advisory. enhancement.tsx:49-51 early-returns props unchanged whenever strategy.mode === 'client', before the navigation resolver runs. Since FormCollectionStrategy extends Partial<FormNavigationStrategy> (_internal/runtime-types.ts:88), the combination { mode: 'client', navigation: 'document' } type-checks, renders no attribute, and gives no diagnostic. Fix by resolving navigation ahead of the mode gate, or by narrowing the type so navigation cannot be expressed for client mode. Not blocking — the shipped intent-button path (mode: 'server' | 'hybrid') is covered by form.test.tsx:187-205.

C2 — a published type widened. Advisory. _internal/prop-types.ts:217-218 changes readonly 'f-client-nav'?: boolean to boolean | 'false', and EnhancedFormProps is publicly re-exported (form/mod.ts:71). Widening in a read position is source-breaking for a consumer assigning it to a boolean. Justified — the literal must be representable — and pre-1.0, but it belongs in the 0.0.6 release notes, not only in the PR body.

C3 — the strategy silently overrides caller-supplied raw props. Advisory. form.tsx:57-63 spreads resolveFormNavigationProps(strategy) after ...props, so a consumer still passing the old {...{ 'f-client-nav': 'true' }} alongside strategy={{ navigation: 'document' }} is overridden without warning. That matches the recorded precedence decision and is the safer direction, but form/README.md:77-94 does not state it. One sentence, alongside the migration note.

C4 — two red checks are not caused by this change. Orchestrator-owned. close-gate's sole reason is the unchecked Separate-session IMPL-EVAL records PASS box, which this verdict resolves. scaffold-runtime (aspire + docker + postgres) and deps-report are also red while scaffold-runtime-sqlite, scaffold-static, quality, code-quality, and check-test are green — and this PR touches no scaffold, plugin, or dependency path.

Acceptance check (#1569)

All six criteria satisfied with evidence: the typed strategy is declared and exported (runtime-types.ts:79-85, mod.ts:90-91, runtime/types.ts:37-38) and passes deno doc --lint; the document strategy is proven against the reviver contract and red without the fix; the default is proven unchanged in SSR, in the browser, and across 236 passing tests; validation errors revive without reload via the sentinel; the browser test asserts an empty runtimeErrors collected from pageerror and console-error events (:45-49,87) with the fixture's <body f-client-nav> wrapping both forms (app.tsx:47); and the README documents client vs document. The one gap is C3's precedence, which the README does not yet cover.

Next

• Orchestrator: tick the IMPL-EVAL Definition-of-Done box — it is close-gate's only recorded failure reason — and confirm scaffold-runtime / deps-report are red on current main before merging.
• Follow-up issue, not this PR: C1, by moving navigation resolution ahead of the mode === 'client' gate at enhancement.tsx:49-58.
• Release owner: record C2 in the 0.0.6 notes and add the C3 precedence sentence when documenting migration off the raw-attribute workaround.