Skip to content

fix: prefetch the navigation's Log again - #1491

Merged
xHeaven merged 2 commits into
dev-null-sh:productionfrom
xHeaven:fix/log-nav-prefetch
Nov 5, 2026
Merged

xHeaven merged 2 commits into
dev-null-sh:productionfrom
xHeaven:fix/log-nav-prefetch

Conversation

@xHeaven

@xHeaven xHeaven commented Nov 5, 2026 •

Copy link
Copy Markdown
Member

Summary

Hovering Log in the navigation stopped prefetching after #1480, so clicking it from another page loaded visibly instead of opening instantly. #1480 set prefetch={item.title !== 'Log'}, which turns prefetching off for the Log item on every page. It only needed to be off on a log page, where clicking Log scrolls to the top instead of visiting.

Log now prefetches again on every other page, on desktop and in the mobile bottom nav. On a log page it still scrolls to the top without a request.

Changes

  • app-nav.tsx: the navigation prefetches a link unless that link has a click handler. Only the Log link has one, and only on log/show, where it scrolls to the top.
  • LogController::show no longer saves the chosen log on a prefetch request (Purpose: prefetch). The navigation's Log normally points at the saved log, so prefetching it changes nothing. After a browser Back, though, a page can still carry an old auth.log for a moment. Hovering Log then would have saved that old log without a click.
  • .ai/guidelines/resources.js.blade.php: the prefetch rule now names the navigation's Log as an exception to "every link to log/{log} keeps prefetch={false}". Without it, the next change would turn prefetching off again. The Following/Latest/Trending tabs keep prefetch={false}.

Validation

Local, on an isolated pgvector/pgvector:pg17 database:

  • composer test:lint: passed. TMPDIR was set inside the worktree to avoid a stale shared Rector stub cache.
  • composer test:types: passed, with 0 PHPStan errors and passing tsc.
  • composer test:type-coverage: passed at 100%.
  • php -d memory_limit=-1 vendor/bin/pest --parallel --processes=4 --ci --coverage --exactly=100.0 --passthru-php="'-d' 'memory_limit=-1'": exit 0, 100.0% coverage.
  • New test LogControllerTest › "does not remember a log that is only prefetched": failed before the controller change and passes after it. LogControllerTest: 79/79 pass.
  • bun run test:lint, bun run test:types: passed. bun run test:unit: 291/291 pass. bunx vp test --run --config tests/draft-images.config.ts: 1,048/1,048 pass.
  • Chrome, 1440×900, seeded data, signed in:
    • On /log/following, hovering Log sends no request.
    • On /starred, hovering Log sends GET /log/following with Purpose: prefetch. Clicking then opens it from the prefetch cache, followed only by Inertia's usual partial reload of shared auth props.
  • iPhone 18 Pro simulator, iOS 27.0, Safari, signed in:
    • From Remotes, the bottom-nav Log opens the log.
    • Tapping Log while scrolled down on the log scrolls back to the composer with no new /log request.
    • The bottom nav stays clear of Safari's toolbar.

Hosted CI: pending at publication.

Independent review

GPT-6.1 Sol, high reasoning, priority service. Base 01c39a2a084d897226780df58de52e464529c19a.

  • Round 1 (head 8fa3e5da): FAIL.
    • A stale auth.log restored from browser history could make a hover prefetch save the wrong log. Fixed in c45fffeb, with a feature test.
    • The full PHP checks had not run. Now run, results above.
  • Round 2 (head c45fffeb): PASS, no actionable findings.

The reviewer independently ran 32 scroll and realtime tests. They did not rerun the broad suites, the browser checks or the simulator checks.

Limitations

  • After a browser Back, the Log link can briefly point at an outdated log. If you hover it and click within the 10-second prefetch cache, that log is shown but not saved as your default. A real visit still saves it.
  • Following/Latest/Trending tabs and remote conversations still don't prefetch. Opening them writes on the server, so they need separate work.
  • No physical device or Android testing.

Summary by CodeRabbit

  • Bug Fixes
    • Previewing the Log page no longer changes your saved log selection.
    • Log links can load their destination in advance without changing which log you’ll see when you open it.
    • When already viewing a Log page, selecting its link scrolls the page to the top.

@coderabbitai

coderabbitai Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Log links now prefetch when they have no click handler. The controller does not update a signed-in user’s remembered log during prefetch requests. A regression test verifies the saved log remains unchanged.

Changes: Log navigation prefetch

Layer / File(s) Summary
Prefetch and remembered-log behavior
.ai/guidelines/resources.js.blade.php, resources/js/components/app-nav.tsx, app/Http/Controllers/LogController.php, tests/Feature/Controllers/LogControllerTest.php
Log links prefetch unless their click handler disables prefetching. The controller skips remembered-log updates for prefetch requests. The regression test checks that the saved log remains unchanged.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related issues

Possibly related PRs

Suggested reviewers: nunomaduro, mnapoli, brendt

Merge Risk: Minimal · up to c45ff

No actionable merge-blocking risk remains identified.

Security architecture risk: Low · up to c45ff

The change prevents background navigation requests from changing an account preference without altering the authentication boundary. Remaining uncertainty concerns remembered-log consistency when navigation reuses a cached response, rather than demonstrated unauthorized access.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The changed persistence path targets the injected current user's remembered-log preference. Its downstream account-update event carries that same user's ID; the changed branch introduces no caller-selected account target.

Security Findings and Attack Paths

  • inferred — A caller-controlled prefetch marker suppresses a preference mutation; it does not select another identity, bypass the guest Following redirect, or add a data-access branch. No introduced security attack path was established in this changed flow.

Trust Boundaries and Controls

  • observed — Unauthenticated navigation uses plain anchors rather than the changed Inertia prefetch link. At the endpoint, authentication-derived user injection and the existing guest restriction remain separate from prefetch handling.

Resilience and Maintainability Implications

  • inferred — Repeated prefetches cannot invoke the guarded preference write or account-update event. Normal visits retain the pre-existing update-then-event ordering, so interrupted-write and event-failure recovery remain existing uncertainties rather than a newly introduced multi-step transition.
Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "fix: prefetch the navigation's Log again" matches the main change. The PR restores prefetching for the navigation's Log link (app-nav.tsx now disables prefetch only when an onClick handler …
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "fix: prefetch the navigation's Log again" matches the main change. The PR restores prefetching for the navigation's Log link (app-nav.tsx now disables prefetch only when an onClick handler exists). It also stops prefetch requests from saving the selected log in LogController::show, which is a supporting change. The title names the primary fix and is specific enough for a teammate scanning history.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 4 files.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5bb2423a-4d60-443e-977c-36ed2257690d
Commits

Reviewing files that changed from the base of the PR and between 01c39a2 and c45fffe.

Files selected for processing: 4

  • .ai/guidelines/resources.js.blade.php
  • app/Http/Controllers/LogController.php
  • resources/js/components/app-nav.tsx
  • tests/Feature/Controllers/LogControllerTest.php

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@xHeaven
xHeaven merged commit facb804 into dev-null-sh:production Nov 5, 2026
19 of 21 checks passed
@xHeaven
xHeaven deleted the fix/log-nav-prefetch branch November 5, 2026 17:47
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