fix(walkthrough): withhold the tour until the vault is unlocked - #486
Merged
Conversation
The walkthrough merge broke `vault-unlock.spec.ts`: a locked vault issued `GET /api/preferences/walkthrough_completed_version`, and that spec asserts — on the wire, not the DOM — that a locked vault requests NO Keepiq API endpoint. It is an allowlist that fails closed, precisely so a new request behind the lock screen is a deliberate act with a reason. The router guard could not have caught this, and says so: it works by refusing to resolve a route, so it only covers what mounts inside the `<router-view>`. CnAppRoot reads `manifest.walkthrough` and fetches the tour's completion preference when the SHELL mounts, before any route resolves — a shell-level sibling, which that guard's own note warns needs its own gating. So the manifest handed to the shell now omits `walkthrough` while locked. Withheld, not disabled: `isLocked` is read inside the render function, so the tour returns the moment the vault is unlocked and still runs on a user's first unlocked visit. Drawing a product tour over a locked vault would be wrong regardless of the request. Fails closed like the guard beside it — only an explicit `false` unlocks — so a store that failed to initialise withholds the tour rather than shipping it. Both properties are mutation-verified: reading the check as `!store?.isLocked` fails the fail-closed test, and removing the withholding fails the first.
Contributor
Quality Report — ConductionNL/keepiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 111/111 | |||
| npm | ✅ | ✅ 550/550 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | 🚨 NO VERDICT — enabled but never ran | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-29 05:28 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 29, 2026
… instead (#490) Two sessions fixed the same failure minutes apart. #486 withheld the walkthrough from the manifest while the vault is locked, so CnAppRoot never resolves the completion preference and the probe cannot reach the wire. #488 then allowlisted that probe — which the file's own note calls pure masking surface: an entry that cannot fire would silently swallow a real regression of exactly this shape. #486 is the better fix; the entry goes. What is NOT fixed by #486 is the order-dependence. The tour's seen state is per user, not per test, so whichever spec runs first wears it — every spec here unlocks, and the tour is offered on the first unlocked visit by design. audit-trail.spec.ts started needing a retry the moment #484 landed, having never retried in the three preceding development runs. Seeding the marker in global-setup, as dossiq already does, removes that. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
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
1 participant
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.
The walkthrough merge broke
vault-unlock.spec.ts: a locked vault issuedGET /api/preferences/walkthrough_completed_version, and that spec asserts on the wire that a locked vault requests no Keepiq API endpoint. It is an allowlist that fails closed, exactly so a new request behind the lock screen is a deliberate act with a reason — so this fixes the behaviour rather than widening the list.The router guard could not have caught it, and says so in its own comment: it works by refusing to resolve a route, so it only covers what mounts inside the
<router-view>.CnAppRootreadsmanifest.walkthroughand fetches the tour's completion preference when the shell mounts, before any route resolves.The manifest handed to the shell now omits
walkthroughwhile locked. Withheld, not disabled —isLockedis a render dependency, so the tour returns on unlock and still runs on a first unlocked visit. Fails closed like the guard beside it, and both properties are mutation-verified.