Skip to content

feat(s5-5a): drain history before navigation + Vec-drain canonicalization#449

Merged
send merged 6 commits into
mainfrom
s5-5-nav-history-enforcement
Jul 5, 2026
Merged

feat(s5-5a): drain history before navigation + Vec-drain canonicalization#449
send merged 6 commits into
mainfrom
s5-5-nav-history-enforcement

Conversation

@send

@send send commented Jul 4, 2026

Copy link
Copy Markdown
Owner

S5-5a — drain history before navigation + Vec-drain canonicalization

First slice of the plan-reviewed S5-5 navigation/history enforcement cluster (child of the S5 boa→VM flip umbrella). The cluster plan-memo (docs/plans/2026-07-s5-5-navigation-history-enforcement.md) rides this slice.

What & why

process_pending_actions (threaded content mode + inline app mode) drained navigation before history and early-returned, so a same-turn pushState('/a'); location.href='/b' stranded the /a entry (the drain never reached history). Per WHATWG HTML §7.4.4 (Non-fragment synchronous "navigations"), the pushState URL/history update is synchronous — it ran during the script — so its NavigationController entry must commit before the async pipeline-replacing navigation supersedes.

  • Reorder both drains to window-opens → history (FIFO) → navigation (window-opens stay first, the S5-4c invariant).
  • Canonicalize the shell history drain to iterate take_pending_history() -> Vec; the boa runtime wrapper adapts Option -> Vec (mirroring take_pending_window_opens), leaving the deletion-bound boa bridge untouched — so the drain site is type-stable across the S5-6 flip (when the VM's Vec-returning drain replaces boa).
  • The inline-mode window-open drop moves first, closing a pre-existing leak.

Behavior scope: only the same-turn history+navigation interleaving changes; handle_history_action / handle_navigate bodies are unchanged. No same-document / fragment / popstate / state-threading (those are S5-5b / 5c).

Testing

5 new tests (content_history_drain_tests.rs), pin-verified — reverting to the old (navigation-first) order fails the two order-sensitive tests: pushState-before-nav commit, multi-pushState FIFO (at the handle_history_action seam, since boa's bridge is single-slot; the real 2-element Vec is VM-covered by elidex-js tests_engine_s1c), replaceState+nav ordering, pure-nav / pure-history regression pins. mise run ci green; the /elidex-review 5-agent + code-review pre-push gate returned 0 CRIT / 0 IMP (3 MIN, all comment-accuracy, fixed).

Slot

Closes #11-s5-shell-drain-history-before-navigation. The same-turn traversal+navigation race (a history.back() traversal rebuilds and supersedes a same-turn navigation — "one wins" either way) stays the bounded deferred #11-traversal-navigation-same-turn-race (memo §6-E7 / §8-D5).

🤖 Generated with Claude Code

…tion

Reorder process_pending_actions (threaded content mode + inline app mode) to drain own-context history (FIFO) BEFORE navigation. A same-turn synchronous pushState/replaceState — whose URL/history update already ran during the script (WHATWG HTML §7.4.4) — must commit its NavigationController entry before an async pipeline-replacing navigation supersedes it; the old navigation-first order early-returned and stranded the history mutation.

Canonicalize the shell history drain to iterate take_pending_history() -> Vec in FIFO order; the boa runtime wrapper adapts Option->Vec (mirroring take_pending_window_opens), leaving the deletion-bound boa bridge untouched, so the drain site is type-stable across the S5-6 flip. Window-opens still drain first (S5-4c invariant); the inline-mode window-open drop moves first, closing a pre-existing leak.

Closes defer slot #11-s5-shell-drain-history-before-navigation. First slice of the plan-reviewed S5-5 navigation/history enforcement cluster; the cluster plan-memo (docs/plans/2026-07-s5-5-navigation-history-enforcement.md) rides this slice.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@send

send commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2482a31c95

ℹ️ 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".

Comment thread crates/shell/elidex-shell/src/content/navigation.rs
Comment thread crates/shell/elidex-shell/src/content/navigation.rs
…build

The FIFO history-drain loop (threaded content + inline app modes) now breaks once a traversal (back/forward/go) rebuilds the pipeline: handle_history_action returns a rebuilt bool (true only when a Back/Forward/Go drove handle_navigate — false for pushState/replaceState and for a no-op out-of-range traversal), and the drain loop breaks on it. Remaining same-turn history intents captured from the now-superseded (navigated-away) document are no longer replayed onto the fresh page (e.g. a trailing pushState after history.back() no longer mutates the new page's URL/history). Two regression tests.

Codex R1: P2 "stop replaying old history intents after a traversal" fixed. (P2 "preserve navigation URL base at enqueue time" = boa-only — the VM resolves the nav URL at enqueue by construction, vm/host/location.rs:136, and boa is deletion-bound; P3 file-split findings target files not in this PR's diff.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@send

send commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

Codex R1 dispositions (head 665040f0)

P2 — "Stop replaying old history intents after a traversal" → ✅ Fixed (665040f0). The FIFO history-drain loop (both content/navigation.rs and the inline app/navigation.rs) now breaks once a traversal (back/forward/go) rebuilds the pipeline: handle_history_action returns a rebuilt bool (true only when a traversal actually drove handle_navigate; false for pushState/replaceState and for a no-op out-of-range traversal), and the loop breaks on it — so remaining same-turn intents captured from the now-superseded document are not replayed onto the fresh page. Tests: handle_history_action_reports_rebuild (return contract incl. Go(999) → false) + pushstate_after_rebuilding_back_is_not_replayed (end-to-end break).

P2 — "Preserve navigation URL base at enqueue time" → the VM (the S5-6 flip target) already resolves the navigation URL at enqueue, by construction: native_location_set_href (crates/script/elidex-js/src/vm/host/location.rs:136) runs resolve_url_or_syntax_error against current_url, and set_location stores the resulting absolute URL in NavigationRequest.url. So for the VM the shell's resolve_nav_url(pipeline.url, …) is a base-independent no-op — a same-turn pushState mutating pipeline.url cannot change the already-absolute target; the base is captured at assignment time. The raw-string path this finding describes is boa-only (crates/script/elidex-js-boa/src/globals/location.rs:117 stores the un-resolved href); boa is deletion-bound (D-26 PR7 = this same S5-6 flip), and per the project's boa-light-touch policy boa-only findings are resolved by the flip rather than patched in boa. VM correct now; the boa residual is eliminated at the flip. Not fixing in this PR.

P3 ×5 — "Split the oversized … module" (tests_observer_keepalive.rs, intersection.rs, gc/collect.rs) → not in this PR's diff. git diff --name-only origin/main..HEAD = {crates/shell/elidex-shell/src/{content/navigation.rs, app/navigation.rs, content/mod.rs, content_history_drain_tests.rs}, crates/script/elidex-js-boa/src/runtime/mod.rs, docs/plans/2026-07-s5-5-navigation-history-enforcement.md}. The three flagged files are pre-existing main content (landed via S5-3 #441 / #430 / #440) — this is a stale-base attribution, not a PR #449 change. Their 1000-line split is already tracked as a standalone follow-up task.

Resolving the corresponding threads.

@send

send commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 665040f0b5

ℹ️ 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".

Comment thread crates/shell/elidex-shell/src/content/navigation.rs
Comment thread crates/shell/elidex-shell/src/content/navigation.rs Outdated
Comment thread docs/plans/2026-07-s5-5-navigation-history-enforcement.md Outdated
…eue-resolve

Report a history rebuild only after a SUCCESSFUL traversal load: handle_navigate / handle_history_action / navigate_to_history_url now return bool (did the pipeline actually get replaced), so the FIFO history-drain loop breaks only on a genuine supersede. A failed-load traversal (Err leaves the pipeline intact, old document still active) no longer drops a trailing same-turn pushState. Both content + inline drivers.

Resolve boa's location navigation URL at enqueue against bridge.current_url (mirroring the VM), so 5a's history-before-navigation reorder cannot corrupt a relative-nav base via a same-turn pushState — NavigationRequest.url is now absolute for both engines (regression-prevention + VM-parity; WHATWG HTML §7.2.4 The Location interface). Normalize the plan-memo audit date (2026-07-05 -> 2026-07-04).

Self-root-check design re-gate (drain-loop touched R1+R2): ratified the loop converges cleanly + the boa fix is within light-touch; corrected the URL-base cite §7.4.2.2 -> §7.2.4 in the location/NavigationRequest docstrings, softened the both-engines-absolute claim to the success path, reconciled the plan-memo (§3.2 boa note, §0 pre-decision, §5.1 supersede contract).

Codex R2: 3 findings (2 P2 + 1 P3). Design re-gate: 1 IMP + 3 MIN.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@send

send commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

Codex R2 — all 3 fixed (+ a self-root-check design re-gate applied, since the history-drain-loop was touched across R1+R2; it ratified the loop is converging cleanly and the boa fix is policy-justified, and caught the §-cite below).

P2 — "Preserve the enqueue-time base for relative navigations" → ✅ Fixed. boa's location href/assign/replace now resolve the nav URL at enqueue against bridge.current_url() (globals/location.rs resolve_against_current, mirroring the VM's current_url.join), so NavigationRequest.url is absolute for both engines and the shell's resolve_nav_url is base-independent — a same-turn location.href='next'; pushState(…,'/dir/') no longer resolves next against the pushState-mutated pipeline.url. (Authority is WHATWG HTML §7.2.4 The Location interface — href setter step 2 / assign step 3 / replace step 2, "encoding-parse relative to the entry settings object".) Both engines now store an absolute URL on the success path.

P2 — "Report rebuild only after a successful traversal load" → ✅ Fixed. handle_navigate now returns bool = "did the pipeline get replaced" (Oktrue, Err [pipeline intact] → false); handle_history_action returns it for Back/Forward/Go, so the FIFO drain loop breaks only on a genuine rebuild. A failed-load traversal (network/load error → state.pipeline unchanged, old document still active) no longer breaks the loop and no longer drops a trailing same-turn pushState. Both content + inline drivers. Regression test failed_traversal_load_does_not_drop_trailing_history pins it (rigor-checked: temporarily flipping the Err branch to true fails all 3 R2 tests).

P3 — "Use a real audit date for spec verification" → ✅ Fixed. Normalized the plan-memo's spec-verification timestamps 2026-07-05 → 2026-07-04 (consistent with the commit + the S5-4d-merge reference + the UTC/git frame). Backstop dates unchanged.

Resolving the threads.

@send

send commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 99e516a3c6

ℹ️ 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".

Comment thread crates/shell/elidex-shell/src/content/navigation.rs Outdated
Comment thread crates/script/elidex-js-boa/src/globals/location.rs Outdated
…ueue-resolve (edifice)

Codex R3 (head 99e516a) surfaced 2 P2 threads; converged dispositions:

- thread1 (content/navigation.rs:435) traversal-cursor restore — REAL, shell-side,
  fixed in 5a. go_back/go_forward/go move the NavigationController index BEFORE
  handle_navigate; a FAILED-load traversal (which under R1/R2 does not supersede)
  left the cursor on the unreached target, so a trailing same-turn pushState
  committed from the wrong index and truncated the active entry. Fix = atomic
  traversal: capture current_index() before the eager move, restore_index() on a
  failed load. New NavigationController::current_index()/restore_index(); both
  drivers (app + content) + test failed_traversal_load_rolls_back_cursor.

- thread2 (elidex-js-boa location.rs:155) boa relative-nav base — REVERT the R2
  enqueue-resolution fix and DEFER the whole boa relative-nav base to the S5-6
  flip. Fully correcting boa needs TWO boa changes (resolve-at-enqueue AND
  pushState updating current_url) = an edifice on the deletion-bound engine that
  §0 pre-decision (2) + boa-light-touch + #396 self-root-check forbid. The VM is
  correct by construction (resolves at enqueue + pushState updates current_url
  synchronously); the boa-only divergence is deletion-bound + rare + within the
  defer envelope, and the flip erases it (no #11- slot). Keeping the R2 partial
  fix = a strangler half-fixed boa, so revert to R1's raw-store disposition.

KEEP from R2: the handle_navigate/handle_history_action/navigate_to_history_url
-> bool supersede contract + break-on-successful-rebuild drain, and the date fix.

mise run ci green (12673 tests). memo §0(2)/§3.2/§5.1.0/§5.3 reconciled.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@send

send commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

Codex R3 dispositions — head 541abeb0

Both R3 P2 threads addressed. mise run ci green (12673 tests).

thread content/navigation.rs:435 — restore the history cursor after failed traversals → FIXED.
Real, shell-side. go_back/go_forward/go move the NavigationController index before handle_navigate runs; under the R1/R2 supersede contract a failed-load traversal does not supersede, so the drain continues — but the cursor was left on the unreached target, and a trailing same-turn pushState then committed from the wrong index and truncated the active entry. Fixed with atomic traversal: capture current_index() before the eager move, restore_index() on a failed load (new NavigationController::current_index()/restore_index()), both drivers (app + content) + regression test failed_traversal_load_rolls_back_cursor (verified to fail when the rollback is neutralized).

thread elidex-js-boa/src/globals/location.rs:155 — keep Boa's URL base in sync → REVERTED + DEFERRED (not fixed in 5a).
The R2 enqueue-resolution fix is reverted and the whole boa relative-nav base is deferred to the S5-6 flip (D-26 PR7 boa deletion). Rationale: fully correcting boa needs two boa changes — resolve-at-enqueue and pushState updating current_url — which is an edifice on the deletion-bound engine that this repo's §0 pre-decision (2) (boa = light-touch, touched only to keep CI compiling) + the #396 self-root-check forbid. The VM is correct by construction (it resolves at enqueue and its pushState updates current_url synchronously); the boa-only divergence is deletion-bound, rare, and within the defer envelope, and the flip erases it (no tracking slot). Keeping the R2 partial fix would leave a strangler half-fixed boa, so 5a reverts to the raw-store disposition. Memo §3.2 records the deferral.

Note for the next round: the boa relative-nav base (either enqueue order) is a standing deferral to the flip — please treat a re-raise as already-dispositioned. The 5 file-split findings on tests_observer_keepalive.rs / intersection.rs / gc / collect.rs are out-of-diff (merged-main S5-3 content, not touched by this PR).

@send

send commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 541abeb0a8

ℹ️ 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".

Comment thread crates/shell/elidex-shell/src/content/navigation.rs
Comment thread crates/shell/elidex-shell/src/content/navigation.rs Outdated
Comment thread crates/shell/elidex-shell/src/content/navigation.rs Outdated
…-commit; defer task-queued model

Codex R4 surfaced 3 P2 on content/navigation.rs — the 4th consecutive round
on the history-drain-then-navigate mechanism, findings in mutual tension with
R2/R3. This fired the #396 self-root-check. An adversarial root-cause
re-derivation (+ webref §7.4 verification) confirmed the mechanism is an
edifice: a traversal is a QUEUED task in the spec (WHATWG HTML §7.4.3 traverse
the history by a delta appends to the traversable's session-history traversal
queue; §7.4.6.1 apply the history step runs within it; §7.4.4 URL and history
update steps run synchronously in the current task), but elidex drains all
same-turn intents in ONE synchronous pass, treating a traversal as a
synchronous pipeline-rebuild mid-FIFO-drain. R1/R2/R3 + #259/#283/#448 + the
carved E7 race are 7 clauses approximating that one missing task boundary.

Disposition = minimize 5a + defer the model (not collapse-in-5a: the
task-queued model is edge-dense, needs its own plan-review; not patch-the-3:
that is more edifice). Reachability (code-traced): boa's single-slot
back-channel makes the drain ≤1 action/turn, so #259 (multi-action FIFO drop)
is post-flip-only and #448 (reentrant restore clobber) needs the dead SW path;
#283 is the ONE live-reachable finding and is 5a's own regression.

- #283 FIXED: break → return true on a superseding traversal, so a successful
  traversal returns immediately and never falls through to
  take_pending_navigation() (which would drain a location.* the freshly-rebuilt
  page's initial scripts queued). Preserves E7 "traversal wins", restores
  pre-5a symmetry. Both drivers (content + app).
- Cursor: R3's eager-move + restore_index replaced by peek-then-commit
  (NavigationController::peek_back/peek_forward/peek_go + commit_index; the
  cursor moves iff the load succeeds — atomic by construction, no speculative
  move for #448 to clobber). current_index/restore_index retired.
  go_back/go_forward/go collapse to thin peek+commit wrappers (One-issue-one-way).
  commit_index debug_asserts the in-range peek-then-commit invariant
  (elidex-review Axis 2).
- #259 + #448 deferred to the reframed §8-D5 slot
  #11-session-history-task-queue-model (from #11-traversal-navigation-same-turn-race):
  implement the spec's task-queued traversal (apply the history step as a
  deferred post-drain content-thread task). One slot subsumes #259/#283/#448 +
  E7; lands in a plan-reviewed 5c/5d (edge-dense). §4.1 amended: the collapse
  correctly drops the multi-navigable serialization but must KEEP the
  single-navigable task boundary (its omission is the root).
- boa relative-nav base stays deferred to the S5-6 flip (unchanged, §3.2).

#283 success-path (successful traversal → fresh page's nav un-drained) is not
unit-producible in the disconnected harness (load always Errs); the complement
(failed traversal → drain continues → nav drains) IS pinned, and the
discriminating success assertion is registered as connected/flip integration
coverage (S5-4b precedent). 3 peek-then-commit unit tests added.

mise run ci green. /elidex-review 5-axis design re-gate 0 CRIT / 0 IMP
(2 borderline-FP MINs: commit_index assert → fixed; #283 success-path
coverage → registered integration deliverable).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@send

send commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

Codex R4 dispositions — structural (the #396 self-root-check fired)

R4's 3 P2 were all on content/navigation.rs — the 4th consecutive round on the history-drain-then-navigate mechanism, with the findings now in mutual tension (R2 made the break success-only → #259 says a success-break drops trailing FIFO; R3 added restore_index#448 says it clobbers reentrant state). Rather than patch a 5th clause, this triggered a root-cause re-derivation (+ webref §7.4 verification).

Root cause. A traversal is a queued task in the spec — WHATWG HTML §7.4.3 traverse the history by a delta appends to the traversable's session-history traversal queue, §7.4.6.1 apply the history step runs within it, and §7.4.4 URL and history update steps (pushState/replaceState) run synchronously in the current task. So history.back(); history.pushState() is "pushState commits synchronously this task; back() runs as a later task" — they phase-separate. elidex instead drains all same-turn intents in ONE synchronous pass, treating a traversal as a synchronous pipeline-rebuild mid-FIFO-drain. R1/R2/R3 + #259/#283/#448 + the carved E7 race are 7 clauses approximating that one missing task boundary.

Disposition = minimize 5a + defer the model. The faithful task-queued model is edge-dense (needs its own plan-review), so it does not belong in 5a's narrow base-case; patching the 3 findings on the synchronous-rebuild model would just accrete the edifice. Reachability (code-traced): boa's single-slot history back-channel makes the drain ≤1 action/turn, so only #283 is live-reachable pre-flip.

thread disposition
#283 navigation.rs:283 (fall-through drains fresh runtime) FIXEDbreakreturn true on a superseding traversal: it returns immediately and never falls through to take_pending_navigation(), so a successful traversal never drains the freshly-loaded page's queued nav. Preserves E7 "traversal wins"; restores pre-5a symmetry. Both drivers. This is 5a's own regression (the reorder created the fall-through).
#259 navigation.rs:259 (break drops trailing FIFO) DEFERRED — unreachable pre-flip (boa single-slot ⇒ ≤1 action/turn; [Back, PushState] never occurs). The real fix is the task boundary, not keep/remove-break (removing it just changes the post-flip failure mode). Re-evals at the reframed slot below.
#448 navigation.rs:448 (restore clobbers reentrant) RETIRED by construction — R3's eager-move + restore_index is replaced by peek-then-commit (peek_back/peek_forward/peek_go + commit_index; the cursor moves iff the load succeeds). No speculative move ⇒ no rollback path to clobber. The reentrancy concern (the dead SW path re-dispatching mid-load) re-lands at the SW-interception wiring (M4-10) via the reframed slot.

Reframed slot #11-traversal-navigation-same-turn-race#11-session-history-task-queue-model (memo §8-D5): implement the spec's task-queued traversal (apply the history step as a deferred post-drain content-thread task). One slot subsumes #259/#283/#448 + E7; lands in a plan-reviewed 5c/5d. §4.1 amended — the collapse onto the synchronous Vec+index correctly drops the multi-navigable serialization but must KEEP the single-navigable task boundary (its omission was the root).

mise run ci green; /elidex-review 5-axis design re-gate 0 CRIT / 0 IMP (§7.4.3/§7.4.4/§7.4.6.1 cites webref-verified). The boa relative-nav base stays deferred to the S5-6 flip (unchanged).

@send

send commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1933dedae6

ℹ️ 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".

Comment thread crates/shell/elidex-shell/src/content/navigation.rs Outdated
…toryCursorOp)

Codex R5: in a successful JS traversal, handle_navigate rebuilt the pipeline and
called notify_navigation (which ships ContentToBrowser::NavigationState —
can_go_back/can_go_forward from nav_controller) BEFORE the caller's commit_index
moved the cursor, so history.back() reported stale chrome state. This is a
regression R4's peek-then-commit introduced (R3's eager-move committed before
handle_navigate; peek-then-commit moved the commit after). A fresh nav already
push()es INSIDE handle_navigate before notify → the asymmetry was the bug.

Fix: thread the cursor op INTO handle_navigate so it commits before notify,
symmetric with the push.
- New enum HistoryCursorOp { Push, Commit(usize), Keep }; handle_navigate takes
  it instead of `is_history_nav: bool`. In the Ok branch, before notify:
  match { Push => push(url), Commit(i) => commit_index(i), Keep => {} }.
- Callers: fresh nav → Push; JS traversal (handle_history_action) → Commit(target)
  via peek, caller-side commit_index removed; chrome GoBack/GoForward (already
  eager-moved via go_*) → Keep; Reload → Keep. go_back/go_forward/go retained
  (chrome + tests). iframe-local handle_navigate is a different fn, untouched.
- App/inline path LEFT AS-IS (no bug): it sends no nav_controller-derived
  NavigationState IPC; the inline chrome reads can_go_back/forward LIVE at egui
  render time, after the synchronous caller-side commit_index — verified, so the
  stale-IPC vector cannot exist there.

Step 4.5 independent re-check (given R4→R5, the fix itself was re-screened) — 5/6
CLEAN + validated the app decision; 1 MIN: the "atomic by construction" claim
overstated — handle_navigate's SW-fetch synchronous message pump could stale the
held peek index before commit (same SW-reentrancy root as #448; unreachable today
— SW controller path dead; debug_assert-backstopped). Dispositioned doc-only:
scoped the navigation.rs claim to the non-reentrant path + folded the SW-pump
held-peek vector AND the pre-existing chrome-eager failed-load atomicity gap into
the deferred slot #11-session-history-task-queue-model (§8-D5). Both re-land at
the SW-interception / async-event-loop wiring (M4-10); the task-queued model
unifies all traversal cursor commits.

R5 success-path (post-commit NavigationState on a successful traversal) is not
unit-producible in the disconnected harness (same boundary as #283); the
failed-load complement + the enum success-gating are pinned; the success
assertion is registered as connected/flip integration coverage.

mise run ci green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@send

send commented Jul 5, 2026

Copy link
Copy Markdown
Owner Author

Codex R5 disposition — head will be the next commit

Real finding, and it was a regression R4's peek-then-commit introduced (thanks — this is exactly the kind of fix-introduced regression the loop is for). In a successful JS traversal, handle_navigate shipped NavigationState (can_go_back/can_go_forward from nav_controller) via notify_navigation before the caller's commit_index moved the cursor → stale chrome state. A fresh nav already push()es inside handle_navigate before notify; the JS-traversal asymmetry (commit deferred to the caller) was the bug.

Fix — commit the cursor INSIDE handle_navigate, before notify (symmetric with the push). New enum HistoryCursorOp { Push, Commit(usize), Keep } replaces is_history_nav: bool; the Ok branch does match cursor_op { Push => push, Commit(i) => commit_index(i), Keep => {} } before notify_navigation. JS traversal → Commit(target) via peek (caller-side commit_index removed); fresh → Push; chrome GoBack/GoForward (already eager-moved) → Keep; Reload → Keep. The app/inline path is left unchanged (verified no bug: it sends no nav_controller-derived NavigationState IPC; the inline chrome reads can_go_back/forward live at egui render time, after the synchronous caller-side commit).

Because R4's fix introduced this, I re-screened the R5 fix with an independent adversarial pass (Step 4.5). It came back clean except for one doc-accuracy MIN: the code's "atomic by construction" claim was overstated — handle_navigate's SW-fetch synchronous message pump could stale the held peek index before commit (the same SW-reentrancy root as #448; unreachable today — the SW controller path is dead — and debug_assert-backstopped). Dispositioned doc-only: scoped the claim to the non-reentrant path and folded the SW-pump held-peek vector and the pre-existing chrome-eager failed-load atomicity gap into the deferred slot #11-session-history-task-queue-model (§8-D5). Both re-land at the SW-interception / async-event-loop wiring (M4-10); the task-queued model unifies all traversal cursor commits at one seam.

mise run ci green. (The success-path assertion — post-commit NavigationState on a successful traversal — is not unit-producible in the disconnected harness, same boundary as #283; the failed-load complement + the enum success-gating are pinned, and the success case is registered as connected/flip integration coverage.)

@send

send commented Jul 5, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 8cbc88ad86

ℹ️ 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".

@send

send commented Jul 5, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8cbc88ad86

ℹ️ 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".

Comment thread crates/shell/elidex-shell/src/content/navigation.rs
@send

send commented Jul 5, 2026

Copy link
Copy Markdown
Owner Author

Codex R7 disposition — repeat of the deferred boa relative-nav base (re-examined + confirmed)

This is the boa relative-nav base already surfaced at R2 and deferred at R3 (memo §3.2). Because it re-raised and my own 5a reorder is implicated, I re-examined it rather than reflexively citing the prior defer — and the re-examination confirms the deferral, on a decisive point:

A single base-choice for the same-turn boa navigation cannot serve both orders.

  • Order A location.href='next'; pushState('/dir/') — correct is /next (the setter runs before pushState). Current 5a resolves next against the post-drain base /dir//dir/next (the bug you flag).
  • Order B pushState('/dir/'); location.href='next' — correct is /dir/next (the setter runs after pushState updated the base). Current 5a resolves against /dir//dir/next ✓ (right).

Any single-base fix — resolving next at enqueue, or snapshotting the pre-drain base — flips the outcome: Order A becomes right, Order B becomes wrong (/next instead of /dir/next). It doesn't fix the divergence, it moves it. Fully correcting boa needs two changes — resolve-at-enqueue (Order A) and pushState synchronously updating current_url (Order B) — which is precisely the edifice on the deletion-bound engine that this repo's §0 pre-decision (2) (boa light-touch) + the #396 self-root-check forbid. The VM is correct by construction (it does both), so the S5-6 flip (D-26 PR7 boa deletion) erases the divergence at the root.

Disposition: DEFERRED to the S5-6 flip (unchanged from R3; memo §3.2). Not fixed on the boa engine in 5a — a one-order fix would regress the other order (and Order B, pushState then relative-nav, is the more natural pattern current behavior already gets right). This is the standing deferral for the boa relative-nav base (both orders); the VM shell path is unaffected.

@send send merged commit 539a09b into main Jul 5, 2026
6 checks passed
@send send deleted the s5-5-nav-history-enforcement branch July 5, 2026 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant