Repository navigation
Conversation
…offline visit The precache manifest was scraped from .next/server/app/index.html alone, so a lazily imported view — Graph, Mind Map, AI Harness, TRIZ, Export, Sync — appeared in no HTML and its chunk was never cached. A user who installed the app, went offline, then opened a view they had never visited online got a rejected dynamic import and '<View> failed to load'. Measured on the pre-fix build: 17 of 36 emitted chunk files were precached. New e2e/offline-views.spec.ts reproduces all six views failing (the topbar renders an h1 with the view name, so assertions are scoped to main or two of them pass against the error fallback). The generator now unions the document's URLs with every .js/.mjs/.css under .next/static/chunks and fonts/images under .next/static/media, and fails closed when a directory or the JS inventory is missing. 61 URLs (21 document, 36 chunks, 17 media); WASM/model weights/source maps stay out (~4 MiB total). STATIC_CACHE moves to dks-static-v3 because a manifest-only change never reinstalls an existing worker. Fix proven load-bearing: dropping the chunk contribution returns the manifest to 17 chunks and the Graph case fails again. Plans/ADR: plans/161, ADR 041.
Next/eslint-config-next 16.3.6/16.3.5 -> 16.3.7 (bug-fix release), react/react-dom 19.2.8 -> 19.3.0, @types/react(-dom) -> 19.3.0. TypeScript stays on 6.0.3: typescript-eslint supports >=4.8.4 <6.1.0, so TypeScript 7 is a separate side-by-side tooling migration. Zustand unchanged at 5.0.15. Lockfile scope reviewed: six specifiers changed and only next, eslint-config-next, react, react-dom and their scheduler dependency re-resolved; every pnpm override, peer rule and ignored-build setting is retained. Verified on a production build: 2790 unit tests, 0 vitest type-check errors, lint/typecheck/build clean with zero warnings, 642 production E2E tests green, and the six-view offline regression green. The Home hydration mismatch seen under prefers-reduced-motion is pre-existing — 8 warnings on both the old and new versions (plans/161).
Source inspection contradicted several completion claims across plans/, so this reconciles the records against live source rather than extending any implementation. Add plans/162-roadmap-progress-and-next-work-2026-10-05.md: current implementation progress, missing implementation, new feature priorities, skills maintenance, documentation maintenance, execution order, and verification evidence. Statuses use a fixed vocabulary (Implemented -- source-confirmed / Partial / Not implemented -- source-confirmed / Recorded follow-on -- runtime not rechecked / Candidate -- not scheduled / Not checked in this audit) so an unchecked historical checkbox is never read as a live defect. Recorded as missing, first implementation priority: recovery reachability and backup outcome (restoreFromRecovery has no production caller; persistRecoverySnapshot returns void and skips oversized writes while importWithRollback still reports success). Also deletion/export integrity, sync join/rejoin, heavy-leaf deferral, and Plan 161's three recorded follow-ons. First new feature: AI request control. Correct stale records rather than restating them: - INDEX W2: withdraw "Closed, nothing to prune"; the ~26-dependency prune was an estimate, not a measured list - INDEX W3: six React.lazy boundaries replace "zero next/dynamic" - INDEX W4: bridge wiring done, join conflict surfacing and persistence re-init still open (ADR 027 is not complete) - INDEX W5: sanitizer helpers exist, production integration not demonstrated; source-error gating is restored - INDEX Plan 158: remediation complete (its own closure table already recorded P2-10/P2-11 Fixed) - Plan 04/11: blanket COMPLETE statuses become historical; gesture and snapshot-compare criteria unchecked; 11.5 renamed to the real JSON + self-contained reader path - ADR 037: Proposed -- not implemented (verified: no hash/popstate integration in src/ or e2e/) - GOAL/ARCHITECTURE: add semantic search, CPU-first in-browser local provider, okf; Node >=22 and ESLint 10; DOCX is a static import inside a lazy view; fail-closed hydration - GOAP: historical note -- the May ledger's Vite/SQLite/Orama/CLI instructions are not a current validation report Verification: quality_gate.sh --scope docs exit 0 (57 files, 211 links, 0 broken). Source probes re-confirm 6 lazy boundaries, ignoreSourceErrors: false, no restoreFromRecovery production caller, no hash/history integration, and verify-before-asserting absent from both generated catalogs. 48 introduced local links resolve, 0 unresolved. validate-links.sh covers SKILL.md only, so the separate introduced-link check is what covers plans/. No tests, builds, E2E, or remote queries were run; Plan 161's figures stay dated 2026-09-30 records.
Plan 162 named recovery reachability the first implementation priority from source inspection alone. The delivery lifecycle requires data-loss work to start at production, so this drives a real browser against a real corpus and records what actually happens. No implementation changes. Reproduced (Chromium, dev build, throwaway spec since removed): importing a >4 MiB single-entity corpus over the seed corpus replaces the library and reports "Imported 1 entity and 0 claims", emits no console warning, and leaves the *previous* corpus's snapshot in localStorage unchanged. No restore affordance exists anywhere in the Export view (RESTORE_UI_COUNT 0), for either a normal or an oversized import. Correction to my own earlier reading, now recorded in Plan 162: a first probe reported the snapshot absent because it read the key dks-recovery-snapshot, but the real key is do-knowledge-studio-recovery (recovery-helpers.ts:16). With the correct key an ordinary import does persist a ~19.7 KB snapshot. "Import never backs up" is therefore false. The reproduced defect is narrower and specific: a skipped backup is indistinguishable from a successful one, and a stale snapshot is left in place that would restore the wrong state if a restore UI existed. Still a missing reachability path plus a missing outcome report, not a data-loss incident. Nothing was lost that exporting first would not also prevent, and no restore was attempted because no UI offers one. The restart-and-restore acceptance leg remains unproven and is left to the implementation. Docs gate: quality_gate.sh --scope docs exit 0, 0 broken links.
Plan 162 recorded that a JSON import could replace the entire library with
no way back and no indication anything was missing:
- persistRecoverySnapshot returned void and silently skipped the write
above its 4 MiB guard, leaving the PREVIOUS import's snapshot in place.
- importWithRollback still returned { success: true }, so the UI showed a
clean green import either way.
- restoreFromRecovery had no production caller at all. The snapshot
existed and nothing could consume it.
Import now reports what actually happened and the backup is reachable.
Outcome reporting: persistRecoverySnapshot returns a discriminated
RecoveryPersistResult, threaded through a new ImportOutcome union to the
import toast, which warns "Imported -- but no backup was kept" instead of
claiming success. describeRecoverySnapshot exposes a count for the UI.
Reachability: new RecoveryBanner, mounted in RecoveryAlerts above the view
router so the offer is reachable from any view -- not just the Export view
the import happened in. Dismissal is session-only and never deletes the
snapshot, mirroring the quarantine banner's rule.
Two data-loss paths found while building this, both of which the new
banner would otherwise have made reachable, and both now guarded:
1. A refused write destroyed the surviving backup. Clearing in the
storage-unavailable branch was wrong: localStorage.removeItem still
succeeds when setItem is refused by a full quota, so it deleted the
only copy of the corpus being replaced. Only the too-large branch
clears now; a stale-but-real backup survives and is re-validated
against schema and TTL on every read.
2. Restore could consume the backup without saving. clearRecoverySnapshot
ran in a catch that also covered applying the snapshot, whose setState
persists and can throw on quota *after* the in-memory swap -- and it
was offered even in a hydration-refused session where every write is
dropped. Restore now refuses while isSyncBlocked(), clears only after a
confirmed apply, and never on a failure path.
readRecoverySnapshot is now contractually non-throwing and separates
unreadable storage (do not clear) from unusable bytes (clear), because
describeRecoverySnapshot runs from a shell effect where a throw would take
down the workspace over an unrelated leftover backup.
Evidence: 13 unit tests in recovery-backup-outcome.test.ts and 3 tests
across all four viewport projects in e2e/recovery-restore.spec.ts. Each
new test was driven against the unfixed code first: 6 fail without the
outcome fix, 4 without the restore-safety fix, and all 3 E2E cases without
the banner. Full gate on the final tree: 181 files / 2804 unit tests, 0
type errors, build clean, E2E 659 passed / 4 skipped / exit 0.
Deliberately unchanged: indexeddb-backup.ts still has no production
caller, so tiered backup remains not operational and no storage migration
is selected here. The graph revision-diff and remaining integrity gaps in
Plan 162 stay open.
… stops covering the topbar The fixed top-0 z-50 banner sat over the topbar's hit points at every configured viewport (measured with document.elementFromPoint: quick filter, command palette and New entity at 1280/1920; menu and search triggers too at 390x844) — while offline, the moment a local-first app most needs to work, its primary controls could not be clicked. OfflineIndicator now publishes its measured offsetHeight as --offline-banner-height (ResizeObserver keeps a wrapped or zoomed banner exact) and AppShell reserves that much paddingTop. Unset on the server, so both renders agree at 0px. The new Offline banner suite in e2e/accessibility.spec.ts pins the user-visible contract (controls are hit-testable) and was re-verified to fail on the old layout before passing here (plans/161). Co-Authored-By: Codebuff <noreply@codebuff.com>
The hook read matchMedia synchronously on the client's first render while
the server answered false, so ~25 call sites branching
initial={reducedMotion ? false : {...}} hydrated different styles than the
HTML they were sent — 8 React mismatch warnings per Home load under
prefers-reduced-motion, on every framework version measured (plans/161 A/B:
8 on Next 16.3.6/React 19.2.8, 8 on 16.3.7/19.3.0).
useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot = () => false)
pins the server answer for the hydration pass and re-reads the real
preference immediately after. Module-scope callbacks keep identity stable so
the subscription survives renders. New e2e/hydration-mismatch.spec.ts pins
the contract in a real browser (the failure is invisible to screenshots and
mocked unit tests) and was confirmed red on the old hook before passing on
all four projects; touch-targets.spec.ts went from 8 logged mismatches to 0.
Co-Authored-By: Codebuff <noreply@codebuff.com>
…r; fix pre-existing workflow lint nits The precache manifest and the worker's cache identity are build artefacts, so e2e/offline-views.spec.ts only means something against `pnpm run start`. The detect-changes job gains an `offline` filter (worker, manifest generator, boot wiring, the offline spec, playwright config, package manifests) and the e2e-tests job runs `pnpm run build && pnpm run test:e2e:offline` after the dev-server suite when it trips — instead of on every UI edit (plans/161, ADR 041). Nightly and manual runs force the filter on like the other outputs. Also clears the two pre-existing CI-lint nits recorded in plans/161: security-scan.yml's yamllint comments-indentation warning (comment block misaligned with the following list item) and ci-and-labels.yml's actionlint SC2002 (useless cat into jq). Both files now pass yamllint and actionlint with zero warnings. The precache manifest is regenerated from the current dependency tree so the committed artefact matches what the offline suite builds. Co-Authored-By: Codebuff <noreply@codebuff.com>
…gate in Plan 161 / ADR 041 Plan 161's follow-on section now documents the reduced-motion hydration mismatch as fixed (mechanism, useSyncExternalStore fix, A/B evidence, the deliberate first-mount residual) and the offline-banner obstruction as fixed (with the measured obstruction table and the two false-pass traps the test had to close before it could be trusted). ADR 041 records that the production offline suite is wired into CI behind the `offline` paths filter. Co-Authored-By: Codebuff <noreply@codebuff.com>
…cy tree 27 chunk hashes moved after the rebase onto main (next 16.3.8, react 19.3.0, react-day-picker 10.0.2). The manifest is a build artefact of that exact tree; regenerating it here keeps the committed list identical to what the production offline suite builds. Verified: pnpm run build writes 61 URLs and the full e2e/offline-views.spec.ts suite passes 6/6 against it. Co-Authored-By: Codebuff <noreply@codebuff.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
documentation
Contributor
|
Blocked merge diagnosis — blocked |
Contributor
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 3 high |
🟢 Metrics 49 complexity · 0 duplication
Metric Results Complexity 49 Duplication 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![nexus-check[bot]](https://avatars.githubusercontent.com/in/4300564?s=60&v=4){kind=small}
Contributor
GitNexus Review · PR #9253 issues found across 2 files. (reviewed 18 of 19 reviewable files) SummaryThis appears to be a broad product and maintenance change spanning offline behavior, recovery, accessibility, dependencies, tests, and roadmap documentation. Its risk posture is elevated by wide graph reach and high-risk CI workflow files. 🔴 CRITICAL blast radius. A cross-cutting PWA, recovery, accessibility, and dependency change across the studio and project tooling, with 49 graph dependents. The implementation centers on Review the recovery helpers, service worker and precache generation path, and the offline and restore tests first. The HIGH risk files are 🔀 Structural changes ·
|
Contributor
🤖 Agent context for GitNexus Review · PR #925This comment carries deterministic graph detail for coding agents and reviewers who want the receipts — the main review comment carries the human summary.
What changedSymbol Changes (58)
Changed Files (38)
What it affectsArchitecture Impact
Blast Radius
Direct dependents (d1)
Indirect dependents (d2)
What to checkFile Risk (9)
Prompt for AI agents (3 issues) |
recovery-helpers: containment + honesty fixes — - notifyAvailability now contains each subscriber in its own try/catch (console.error) and runs outside the write try, so a listener bug can no longer be misreported as a storage failure or abort the import caller - the size guard measures UTF-8 bytes via TextEncoder, not UTF-16 code units; Unicode-heavy content could previously pass a 4 MiB code-unit check and then fail in storage without the protective clear - JSON.stringify failure returns its own 'unserializable' reason instead of 'too-large'; the import toast gains matching copy so no user is told their library was too big when nothing was measured - clearRecoverySnapshot notifies only after a successful removal offline-indicator: the reserved height is now released by AnimatePresence's onExitComplete instead of the effect cleanup, so returning online no longer slides the topbar back under the still-animating banner; pinned by a new real-browser e2e that asserts the padding clears after the exit. tests: the restore-failure test now calls through before throwing so the store is genuinely swapped when the write is refused (matching the commented failure mode); refuseRecoveryWrite steps aside for non-recovery keys instead of silently swallowing Zustand's persistence writes (jsdom's proxy re-dispatches at call time, so a captured "original" recurses — the mock restores, calls through, and reinstalls). playwright.config: the production-offline comment no longer claims mobile coverage the chromium-only run does not provide. Co-Authored-By: Codebuff <noreply@codebuff.com>
Contributor
🔄 What's new in this push (
|
| // an oversized snapshot past a code-unit check (GitNexus on PR #925). | ||
| if (new TextEncoder().encode(serialized).length > MAX_RECOVERY_SIZE_BYTES) { | ||
| console.warn('Recovery snapshot exceeds size limit, skipping persistence') | ||
| clearRecoverySnapshot() |
Contributor
There was a problem hiding this comment.
🟡 Warning — Oversized imports can leave a stale recovery snapshot when removal fails
The oversized path relies on clearRecoverySnapshot() to prevent a later restore from returning the corpus replaced by a prior import. But clearRecoverySnapshot catches a failed localStorage.removeItem and returns without indicating failure (recovery-helpers.ts:196-203); this call then reports too-large and the import proceeds. If removal is refused, the old snapshot remains available and can later restore the wrong corpus.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/studio/recovery-helpers.ts, line 151:
<comment>The oversized path relies on 'clearRecoverySnapshot()' to prevent a later restore from returning the corpus replaced by a prior import. But 'clearRecoverySnapshot' catches a failed 'localStorage.removeItem' and returns without indicating failure (recovery-helpers.ts:196-203); this call then reports 'too-large' and the import proceeds. If removal is refused, the old snapshot remains available and can later restore the wrong corpus.</comment>
<context>Enclosing symbol: persistRecoverySnapshot.</context>
Why this matters: GitNexus flagged this from your code graph — a caller or contract relies on what changed here. · llm-review
| ); | ||
| if (banner === undefined) return 'no banner'; | ||
| const bannerRect = banner.getBoundingClientRect(); | ||
| if (Math.round(bannerRect.bottom) > Math.round(bannerRect.height)) { |
Contributor
There was a problem hiding this comment.
🟡 Warning — Banner-position poll accepts the off-screen entry position
The condition rejects the banner only when its bottom is below its height (i.e. its top is positive). During the stated slide-in from above, the banner has a negative top and bottom less than its height, so this branch does not wait and the poll can proceed while the banner remains off-screen. That defeats the stated purpose of ensuring the geometric probe sees the resting layout; it should require the banner’s top/bottom to be at the resting position, rather than only reject a positive top.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At e2e/accessibility.spec.ts, line 238:
<comment>The condition rejects the banner only when its bottom is below its height (i.e. its top is positive). During the stated slide-in from above, the banner has a negative top and bottom less than its height, so this branch does not wait and the poll can proceed while the banner remains off-screen. That defeats the stated purpose of ensuring the geometric probe sees the resting layout; it should require the banner’s top/bottom to be at the resting position, rather than only reject a positive top.</comment>
Why this matters: GitNexus flagged this from your code graph — a caller or contract relies on what changed here. · llm-review
| // disappear: dropping it while the banner is still leaving would slide | ||
| // the topbar under it for the duration of the exit, and keeping it would | ||
| // leave a permanent gap (GitNexus on PR #925). | ||
| await expect |
Contributor
There was a problem hiding this comment.
🟡 Warning — Release test can pass even if space is cleared during the exit animation
The test waits for paddingTop to become zero, then separately waits for the offline status element to disappear. If a regression clears the reservation immediately when going online while the banner is still animating out, the first poll resolves early and the second simply waits for the banner to disappear; both assertions still pass. Thus this test does not verify the ordering its comment says it protects.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At e2e/accessibility.spec.ts, line 298:
<comment>The test waits for 'paddingTop' to become zero, then separately waits for the offline status element to disappear. If a regression clears the reservation immediately when going online while the banner is still animating out, the first poll resolves early and the second simply waits for the banner to disappear; both assertions still pass. Thus this test does not verify the ordering its comment says it protects.</comment>
Why this matters: GitNexus flagged this from your code graph — a caller or contract relies on what changed here. · llm-review
This branch had an error being deployed
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
2 participants
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.
What ships in this batch
Nine commits rebased onto current main (next 16.3.8, react-day-picker 10.0.2, @types/node 26.6.3 all kept at main's versions):
--offline-banner-heightpublished by the indicator, reserved by the shell; controls are hit-testable at every viewport while offline.useSyncExternalStorepins the server answer; 8 React mismatch warnings per Home load under prefers-reduced-motion → 0, pinned by a new real-browser spec.offlinepaths filter —pnpm build && pnpm run test:e2e:offlineruns where the defect can be found, not on every UI edit. Also clears the two pre-existing workflow lint nits (yamllint comments-indentation, actionlint SC2002) recorded in plans/161 — both files now lint clean.Verification (this tree, this machine)
scripts/minimal_quality_gate.sh(lint + typecheck + full unit suite) green on the rebased tree — via pre-commit on every commit.pnpm run buildclean; regenerated manifest matches the tree (27 hashes moved).e2e/offline-views.spec.ts).hydration-mismatch.spec.ts+accessibility.spec.ts(incl. axe + new Offline banner suite) +touch-targets.spec.ts31/31 on chromium.Merge order note
Depends on nothing. PR #922 (sharp) and #924 (source-map-js override) touch the lockfile; whichever lands last gets an
@dependabot rebase/ lockfile regen. Closing #915 is justified by item 3 above.🤖 Generated with Codebuff
📝 Summary by GitNexus
Summary
This appears to be a broad product and maintenance change spanning offline behavior, recovery, accessibility, dependencies, tests, and roadmap documentation. Its risk posture is elevated by wide graph reach and high-risk CI workflow files.
🔴 CRITICAL blast radius. A cross-cutting PWA, recovery, accessibility, and dependency change across the studio and project tooling, with 49 graph dependents.
The implementation centers on
public/sw.js,public/precache-manifest.json,scripts/generate-precache-manifest.mjs, andsrc/lib/studio/recovery-helpers.ts, alongside studio components and end-to-end tests. The graph impact lands mainly inStudioandViews, with additional impact inSlicesandScripts. The PR also updates dependencies to React 19.3.0 and changes plans includingplans/161-offline-lazy-view-precache-and-framework-refresh-2026-09-30.mdandplans/162-roadmap-progress-and-next-work-2026-10-05.md.Review the recovery helpers, service worker and precache generation path, and the offline and restore tests first. The HIGH risk files are
.github/workflows/ci-and-labels.ymland.github/workflows/security-scan.yml.Added by GitNexus for PR #925. Edit freely — this block is replaced on the next review, everything above it is left untouched.