Repository navigation
feat(local-edge): let the host own the debug-seed URL rewrite - #25
Merged
Merged
Conversation
When diagnostics change, the loader removes `__fwa_debug` from the current URL with a direct `history.replaceState`. An application whose navigation runtime is the only history writer rejects that write silently, so the seed stays in the URL and a reload flips diagnostics back. - Add `debug.setUrlWriter(writer | null)` and the matching queued command. With a writer registered, the loader hands it the cleaned path + search + hash instead of writing history; `null` restores the direct write, so existing consumers are unchanged. - The loader still never rewrites the URL at startup. - Document the URL ownership rule in the loader contract and cover the writer, the default path, a throwing writer and clearing it in tests.
Contributor
|
@codex review this |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 137987470d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Document the startup exception: `__fwa_debug=reset` is consumed during startup with a direct write, before any writer can be registered; 1 and 0 stay in the URL until diagnostics change. - Only `null` clears the writer; other non-function values are ignored, matching the queued command. - The writer is synchronous by contract; a returned rejected promise is absorbed instead of surfacing as an unhandled rejection. - Move queued-command dispatch into command-dispatch.ts (no behavior change) so it can be tested, and cover queued registration, invalid inputs, the reset-seed startup path and the async writer.
The loader sources are published as an explicit file list; the new module was missing, so a packed consumer failed to resolve it.
A host that queues debug.setUrlWriter before the loader runs expects the startup __fwa_debug=reset removal to go through it too. The loader now applies queued writers before debug.start(); the commands stay queued and replaying them is idempotent.
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.
Outcome
Applications whose navigation runtime is the only history writer can now own the URL rewrite that Local Edge diagnostics perform. The first consumer is ds-playground, where the FWA navigation backend replaces
history.pushState/replaceStateand admits only its own writes.Problem
When diagnostics change through
debug.setEnabledor the panel, the loader removes__fwa_debugwith a directhistory.replaceState. In ds-playground that write is rejected silently by the navigation backend's write admission, and the loader'stry/catchthen swallows the error. The seed therefore stays in the URL:?__fwa_debug=0remains after turning Developer mode on, and a reload turns diagnostics back off.Changes
debug.setUrlWriter(writer | null)on the publicFwaDebugApi, plus the queued command['debug.setUrlWriter', writer | null]for hosts that register before the loader runs. The loader applies queued writers before debug startup. Onlynullclears a writer; other non-function values are ignored.pathname + search + hashand does not write history itself.nullrestores the direct write.__fwa_debug=1and0are never rewritten at startup; see the demo case "shows opt-in diagnostics without changing app routing".__fwa_debug=resetis still consumed at startup, through a queued writer if one exists. Consumers that register no writer behave exactly as before.command-dispatch.ts, which is added to the published file list, so it can be tested.docs/loader-contract.mdgains a "URL ownership" section, an API table row, and the fulldebug.*command list.Verification
pnpm run cipasses atccf3652: 211 package tests, 39 demo e2e, and the document-shell packed consumer smoke test. The packed-consumer check caught a missingfilesentry during review, and it was fixed incd4ee4d.debug-runtime.test.tsandcommand-dispatch.test.tscover:Removing the writer branch fails the host-writer test.
Read-only review in a local session (gpt-5.6-terra / high), three rounds:
ccf3652and resolved.Limits
local-edge.spec.ts:673) needs a published0.1.0-beta.8and a consumer-side writer. That follow-up lives in ds-playground and is not verified here.Release: after merge, prepare
local-edge@0.1.0-beta.8through the Prepare release workflow (owner approved republishing).