Skip to content

Commit ae5e923

Browse files
committed
fix(database): bound recovery checkpoint path
Summary: Reuse the bounded canonical checkpoint basename for selected-database recovery so recovery cannot reintroduce the Windows SQLite path amplification removed from automatic startup. Refs: - Change: spec/changes/active/windows-canonical-authority-second-launch - Change: spec/changes/active/database-resource-lifecycle-hardening Verification: - Desktop canonical storage recovery test: 6/6 passed - Desktop typecheck: passed - Focused Desktop ESLint: passed - Spec governance and traceability: passed for 15 active changes - Affected-file Prettier and git diff --check: passed - Full release harness and real Windows packaged two-launch gate: pending final isolated candidate
1 parent 7f1b05f commit ae5e923

14 files changed

Lines changed: 80 additions & 31 deletions

File tree

apps/desktop/src/main/services/canonical-storage-recovery.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ function recoverSelectedDatabase(
8888
checkpointPath: path.join(
8989
options.activeRoot,
9090
"cache",
91-
`.canonical-recovery-checkpoint-${process.pid}-${crypto.randomUUID()}`,
91+
`.canonical-checkpoint-${crypto.randomUUID()}`,
9292
),
9393
deviceId: deriveLocalResourceDeviceId(options.activeRoot),
9494
mcpLibrary,

apps/desktop/tests/unit/main/canonical-storage-recovery.test.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* @vitest-environment node
33
*/
44
import { beforeEach, describe, expect, it, vi } from "vitest";
5+
import path from "node:path";
56
import type { ExtractedMcpSecret } from "@prompthub/core";
67

78
import type { McpResourceSecretEncryption } from "../../../src/main/services/mcp-resource-secret-store";
@@ -93,7 +94,7 @@ describe("canonical storage recovery orchestration", () => {
9394
});
9495
});
9596

96-
it("closes SQLite and rebuilds canonical storage with device-bound secrets", async () => {
97+
it("closes SQLite and rebuilds through a bounded checkpoint with device-bound secrets", async () => {
9798
const onSuccess = vi.fn();
9899
const onFailure = vi.fn();
99100
const scheduleRelaunch = vi.fn();
@@ -152,15 +153,21 @@ describe("canonical storage recovery orchestration", () => {
152153
activeRoot: "/root",
153154
sourceDatabasePath: "/root/data/prompthub.db",
154155
deviceId: "device-test",
155-
checkpointPath: expect.stringContaining(
156-
"/root/cache/.canonical-recovery-checkpoint-",
157-
),
156+
checkpointPath: expect.any(String),
158157
mcpLibrary: expect.objectContaining({
159158
kind: "prompthub-mcp-library",
160159
servers: [],
161160
}),
162161
}),
163162
);
163+
const recoveryOptions = mocks.recoverCanonicalStorageAuthorityFromDatabase
164+
.mock.calls[0]?.[0] as { checkpointPath?: unknown } | undefined;
165+
const checkpointPath = String(recoveryOptions?.checkpointPath);
166+
expect(path.dirname(checkpointPath)).toBe(path.join("/root", "cache"));
167+
expect(path.basename(checkpointPath)).toMatch(
168+
/^\.canonical-checkpoint-[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/u,
169+
);
170+
expect(path.basename(checkpointPath).length).toBeLessThanOrEqual(64);
164171
expect(mocks.createMcpResourceSecretStore).toHaveBeenCalledWith({
165172
filePath: "/root/secrets/mcp-resource-secrets.json",
166173
encryption,

spec/changes/active/database-resource-lifecycle-hardening/design.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ Canonical catalog, full-storage catalog, checkpoint verification/snapshot,
2222
catalog self-heal, file-authoritative recovery, and consistent-image failure
2323
paths use the database primitive. Checkpoint directory staging remains a
2424
separate bounded same-parent directory because it contains a complete tree, not
25-
only SQLite artifacts.
25+
only SQLite artifacts. Both automatic startup and selected-database recovery
26+
use the same `.canonical-checkpoint-<uuid>` target form; a recovery label or PID
27+
does not add another unbounded ancestor segment.
2628

2729
No success-path data, schema, hash, quick-check, rename, or publication order
2830
changes.

spec/changes/active/database-resource-lifecycle-hardening/implementation.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ pending, so publication is not yet claimed.
1414
- The canonical checkpoint directory stage duplicated its long target basename,
1515
so bounded leaf filenames alone did not keep the complete SQLite path within
1616
budget.
17+
- The selected-database recovery caller still added a longer recovery label and
18+
PID to its checkpoint ancestor after automatic startup had been bounded.
1719
- Task-owned database cleanup loops omitted `.lock` in Core, DB, and Desktop
1820
failure paths.
1921
- Cherry Studio and Hermes could leak an opened connection during schema
@@ -36,6 +38,8 @@ pending, so publication is not yet claimed.
3638
- Canonical startup now uses `.canonical-checkpoint-<uuid>` and checkpoint
3739
construction uses `.checkpoint-stage-<uuid>`, removing the long duplicated
3840
ancestor that still put a short SQLite leaf over the Windows budget.
41+
- Selected-database recovery now uses the same `.canonical-checkpoint-<uuid>`
42+
target instead of `.canonical-recovery-checkpoint-<pid>-<uuid>`.
3943
- Cherry Studio and Hermes close stores rejected during schema validation;
4044
Cherry Skill closes capability-probe failures; NanoClaw closes its inbound
4145
database when its paired outbound database cannot open.
@@ -66,6 +70,8 @@ pending, so publication is not yet claimed.
6670
instrument source outside `packages/core`, so it reported 0 files for that
6771
cross-package helper despite all 10 tests executing; this tooling limitation
6872
is recorded rather than presented as numeric coverage.
73+
- The selected-database recovery checkpoint regression passed 6/6 tests after
74+
the final audit finding; Desktop typecheck and focused ESLint passed.
6975
- Static Windows modeling with the release smoke's long root puts final
7076
verification/catalog-stage/post-publication verify paths at 198/194/199
7177
characters and their `.lock` paths at 203/199/204, respectively. The real

spec/changes/active/database-resource-lifecycle-hardening/proposal.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ weakness elsewhere.
2727
names and complete artifact cleanup.
2828
- Apply complete cleanup to canonical catalog/checkpoint/rebuild, safety-point,
2929
and file-authoritative recovery temporaries.
30+
- Remove avoidable checkpoint-directory amplification from both automatic
31+
startup and selected-database recovery.
3032
- Close Cherry Studio, Hermes, and NanoClaw database handles on every validation
3133
and multi-open failure path.
3234
- Add failure, lock-artifact, path-boundary, and integration regressions.

spec/changes/active/database-resource-lifecycle-hardening/specs/database-lifecycle/spec.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,23 @@ lock without the existing lease policy.
2222

2323
PromptHub-owned temporary SQLite basenames MUST use a validated fixed label and
2424
a full UUID, remain at or below 64 characters, and not incorporate an arbitrary
25-
destination basename.
25+
destination basename. PromptHub-owned checkpoint ancestors that contain these
26+
databases MUST likewise use a bounded UUID form rather than adding operation
27+
names and process identifiers without a fixed budget.
2628

2729
#### Scenario: caller provides an unsafe or oversized label
2830

2931
- **When** a temporary database path is requested with traversal, separators,
3032
an empty value, or a label that would exceed the basename budget
3133
- **Then** path construction fails before creating a file
3234

35+
#### Scenario: canonical recovery prepares a checkpoint
36+
37+
- **When** automatic startup or selected-database recovery prepares a canonical
38+
checkpoint
39+
- **Then** the checkpoint basename is bounded independently of the active root
40+
- **And** both flows use the same fixed-size UUID form
41+
3342
### `FR-DBLIFE-001`: External Session Store Validation Releases Handles
3443

3544
Agent session adapters MUST close every SQLite handle they open when schema

spec/changes/active/database-resource-lifecycle-hardening/tasks.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
- [x] `T-DBLIFE-001` (`NFR-DBLIFE-001`, `NFR-DBLIFE-002`,
44
`DES-DBLIFE-001`, `TEST-DBLIFE-001`): add the bounded task-owned database
55
primitive and adopt complete artifact cleanup across canonical, recovery,
6-
checkpoint, and safety-point paths.
6+
checkpoint, and safety-point paths, including bounded automatic and
7+
selected-database recovery checkpoint ancestors.
78
- [x] `T-DBLIFE-002` (`FR-DBLIFE-001`, `DES-DBLIFE-003`,
89
`TEST-DBLIFE-003`): close Cherry Studio, Hermes, Cherry Skill, and NanoClaw
910
handles on every validation and paired-open failure.

spec/changes/active/windows-canonical-authority-second-launch/design.md

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,14 @@ the full checkpoint path. The catalog builder then uses
1919
- same parent directory as the destination
2020
- unchanged create, verify, rename, and SQLite sidecar cleanup sequence
2121

22-
The startup checkpoint target uses `.canonical-checkpoint-<uuid>`, and
23-
`createCanonicalStorageCheckpoint()` uses `.checkpoint-stage-<uuid>` in the
24-
same parent. This removes the earlier target-basename duplication from every
25-
ancestor of the temporary SQLite catalogs. With the release smoke's long
26-
Windows runner profile, the resulting database and `.lock` paths retain
27-
explicit headroom below the VFS limit rather than relying on a short leaf name
28-
inside an already overlong directory.
22+
Startup and selected-database recovery checkpoint targets both use
23+
`.canonical-checkpoint-<uuid>`, and `createCanonicalStorageCheckpoint()` uses
24+
`.checkpoint-stage-<uuid>` in the same parent. This removes the earlier
25+
target-basename duplication from every ancestor of the temporary SQLite
26+
catalogs and avoids reintroducing a longer recovery-only ancestor. With the
27+
release smoke's long Windows runner profile, the resulting database and `.lock`
28+
paths retain explicit headroom below the VFS limit rather than relying on a
29+
short leaf name inside an already overlong directory.
2930

3031
Naming is `O(1)` time and space. Catalog construction remains `O(B + R)` for
3132
canonical bytes and records; no additional scan, copy, database open, or
@@ -75,9 +76,9 @@ the required black-box coverage.
7576

7677
- `TEST-WINCAT-001`: Core and Desktop filesystem/SQLite regressions capture the
7778
verification database and actual stage used for long checkpoint/catalog
78-
destinations, including the checkpoint directory stage, and assert basename
79-
budgets, sibling placement, target independence, successful publication, and
80-
existing failure cleanup.
79+
destinations, including startup/recovery checkpoint targets and the
80+
checkpoint directory stage, and assert basename budgets, sibling placement,
81+
target independence, successful publication, and existing failure cleanup.
8182
- `TEST-WINCAT-002`: Desktop unit/contract tests cover the release-only auto-exit
8283
guard, both signal orders, exactly-once exit, migration failure behavior, two
8384
launches, per-launch event boundaries, clean exit, and workflow blocking

spec/changes/active/windows-canonical-authority-second-launch/implementation.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ external-store handle findings are tracked separately in
4646
repeating the target basename. Under the deliberately long Windows release
4747
profile, the final verification, build-stage, and post-publication database
4848
lock paths model at 203, 199, and 204 characters rather than exceeding 260.
49+
- The database-wide follow-up found that selected-database recovery still used
50+
`.canonical-recovery-checkpoint-<pid>-<uuid>`. Recovery now shares the bounded
51+
`.canonical-checkpoint-<uuid>` form, so the same path amplification cannot
52+
return outside normal startup.
4953
- The packaged Windows smoke keeps one deliberately long runner-owned profile
5054
for two launches. The first launch requires the upgrade safety snapshot,
5155
`waiting-renderer-migration`, and `window_ready`; the second reads only newly
@@ -101,6 +105,9 @@ basename.
101105
tests and Desktop 121/121 tests across release smoke, canonical publication,
102106
recovery, safety points, and affected external database adapters. DB, Core,
103107
and Desktop typechecks plus targeted Desktop ESLint passed.
108+
- The final selected-database recovery checkpoint regression passed 6/6 tests;
109+
Desktop typecheck, focused ESLint, spec governance, and formatting passed
110+
after the recovery-only ancestor was shortened.
104111
- The release-smoke lifecycle helper reached 100% statement, branch, function,
105112
and line coverage. Core canonical path/cleanup coverage reached 99.52%
106113
statements/lines, 98.37% branches, and 100% functions across the two touched

spec/changes/active/windows-canonical-authority-second-launch/proposal.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,10 @@ detect a failure that appears when the same profile is opened again.
3131
- Keep canonical SQLite verification and stage filenames short and independent
3232
of the checkpoint/destination basename while preserving same-directory
3333
publication and atomic rename.
34-
- Keep the checkpoint target and its pre-publication directory stage bounded as
35-
well; a short SQLite basename is insufficient when an internally amplified
36-
parent path already consumes the Windows VFS budget.
34+
- Keep startup and selected-database recovery checkpoint targets, plus their
35+
pre-publication directory stage, bounded as well; a short SQLite basename is
36+
insufficient when an internally amplified parent path already consumes the
37+
Windows VFS budget.
3738
- Preserve existing stage cleanup for create, quick-check, graph-hash, and
3839
destination-race failures.
3940
- Extend the packaged Windows x64 upgrade smoke to launch the same isolated

0 commit comments

Comments
 (0)