Skip to content

test(settings-store): inventory deferred load facade locals - #1649

Merged
ewen-poch merged 1 commit into
mainfrom
test/settings-store-architecture-inventory
Aug 24, 2026
Merged

test(settings-store): inventory deferred load facade locals#1649
ewen-poch merged 1 commit into
mainfrom
test/settings-store-architecture-inventory

Conversation

@ewen-poch

Copy link
Copy Markdown
Member

Problem

perf(startup): defer transcript and runtime probes (#1637) changed settings-store load() so the persisted Settings snapshot can publish before runtime probes finish. The architecture inventory still expected the previous combined Promise.all binding and a single catch (error).

That guard only runs in the full portable suite (Full Module tests (macOS)), not in test:module, so later PRs fail PR Gate even when they do not touch Settings.

Observed on PRs including #1641, #1643, #1648, #1524, and #1432:

shouldInitializeRuntime is new facade state
settingsPromise is new facade state
runtimeInitialization is new facade state
snapshot is new facade state
[preflight, encryptionAvailable, npmAvailable] is new facade state
[snapshot, preflight, encryptionAvailable, npmAvailable] declared 0 times
error declared 2 times

Proposed change

Update the facade variable inventory to the current load() locals and allow the two catch (error) bindings. Production load behavior is unchanged.

Scope and non-goals

Acceptance criteria and validation

  • Architecture inventory matches the deferred-load load() locals.
  • New undeclared facade bindings are still rejected.
  • After the last material edit:
    • npm test -- src/renderer/src/stores/settings-store.architecture.test.ts src/renderer/src/stores/settings-store.test.ts → pass
    • npx vitest run --project architecture → 28 files / 221 tests pass
    • npx eslint src/renderer/src/stores/settings-store.architecture.test.ts → pass

Review focus

Confirm the inventoried names are load-local control flow from #1637, not new feature actions or Settings writes on the facade.

Historical compatibility

None. No schema, Session JSON, or on-disk format change.

New enum / state values

None.

Persistence

None.

#1637 split Settings load so the snapshot can publish before runtime
probes finish. The architecture inventory still expected the previous
combined Promise.all binding, so Full Module tests failed on every PR.
@github-actions

Copy link
Copy Markdown

Codex Review

Verdict: mergeable

No actionable findings.

Summary: Static inspection found no concrete merge-blocking defects in the pull request changes. The architecture test inventory matches the current Settings facade declarations.

@github-actions github-actions Bot added the ready-to-merge All completed AI reviewers found this pull request mergeable. label Aug 24, 2026
@ewen-poch
ewen-poch merged commit f2d8d88 into main Aug 24, 2026
24 checks passed
@ewen-poch
ewen-poch deleted the test/settings-store-architecture-inventory branch August 24, 2026 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge All completed AI reviewers found this pull request mergeable.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant