refactor(storage): make the project catalog part of the operational database - #2016
Merged
Conversation
…atabase The project catalog decides how every session is organized, yet it stayed in its own `projects.json` when the rest of the File stores were retired. That left the only copy of every project name, relink alias and archive state outside `createOperationalStateBackup`, which captures `runtime.sqlite` plus the Artifact tree and nothing else — a restore silently dropped all of it, and no test could notice because the state was not in the database the validator checks. Schema 21 adds `projects`, `project_locations` and `project_aliases`, keyed by the random project id the renderer already contracts on, with `identity` as a unique index rather than the primary key so `data-project-id` keeps carrying an opaque id instead of a filesystem path. `session_metadata` gains a `project_id` column lifted out of `payload_json`, which makes grouping by project a SQL query for the first time; the column stays two-valued because `json_type` already distinguishes "no project" from "never resolved" for the one caller that needs it. Only persistence moved. Read-modify-write under a serial queue, whole-catalog validation on every write and each method's semantics are untouched, so all 17 catalog contract tests carry over unchanged. `projects.json` is imported once and renamed rather than deleted, so a failed upgrade stays inspectable; this is not legacy-format support but recovery of state this refactor would otherwise strand. The legacy-schema rewind fixtures are brought back in step with the migration list: #1994 left six `DROP TABLE` statements duplicated inside a single `exec`, which failed with `no such table` on the second one, and the new tables need their own teardown. Backup validation failures now carry their cause in the message — with integrity, foreign-key, ~60 required tables and Artifact reconciliation behind one error, a bare "is invalid" tells an operator nothing once `cause` is dropped from a log.
`CreateSessionInput.projectId` is documented as three-valued — an id, an explicit `null` for "no project", and absent meaning "main resolves it automatically" (packages/core/src/runtime-inputs.ts:28-30). #1994 deleted the code that did that resolving along with the File stores, so the third state became permanent: any session whose project was never decided could no longer acquire one, and the sidebar's "group by project" view collapsed every such session into 未归属项目. The e2e case that renames a project failed deterministically as a result, because the ungrouped bucket has no project actions to click. Resolution is restored, scoped by SQL to the sessions that still need it instead of walking every header, and skipping the explicit `null` so a user who detached a session keeps that choice. A session whose directory is gone still resolves — a session outlives the folder it ran in — while a session that fails to resolve is left alone and retried next start rather than frozen into a wrong group. Verified against the failure this fixes: e2e/sidebar-navigation.spec.ts goes from a reproducible 30s timeout to 7/7, with the fixture untouched — the 66 seeded sessions now find their project from `cwd` on their own.
…oped The follow-up review of the SQLite project-authority change found that the rebuilt catalog was correct in structure but wrong in three ways that only show up on a real upgrade, plus one path that never had a consumer. Backfill input. `listSessionsWithUnresolvedProject` returned only id and cwd, so every project was created with "now" as its timestamp and `preferredPath` fell out of session-id order rather than real activity. It also returned subagent sessions, whose disposable Git worktrees then became — and outranked — the user's own project locations. Both are one query: carry the session's last activity and exclude rows that have a subagent parent. Sessions are grouped by directory before resolution so an upgrade pays one Git probe per project instead of one per session. Identity. A directory that no longer exists was canonicalized with `normalize` alone, so on macOS the same folder resolved to `/var/...` after deletion and `/private/var/...` before it, splitting one project in two. The nearest surviving ancestor is now resolved with `realpath` and the missing segments re-appended. Concurrency. Every mutation rewrites the whole catalog, and the read and the write sat in separate transactions, so a second window's rename or archive was replayed away with no error. The synchronous mutations now read, change and rewrite inside one `BEGIN IMMEDIATE`; `select` and `relink` await the filesystem mid-change and re-derive their commit from the state under the lock. Dead weight. `session_metadata.project_id` and its index had no reader — three-valued membership can only be answered by `json_type` on the payload — so the column, the recency index and the unused `databaseLease` dependency are gone rather than kept as a second source of truth. Also: a failed `projects.json` import no longer takes the read path down with it, the set-aside file is timestamped so a second attempt cannot overwrite the first, the catalog releases its database lease, and project resolution runs after session recovery instead of ahead of it. Tests cover what the reviews found untested: the backup/restore round trip that is this change's whole premise, the legacy import in both its success and failure branches, identity stability across deletion, recency order, subagent exclusion, unresolvable directories, and the concurrent lost update. Each was confirmed to fail against the unfixed code. The e2e fixture resolves projects while seeding, so sidebar tests assert on a settled state instead of racing the startup resolver.
…decisions Two reviews of the previous commit found the same thing from different angles: its concurrency guarantees were asserted in comments but not implemented. The backfill claimed a user detaching a session mid-resolution would win, but read the header and wrote it in two separate transactions, so a detach landing between them was overwritten back to a resolved project. `updateHeaderVersioned` already exists for exactly this; the listing now carries each row's metadata revision and the assignment is fenced by it. The unreliable extra read is gone rather than kept alongside the fence — a version conflict simply means someone decided first, and a session that is still unresolved is retried next start. `relink` re-derived its merge from the catalog as it stood at commit time. That kept the catalog self-consistent while leaving it inconsistent with the world: `beforeCommit` reassigns the sessions of the project being merged away, and those writes cannot be re-derived. If the merge target moved while the callback ran, the sessions had already been handed to the wrong owner. The commit now refuses when the conflict it would merge is not the one the callback was shown. Relink was already retryable — a throwing callback leaves the catalog untouched — so this hands the decision back instead of committing a half-true one. A historical working directory whose ancestor was replaced by a plain file raises ENOTDIR, not ENOENT, and was left permanently unresolved even though walking one level further up canonicalizes it exactly like a deleted directory. Both mean "cannot reach this path"; they are now handled together. Finally, `mutate` made `withQueue` redundant for every fully synchronous mutation: the SQLite write lock does that job now. The queue is kept only where it still earns its place — `select` and `relink`, which await the filesystem or a caller's callback mid-change. Removing it elsewhere deletes five `Failed to …` branches that `mutate` had already made unreachable. Each new test was confirmed to fail against the unfixed code.
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.
Summary
mainis currently red one2e_shard (2/2), and the cause is not a flake: #1994 removed the code that resolves a session's project, soe2e/sidebar-navigation.spec.ts:72times out deterministically.CreateSessionInput.projectIdis documented as three-valued — an id, an explicitnullfor "no project", and absent meaning "Main resolvesundefinedautomatically" (packages/core/src/runtime-inputs.ts:28-30). The code doing that resolving was deleted alongside the File stores, which turned "never decided" into a permanent state: every such session falls into 未归属项目 forever, and that bucket has no project actions to click. This affects real upgrades, not just the fixture — any session that predates its project assignment loses its grouping.Investigating it surfaced the reason the bridge could be deleted unnoticed: the project catalog never moved into SQLite.
projects.jsonand a class literally namedFileProjectCatalogsurvived a PR titled "make SQLite the sole operational authority". That left the only copy of every project name, relink alias and archive state outsidecreateOperationalStateBackup, which capturesruntime.sqliteplus the Artifact tree and nothing else — a backup/restore round trip silently drops all of it today, and the new validator cannot catch it because the state is not in the database it checks.So this PR closes the gap rather than reattaching the bridge:
projects,project_locations,project_aliases, keyed by the random project id the renderer already contracts on, withidentityas a unique index rather than the primary key —data-project-idkeeps carrying an opaque id instead of a filesystem path.projects.jsonis imported once and set aside rather than deleted. This is not legacy-format support — it recovers state this refactor would otherwise strand, which is why it is not covered by refactor(storage): make SQLite the sole operational authority #1994's "no old data migration" boundary. An import that cannot complete is reported and skipped: SQLite is the authority now, so it must not take the read path down with it.BEGIN IMMEDIATE. The catalog rewrites its whole table on each write, so splitting the read from the write let a second window's rename or archive be replayed away with no error anywhere.nullso a deliberate detach survives, and skipping subagent sessions — they inherit their parent's project, and their disposable Git worktrees must never become one of the user's project locations./var/...after deletion and/private/var/...before it and split one project in two.Every method's user-visible semantics are unchanged, so all 17 existing catalog contract tests carry over unchanged.
Refs #1994
Review round
Four independent reviews of the first two commits agreed on one root cause behind most findings — the backfill's input contract — and on one path with no demand. Both are fixed in
390e61f4rather than patched around:session_metadata.project_idcolumn, its index and the recency index had zero readers: three-valued membership can only be answered byjson_typeon the payload, so the column could never serve that query. They are deleted, along with the unuseddatabaseLeasedependency. An earlier revision of this description claimed the column made grouping by project a SQL query for the first time; that was wrong, and the column is gone.Verification
@maka/storage@maka/desktopmainlint/formatNew tests cover what the reviews found untested — the backup/restore round trip that is this change's whole premise, the legacy import in both branches, identity stability across deletion, recency order, subagent exclusion, unresolvable directories, and the concurrent lost update. Each was confirmed to fail against the unfixed code before being fixed, e.g.:
The e2e fixture now resolves projects while seeding, so the sidebar tests assert on a settled state instead of racing the startup resolver, which runs concurrently with window creation.
The regression this fixes, before and after, with the fixture untouched: