diff --git a/docs/DATABASE_SCHEMA.md b/docs/DATABASE_SCHEMA.md index da7c4e7..741b934 100644 --- a/docs/DATABASE_SCHEMA.md +++ b/docs/DATABASE_SCHEMA.md @@ -5,8 +5,11 @@ schema created by [`migrations/001_init.sql`](../migrations/001_init.sql) (rever [`migrations/001_init.down.sql`](../migrations/001_init.down.sql)). - Engine: PostgreSQL 13+ -- Migration tool: `node-pg-migrate`, raw-SQL mode (`001_init.sql` / `001_init.down.sql` - is one up/down migration pair) +- Migration tool: a small custom runner (`scripts/migrate.ts`) over plain numbered + raw-SQL files (`001_init.sql` / `001_init.down.sql` is one up/down migration pair) — + not `node-pg-migrate`; an earlier draft of this doc named that tool, but it was never + added as a project dependency. See `docs/TRD.md` §1 and `scripts/migrate.ts`'s header + comment for why. - Primary keys: `uuid`, generated with `gen_random_uuid()` (from the `pgcrypto` extension, enabled by the migration) - Timestamps: `timestamptz`, `created_at`/`updated_at` default to `now()` @@ -242,6 +245,25 @@ The top-level entity: one row per tracked codebase/initiative. | `updated_at` | `timestamptz` | `NOT NULL DEFAULT now()` | Auto-maintained by `trg_projects_set_updated_at` | | `deleted_at` | `timestamptz` | nullable | `NULL` = active. See [Soft delete](#soft-delete-vs-hard-delete-for-projects) | +**Constraints:** `uq_projects_source_ref_active` (added by +`migrations/002_projects_unique_source_ref.sql`) — unique partial index on +`(source_type, source_ref) WHERE deleted_at IS NULL`. Backs the documented +`kt_register_project` upsert invariant ("`(source_type, source_ref)` is +unique; calling again with the same pair updates the existing row, never +creates a duplicate") at the database level via `INSERT ... ON CONFLICT ... +DO UPDATE` targeting this index, closing a race where two concurrent +first-registrations of the same `source_ref` could otherwise both insert. +Scoped to non-soft-deleted rows, so soft-deleted projects never collide +with an active one reusing the same `(source_type, source_ref)`. The +`source_ref` column itself is nullable at the DB level (no `NOT NULL` +constraint in `migrations/001_init.sql`), but `kt_register_project`'s +input schema requires it as a non-empty string (`z.string().min(1)`, +`src/schemas/tools.ts`) for every `source_type` including `'local'` +(a filesystem path, per `docs/TRD.md`'s tool contract) — so in the +current build no row is ever actually inserted with `source_ref IS +NULL` through the real registration path; the column's nullability is +unused headroom, not something `source_type='local'` projects rely on. + **Indexes:** `idx_projects_not_deleted` — partial index on `(id) WHERE deleted_at IS NULL`, backing the "active projects" filter every read path applies. @@ -459,6 +481,19 @@ tracked state — e.g. a file changed that isn't linked to any known item | `raised_at` | `timestamptz` | `NOT NULL DEFAULT now()` | | | `resolved_at` | `timestamptz` | nullable | `NULL` = still open. Set once a human or automated process resolves the flag. | +**Constraints:** `uq_drift_flags_open_item_kind` (added by +`migrations/003_drift_flags_open_unique.sql`) — unique partial index on +`(item_id, kind) WHERE resolved_at IS NULL`. Backs the "at most one open +flag per `(item_id, kind)`" invariant the rest of the system assumes, at +the database level via `INSERT ... ON CONFLICT ... DO NOTHING` in +`src/db/queries/drift-flags.ts`, closing a race where two concurrent +`kt_record_session_summary` calls scanning the same out-of-sequence item +could otherwise both insert an open flag for it. `item_id` is nullable +(`ON DELETE SET NULL`), so multiple resolved-at-null rows with +`item_id IS NULL` would remain unaffected by this constraint — not a gap +in practice, since the only kind this build raises (`out_of_sequence`) +always sets `item_id`. + **Indexes:** - `idx_drift_flags_project_id` on `(project_id)` - `idx_drift_flags_track_id` on `(track_id)` diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 5d7849b..02ce36c 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -181,6 +181,62 @@ composed entirely from tracks/items already implemented by `T2.3`–`T2.8`, so it is built and unit-tested as part of `T2.15`'s hardening pass rather than getting its own numbered item. +**Status reconciliation (added retroactively — this Track's items above +describe the plan, not yet what shipped).** As of the current build, only +5 of the 14 tools have the full implementation this Track calls for: +`kt_register_project` (`T2.2`), `kt_create_track` (`T2.3`), +`kt_create_item` (`T2.4`), `kt_get_project_status` (`T2.7`), and +`kt_record_session_summary` (`T2.9` — see below, it actually exceeds its +own acceptance criterion). The other 9 are registered as stubs +(`src/mcp/tools/stubs.ts`: correct request/response shape, no real +logic, no external calls) rather than the tools this Track's items +describe: +- **`T2.5` (`kt_list_tracks`), `T2.6` (`kt_get_track`), `T2.8` + (`kt_update_item_status`), `T2.10` (`kt_record_decision`), `T2.12` + (`kt_render_roadmap`), and the `kt_get_next_steps` read query above** — + this Track's acceptance criteria call for all six to be **fully** + implemented (not stubs). None currently is. This is the gap behind the + "old model path" flag and the PR #1 CodeRabbit deferrals recorded in + this doc's backlog section — it was never previously written down in + one place that six specific tools are behind this Track's own stated + scope, not merely "not yet built" in the abstract. +- **`T2.9` (`kt_record_session_summary`)** exceeds its own acceptance + criterion, but not by fully doing `T6.1`'s job. The `T2.9` criterion + asks only for an `events` insert "with no drift analysis performed + yet"; the shipped version already runs a real scoped check + (`findSequenceSkips`, wired in via `adversarial-review` fixes) and + writes/resolves `drift_flags` rows for it, targeting the DB's + `kind='out_of_sequence'` — the same thing TRD Appendix C calls + `SEQUENCE_SKIP` (positional: an earlier item in the same track still + `pending`/`blocked` while a later one is `done`). That is **not** what + `T6.1`'s acceptance criterion literally asks for ("an item marked + `done` while an item it `depends_on` via `item_dependencies` is not + `done`" — a dependency-graph check, closer to TRD's `DEPENDENCY_GAP`, + which is a different rule already enforced synchronously by + `kt_update_item_status`'s 409 check, not by this drift check). + `T6.1`'s own title ("out-of-sequence detection") and its + `kind='out_of_sequence'` target don't actually match its + acceptance-criterion wording either — a separate, small inconsistency + in `T6.1` itself, worth fixing when `T6` is actually built rather than + papered over here. `T6.2` (orphan-file-change) isn't implemented at + all, so `T6.3`'s "both heuristics" criterion is unmet regardless of + how `T6.1` gets reconciled. `T2.11` (`kt_check_drift`) and the two + sync stubs (`T2.13`, `T2.14`) do **not** actually match their + stub-only acceptance criteria either, on closer check: + `registerStubTools` routes every one of the 9 stub tools through the + same generic `notImplementedResult` (`src/mcp/tool-helpers.ts`), which + always returns a uniform `500 INTERNAL_ERROR` — not `T2.11`'s specific + "empty result with a `no heuristics configured` note", and not + `T2.13`/`T2.14`'s specific "adapter not configured" result after + validating the item exists. The generic-500 stub shape is the same for + all 9 unimplemented tools; none of the three has the bespoke + stub-response behavior its own T2 line calls for. +- Nothing in `T1.6`'s "cross-document consistency... zero open + discrepancies" gate accounted for this Track-vs-build gap either — it + checks the docs against each other, not the docs against what actually + got built. Worth knowing before treating "T1 done, T2 blocked" as + literal build status. + --- ## T3 — Deploy + auth (Railway reference deployment) @@ -556,3 +612,101 @@ scope decisions, not bugs, per the `clear-decisions` walkthrough:** `kt_get_project_status` is re-verified against real code — but the three stub sections' prose can and should be fixed now, against the schemas already shipping. + +**Deferred from a documentation-completeness audit (2026-08-24), prompted +by "do we have clarity on the gaps and how it maps to the roadmap":** +- **Fixed in this same round, not a backlog item — TRD.md's Appendix A + and §5 described a schema that was never built.** TRD Appendix A + carried a full hand-copied DDL block that had drifted from the real + schema (`migrations/001_init.sql`) across nearly every table: a + fictional `adapter_credentials` table instead of the real `adapters` + table; a `projects.adapters` column and a non-nullable `source_ref` + that don't exist; a `project_id` column directly on `items` that + doesn't exist; a `drift_flags.flag_type`/`severity`/`status` shape + where the real table has `kind`/`detail`/`resolved_at`; and no + `api_tokens` table at all. §5's credential-storage description had the + same `adapter_credentials`/`key_version` drift, independently + discovered and partly self-documented already in + `src/crypto/credential-cipher.ts`'s header comment. Fixed by pointing + Appendix A at the real migration files instead of duplicating them + (the duplication is what let this drift accumulate unnoticed), and + correcting §5's storage/never-returned/known-gap bullets to match + `src/db/queries/adapters.ts`. Also fixed: §1's Tech Stack table still + named `node-pg-migrate` as the migration tool; the shipped build uses + a hand-written raw-SQL + custom-runner approach instead + (`scripts/migrate.ts`), and `node-pg-migrate` isn't even a project + dependency. Also fixed `DATABASE_SCHEMA.md`'s top-of-doc "Migration + tool" line, which had the same wrong claim and would have undermined + this Appendix pointing readers there as the source of truth. +- **`T9.x` (new, unscheduled) — sweep remaining stale `node-pg-migrate`/ + `adapter_credentials` mentions.** This audit fixed the load-bearing + instances (TRD §1/§5/Appendix A/B, `DATABASE_SCHEMA.md`'s top-of-doc + migration-tool line) but didn't chase every mention repo-wide: + `PRD.md` §5.3 (two `adapter_credentials` mentions), `ARCHITECTURE.md` + (three `node-pg-migrate` mentions — tech-stack summary, component + diagram label, and a deployment-topology aside), and + `DATABASE_SCHEMA.md`'s two remaining secondary `node-pg-migrate` + mentions (both in parenthetical rationale, lower-stakes than the + top-of-doc claim already fixed). Also found this round but not fixed: + TRD §2's Repository Layout tree lists the 9 unimplemented tools as + separate files under `src/mcp/tools/` when they're all actually in one + `stubs.ts`; lists a `decisions.ts` query file that doesn't exist yet + (nothing writes `decisions` until `kt_record_decision` is built); and + shows a `src/adapters/` tree that doesn't exist at all yet (`T5` not + started). None of these are load-bearing the way the fixed ones were, + but they're stale and should be swept in one pass rather than + piecemeal. +- **`T9.x` (new, unscheduled) — encryption-key rotation.** TRD §5's + "Known gap" bullet says compromising `KNOTRACK_ENCRYPTION_KEY` today + has no in-band recovery path: there is no `key_version` column on + `adapters` (so old and new keys can't be run side-by-side mid-rotation) + and no `scripts/rotate-encryption-key.ts` (or corresponding + `package.json` script) to do the re-encrypt sweep. This item is what + that TRD bullet points to as "tracked as follow-up work" — previously + that claim wasn't backed by an actual backlog entry. Needs: a new + migration adding `adapters.key_version integer NOT NULL DEFAULT 1`, + and a rotation script that decrypts every `adapters.encrypted_credential` + with the key matching its row's `key_version`, re-encrypts with the + new key, and bumps `key_version`. **This is not a pre-`T5` deferral — + it's live now.** `kt_register_project` (`T2.2`, already fully shipped, + not a stub) already accepts `adapters.github`/`adapters.linear` in its + input and calls `encryptCredential` + `upsertAdapter` + (`src/mcp/tools/register-project.ts`) today; adapter rows with real + encrypted credentials can exist in any deployment right now, well + before `T5`'s sync clients are built. A compromised + `KNOTRACK_ENCRYPTION_KEY` today has no rotation path for whatever + credentials are already stored. The team should prioritize + encryption-key rotation ahead of, not after, further `T5` work. +- **`T9.x` (new, unscheduled) — `SYNC_DRIFT`'s missing schema.** TRD + Appendix B's `SYNC_DRIFT` drift-flag rule depends on + `last_github_sync_at`/`last_linear_sync_at` columns that don't exist + anywhere in the real schema — not on `tracks`, not on `adapters`, + confirmed against `migrations/001_init.sql`. Not a regression (the + `drift_flags.kind` CHECK doesn't even have a sync-drift value yet + either — this rule was never built, only specified), but it means + `SYNC_DRIFT` can't be implemented as currently documented without a + schema change first. This needs Paul's call, not a guess: add the two + `last_*_sync_at` columns via a new migration. They must be scoped per + **track** (either directly on `tracks`, or on a new join table keyed + by both `track_id` and `adapter_id`) — **not** on `adapters` alone. + `uq_adapters_project_type` allows only one `adapters` row per + `(project_id, type)`, so a project with multiple tracks syncing + through the same GitHub/Linear adapter would share a single + `last_*_sync_at` value: syncing one track would advance that shared + timestamp and make every other unsynced track in the project look + current too. Putting the column(s) on `adapters` is not a valid + alternative to `tracks` for any project with more than one track using + the same adapter type — only "redefine the rule against columns that + already exist" remains a real alternative to a `tracks`-scoped column. + Blocks real progress on `T6` until decided; not urgent before then + since `T6` depends on `T5` (adapters), which hasn't started. +- **Process note, not a backlog item — `T1.6`'s sign-off gate has never + formally closed.** `T1.6`'s acceptance criterion is a cross-document + consistency pass with "zero open discrepancies," recorded in + `docs/SIGNOFF.md` — that file doesn't exist in this repo. This audit + alone found three more discrepancies beyond the ones already fixed + across PR #1/#2 (the T2 planned-vs-shipped gap and the TRD Appendix + A/§5/§1 drift above), which is itself evidence `T1.6` was never + actually satisfied — not a new problem to fix here, just worth naming + plainly rather than treating "T1 done, T2 blocked" as literal status + (see the reconciliation note under `T2`, above). diff --git a/docs/TRD.md b/docs/TRD.md index c16ca03..33b543d 100644 --- a/docs/TRD.md +++ b/docs/TRD.md @@ -25,7 +25,7 @@ Deployment model: **self-hosted, single-tenant per instance.** One running KnoTr | MCP implementation | `@modelcontextprotocol/sdk` (official) | Only implementation guaranteed to track the MCP spec's wire format and transport details across revisions; hand-rolling JSON-RPC framing is pure risk with no upside. | | Database | PostgreSQL (only supported DB) | One of three documented deploy targets is Render's free tier, which has **no attachable persistent local disk** — a SQLite/file-based DB would silently lose all data on every restart there. Postgres is available managed on all three targets (Supabase, Railway, Fly), so it's the only option that works identically everywhere. | | DB driver | `pg` (node-postgres) | Minimal, direct SQL, no query-builder magic to fight when writing the recursive/graph queries drift detection and dependency validation need. | -| Migrations | `node-pg-migrate` | Produces plain, numbered, human-readable migration files that generate straight SQL; avoids pulling in a full ORM (Prisma/TypeORM) whose schema-modeling layer this project doesn't need and whose extra runtime/dependency weight self-hosted installers shouldn't have to carry. | +| Migrations | Hand-written raw SQL + a small custom runner (`scripts/migrate.ts`) | An earlier draft of this doc specified `node-pg-migrate`; the shipped build instead uses plain numbered `.sql`/`.down.sql` pairs under `migrations/`, applied by a small idempotent runner that tracks applied migrations in a `schema_migrations` table (see `scripts/migrate.ts`'s header comment for the reasoning) — `node-pg-migrate` is not a project dependency. This still avoids a full ORM's (Prisma/TypeORM) schema-modeling layer and extra runtime weight, the same goal the original choice served; it just isn't `node-pg-migrate` specifically. | | HTTP framework | Fastify | Lightweight, first-class TypeScript types, low overhead, and its raw Node `req`/`res` are directly compatible with the MCP SDK's Streamable HTTP transport, which attaches to the raw HTTP layer rather than an Express-style middleware chain. | | Testing | Vitest | Native ESM/TS support with no Babel/ts-jest transform step; fast enough to run the full suite (unit + integration against a real Postgres) on every commit. | | Linting | ESLint + `typescript-eslint` | Type-aware lint rules catch a class of bugs (unsafe `any`, unchecked promise rejections) that matter a lot in code that talks to arbitrary untrusted MCP clients. | @@ -74,10 +74,6 @@ knotrack/ │ │ └── tools.ts # one zod schema per tool; single source of truth, converted to JSON Schema for tools/list │ ├── db/ │ │ ├── pool.ts # pg.Pool singleton, sized from KNOTRACK_DB_POOL_MAX -│ │ ├── migrations/ -│ │ │ ├── 1735689600000_init.js -│ │ │ ├── 1735689700000_add-sync-timestamps.js -│ │ │ └── ... # node-pg-migrate CommonJS migration files, timestamp-prefixed │ │ └── queries/ │ │ ├── projects.ts │ │ ├── tracks.ts @@ -85,7 +81,7 @@ knotrack/ │ │ ├── events.ts │ │ ├── decisions.ts │ │ ├── drift-flags.ts -│ │ └── adapter-credentials.ts +│ │ └── adapters.ts │ ├── domain/ │ │ ├── dependency-graph.ts # topo sort + cycle detection, shared by create-track and create-item │ │ ├── drift-detector.ts # the 6 drift flag rules (see Appendix C) @@ -105,8 +101,9 @@ knotrack/ │ ├── unit/ # domain/ and crypto/ logic, no DB │ ├── integration/ # full tool calls against a real Postgres (docker-compose or testcontainers) │ └── fixtures/ +├── migrations/ # plain numbered .sql / .down.sql pairs, applied by scripts/migrate.ts (§1) — not node-pg-migrate ├── scripts/ -│ ├── migrate.ts # runs node-pg-migrate programmatically; invoked at deploy time, not in-process +│ ├── migrate.ts # custom runner: applies migrations/*.sql in order, tracks applied ones in schema_migrations; run via `npm run migrate`, not node-pg-migrate │ └── generate-token.ts # prints a new candidate bearer token for KNOTRACK_API_TOKENS ├── .env.example ├── package.json @@ -121,6 +118,8 @@ knotrack/ └── README.md ``` +**This tree still has known staleness beyond the migrations fix above, not yet swept:** the real `src/mcp/tools/` has 6 files, not 14 — the 9 unimplemented tools listed individually above (`list-tracks.ts`, `get-track.ts`, `get-next-steps.ts`, `record-decision.ts`, `update-item-status.ts`, `check-drift.ts`, `render-roadmap.ts`, `sync-to-github.ts`, `sync-to-linear.ts`) are actually all registered together in one `stubs.ts` file; `src/db/queries/` has no `decisions.ts` yet (nothing writes to `decisions` until `kt_record_decision` is built); and `src/adapters/` doesn't exist yet at all (no code path uses it until `T5`). Tracked in `docs/ROADMAP.md`'s backlog alongside the other stale-mention sweeps. + --- ## 3. Tool Contract Reference @@ -691,7 +690,7 @@ Example operational-failure output (still a successful tool call): ``` Other `error` string prefixes used: `GITHUB_AUTH_FAILED` (401/403 from GitHub — token revoked or insufficient scope), `GITHUB_NOT_FOUND` (repo or issue not found), `GITHUB_TIMEOUT` (exceeded `KNOTRACK_GITHUB_SYNC_TIMEOUT_MS`, default 8000ms), `GITHUB_UNKNOWN_ERROR` (anything else, with the upstream status code appended). -Errors (tool-level, via `isError`): `401`; `404` (project or track not found); `409` (no GitHub credentials configured for this project — i.e. no row in `adapter_credentials` for `(project_id, 'github')`); `422` (malformed uuid); `500` (credential decryption failure, unexpected local exception before the GitHub call was even attempted). +Errors (tool-level, via `isError`): `401`; `404` (project or track not found); `409` (no GitHub credentials configured for this project — i.e. no row in `adapters` for `(project_id, 'github')`); `422` (malformed uuid); `500` (credential decryption failure, unexpected local exception before the GitHub call was even attempted). ### 3.15 `kt_sync_to_linear` @@ -753,18 +752,18 @@ Errors (tool-level, via `isError`): `401`; `404` (project or track not found); ` - **Key:** exactly 32 raw bytes, provided as a base64 string in `KNOTRACK_ENCRYPTION_KEY`. Generate with `openssl rand -base64 32`. Decoded once at boot; the server refuses to start if the decoded length is not exactly 32 bytes. - **Per-secret encryption:** 1. Generate a fresh random 12-byte IV: `crypto.randomBytes(12)` (12 bytes / 96 bits is the AES-GCM-recommended nonce size). - 2. `const cipher = crypto.createCipheriv('aes-256-gcm', key, iv)` - 3. `const ciphertext = Buffer.concat([cipher.update(plaintextUtf8, 'utf8'), cipher.final()])` + 2. `const cipher = crypto.createCipheriv('aes-256-gcm', key, iv, { authTagLength: 16 })` — `authTagLength` is passed explicitly, not left to the Node default, so the paired decrypt call (below) enforces exactly a 16-byte tag rather than silently accepting a shorter one. + 3. `const ciphertext = Buffer.concat([cipher.update(plaintext, 'utf8'), cipher.final()])` 4. `const authTag = cipher.getAuthTag()` (16 bytes) - 5. Persist all three (`ciphertext`, `iv`, `authTag`) plus a `key_version` integer (see rotation, below). + 5. Pack `iv || authTag || ciphertext` into a single buffer and persist it as `adapters.encrypted_credential` (see Storage, below — the real schema has no separate `key_version` column; see the "Known gap" bullet for the rotation implication of that). - **Decryption:** - 1. `const decipher = crypto.createDecipheriv('aes-256-gcm', key, iv)` + 1. `const decipher = crypto.createDecipheriv('aes-256-gcm', key, iv, { authTagLength: 16 })` — same explicit option as encryption, so `setAuthTag` below enforces the full 16-byte tag rather than accepting a truncated one. 2. `decipher.setAuthTag(authTag)` 3. `const plaintext = Buffer.concat([decipher.update(ciphertext), decipher.final()]).toString('utf8')` 4. If `decipher.final()` throws (auth tag mismatch — tampered or corrupted ciphertext, or wrong key), the error is caught, logged server-side with no secret material in the log line, and surfaced to the caller as a generic `500 INTERNAL_ERROR` (never leaking which part of the crypto operation failed). -- **Storage:** table `adapter_credentials` — `project_id`, `adapter_type` (`'github'` | `'linear'`), `ciphertext bytea`, `iv bytea`, `auth_tag bytea`, `key_version int default 1`, unique on `(project_id, adapter_type)`. See Appendix A for full DDL. -- **Never returned:** the `projects.adapters` JSONB column stores **only non-secret metadata** — e.g. `{"github": {"repo": "acme/widgets", "connected": true}}` — and is what every read-path tool (`kt_get_project_status`, `kt_list_tracks`, `kt_get_track`) serializes. The `adapter_credentials` table is read **only** by `src/adapters/github/client.ts` and `src/adapters/linear/client.ts` immediately before making an outbound API call in `kt_sync_to_github`/`kt_sync_to_linear`, and its columns never appear in any tool's output type — there is no code path that could accidentally include them. -- **Known gap — no key rotation path shipped in v1:** the `key_version` column exists specifically to support rotation later, but no rotation script or command ships yet — there is no `scripts/rotate-encryption-key.ts` and no corresponding `package.json` script. If `KNOTRACK_ENCRYPTION_KEY` is compromised today, the only recourse is manual: write and run a one-off script that loads every `adapter_credentials` row, decrypts with the old key, re-encrypts with a new key and fresh IV, writes back, and bumps `key_version`, then update `KNOTRACK_ENCRYPTION_KEY` and redeploy — the same shape a shipped `rotate-encryption-key` command would follow, just not packaged as one. This is tracked as follow-up work, to be built when key rotation is actually needed rather than speculatively now. +- **Storage:** table `adapters` — `project_id`, `type` (`'github'` | `'linear'`), `encrypted_credential bytea` (a single packed blob: `iv (12 bytes) || authTag (16 bytes) || ciphertext`, per `src/crypto/credential-cipher.ts`), `config jsonb` (non-secret metadata only), unique on `(project_id, type)`. See Appendix A. An earlier draft of this section described a dedicated `adapter_credentials` table with separate `ciphertext`/`iv`/`auth_tag`/`key_version` columns — the already-applied migration (`migrations/001_init.sql`) never had that table; it packs all three secret components into the one `encrypted_credential` column instead, and there is no `key_version` column at all. `src/crypto/credential-cipher.ts`'s header comment documents this as a deliberate fit to the real, already-migrated schema, not an oversight. +- **Never returned:** the `adapters.config` column stores **only non-secret metadata** — e.g. `{"owner": "acme", "repo": "widgets"}` for GitHub, `{"team_id": "..."}` for Linear. (There is no `projects.adapters` column; an earlier draft of this section described one, but it was never part of the migrated schema — see `docs/DATABASE_SCHEMA.md`'s `projects` table.) As of this build, `kt_get_project_status`, `kt_list_tracks`, and `kt_get_track` don't yet serialize any adapter data into their responses at all — none of the three currently reads the `adapters` table. If/when they do, they must read only `config`, never `encrypted_credential`. Neither `src/adapters/github/client.ts` nor `src/adapters/linear/client.ts` exists in the repo yet (both are `T5` work, not started) — the intent is for the `encrypted_credential` column to be read **only** by those two files, immediately before making an outbound API call in `kt_sync_to_github`/`kt_sync_to_linear`, never appearing in any tool's output type, but that is a design intent for `T5`, not a verifiable access boundary in the current build. As of this build, the only code that actually reads `encrypted_credential` is `listAdaptersForProject` (`src/db/queries/adapters.ts`), whose `SELECT *` includes the column; it currently has no production caller. +- **Known gap — no key rotation path shipped in v1:** there is no `key_version` column (see the Storage note above) and no rotation script — no `scripts/rotate-encryption-key.ts`, no corresponding `package.json` script. If `KNOTRACK_ENCRYPTION_KEY` is compromised today, the only recourse is manual: write and run a one-off script that loads every `adapters` row, decrypts `encrypted_credential` with the old key, re-encrypts with a new key and fresh IV, writes the repacked blob back, then updates `KNOTRACK_ENCRYPTION_KEY` and redeploys. Without a `key_version` column there's no way to run old and new keys side-by-side mid-rotation — a real rotation implementation needs to add one first. Tracked as follow-up work in `docs/ROADMAP.md`, to be built when key rotation is actually needed rather than speculatively now. --- @@ -889,115 +888,13 @@ Behavior: no arguments, no auth, no DB access — always `200 OK`, computed enti --- -## Appendix A — PostgreSQL Schema (DDL) - -Expressed here as the target schema; in the repository this is built up incrementally across `node-pg-migrate` files under `src/db/migrations/` (plain CommonJS files using the `pgm` builder API — e.g. `pgm.createTable(...)`, `pgm.addConstraint(...)` — which each generate and print the exact SQL they run, keeping migrations both diffable and human-readable without hand-writing raw SQL strings). - -```sql -create extension if not exists pgcrypto; -- only for gen_random_uuid(); credentials themselves never use pgcrypto (see §5) - -create table projects ( - id uuid primary key default gen_random_uuid(), - name text not null, - source_type text not null check (source_type in ('github', 'linear', 'local')), - source_ref text not null, - adapters jsonb not null default '{}'::jsonb, -- non-secret metadata only, see §5 - created_at timestamptz not null default now(), - updated_at timestamptz not null default now(), - unique (source_type, source_ref) -); - -create table adapter_credentials ( - id uuid primary key default gen_random_uuid(), - project_id uuid not null references projects(id) on delete cascade, - adapter_type text not null check (adapter_type in ('github', 'linear')), - ciphertext bytea not null, - iv bytea not null, - auth_tag bytea not null, - key_version int not null default 1, - created_at timestamptz not null default now(), - updated_at timestamptz not null default now(), - unique (project_id, adapter_type) -); - -create table tracks ( - id uuid primary key default gen_random_uuid(), - project_id uuid not null references projects(id) on delete cascade, - title text not null, - status text not null default 'on_track' check (status in ('on_track', 'pivot_pending', 'blocked', 'done')), -- stored, see §3.5; written only by kt_create_track (§3.6) and kt_record_decision (§3.10) - source_doc_ref text, - last_github_sync_at timestamptz, - last_linear_sync_at timestamptz, - created_at timestamptz not null default now() -); -create index on tracks (project_id); - -create table track_dependencies ( - track_id uuid not null references tracks(id) on delete cascade, - depends_on_track_id uuid not null references tracks(id) on delete cascade, - primary key (track_id, depends_on_track_id), - check (track_id <> depends_on_track_id) -); - -create table items ( - id uuid primary key default gen_random_uuid(), - project_id uuid not null references projects(id) on delete cascade, - track_id uuid not null references tracks(id) on delete cascade, - title text not null, - status text not null default 'pending' check (status in ('pending', 'in_progress', 'done', 'blocked')), - sequence_position int not null, - created_at timestamptz not null default now(), - updated_at timestamptz not null default now() -); -create index on items (track_id); -create index on items (project_id); - -create table item_dependencies ( - item_id uuid not null references items(id) on delete cascade, - depends_on_item_id uuid not null references items(id) on delete cascade, - primary key (item_id, depends_on_item_id), - check (item_id <> depends_on_item_id) -); - -create table events ( - id uuid primary key default gen_random_uuid(), - project_id uuid not null references projects(id) on delete cascade, - track_id uuid not null references tracks(id) on delete cascade, - event_type text not null default 'session_summary' check (event_type in ('session_summary')), - summary_text text not null, - files_touched jsonb not null default '[]'::jsonb, - items_touched uuid[] not null default '{}', - created_at timestamptz not null default now() -); -create index on events (track_id, created_at desc); -create index on events (project_id, created_at desc); - -create table decisions ( - id uuid primary key default gen_random_uuid(), - project_id uuid not null references projects(id) on delete cascade, - track_id uuid not null references tracks(id) on delete cascade, - title text not null, - rationale text not null, - what_changed text not null, - created_at timestamptz not null default now() -); -create index on decisions (project_id, created_at desc); - -create table drift_flags ( - id uuid primary key default gen_random_uuid(), - project_id uuid not null references projects(id) on delete cascade, - track_id uuid references tracks(id) on delete cascade, - item_id uuid references items(id) on delete cascade, - flag_type text not null check (flag_type in - ('STALE_TRACK', 'DEPENDENCY_GAP', 'SEQUENCE_SKIP', 'UNDOCUMENTED_DECISION', 'ORPHAN_ITEM', 'SYNC_DRIFT')), - severity text not null check (severity in ('info', 'warning', 'critical')), - detail text not null, - status text not null default 'open' check (status in ('open', 'resolved', 'dismissed')), - raised_at timestamptz not null default now(), - resolved_at timestamptz -); -create index on drift_flags (project_id, status, raised_at desc); -``` +## Appendix A — PostgreSQL Schema + +**The authoritative schema is `migrations/001_init.sql`** (plus `002_projects_unique_source_ref.sql` and `003_drift_flags_open_unique.sql`), applied by the custom runner at `scripts/migrate.ts` (§1). It is not reproduced here. + +An earlier draft of this Appendix carried a full hand-copied DDL block that, by the time this note was written, had drifted from the schema actually built — across nearly every table, not just the adapter-credential shape already called out in §5. Concretely, the old block: named a fictional `adapter_credentials` table instead of the real `adapters` table (§5); described `projects` with a `source_ref` that's actually nullable and a `projects.adapters` column that was never built; put a `project_id` column directly on `items` that doesn't exist (item→project scoping goes through `track_id` only); gave `tracks` two `last_github_sync_at`/`last_linear_sync_at` columns that don't exist anywhere in the real schema (see the Appendix B note on `SYNC_DRIFT`, below); described `drift_flags` with a six-value `flag_type` plus separate `severity` and `status` columns, where the real table has just a two-value `kind` plus `resolved_at` (see `src/db/queries/drift-flags.ts`'s header comment); and omitted the `api_tokens` table and the `set_updated_at` trigger infrastructure entirely. + +Keeping a second, hand-maintained copy of the DDL in this doc is exactly what let that drift accumulate silently — the same lesson `src/crypto/credential-cipher.ts` and `src/db/queries/adapters.ts` already document for the credential-storage piece specifically. This Appendix now points at the single source of truth instead of duplicating it. For the full table reference — columns, constraints, indexes, and the rationale behind each design choice — see `docs/DATABASE_SCHEMA.md`; for how each table maps to a tool's request/response contract, see this document's §3. Note on `items.status`: **item status is stored** and is the terminal write target of `kt_update_item_status`, which can set it to any of the four values. `tracks.status` is also stored (§3.5), but with a narrower set of writers: only `kt_create_track` (initial value) and `kt_record_decision` (→ `pivot_pending`) ever write it — there is no tool analogous to `kt_update_item_status` for tracks. @@ -1012,7 +909,9 @@ Note on `items.status`: **item status is stored** and is the terminal write targ | `SEQUENCE_SKIP` | `info` | An item with `sequence_position = k` and `status = 'done'` exists while another item in the **same track** with `sequence_position < k` has `status` of `pending` or `blocked` — i.e. work finished out of its intended order. Informational, not necessarily wrong. | | `UNDOCUMENTED_DECISION` | `warning` | A `decisions` row exists for a track, and **no** `events` row for that same `track_id` has `created_at` later than the decision's `created_at` — i.e. a decision was logged but no subsequent session summary shows it was acted on. | | `ORPHAN_ITEM` | `warning` | An item's `depends_on_item_id` points to an item belonging to a **different** `track_id` than the item itself. (Should be prevented at write time by `kt_create_item`'s same-track restriction — defensive check only, e.g. for imported/migrated data.) | -| `SYNC_DRIFT` | `warning` | The project has credentials configured for an adapter (a row exists in `adapter_credentials` for `github` and/or `linear`), and the track's `updated_at`-equivalent (most recent item status change or event on that track) is later than its `last_github_sync_at` / `last_linear_sync_at` respectively — i.e. local state has moved since the last successful sync. `last_github_sync_at`/`last_linear_sync_at` are updated only on a successful (`{ok: true}`) `kt_sync_to_github`/`kt_sync_to_linear` call. | +| `SYNC_DRIFT` | `warning` | The project has credentials configured for an adapter (a row exists in `adapters` for `github` and/or `linear`), and the track's `updated_at`-equivalent (most recent item status change or event on that track) is later than its `last_github_sync_at` / `last_linear_sync_at` respectively — i.e. local state has moved since the last successful sync. `last_github_sync_at`/`last_linear_sync_at` are updated only on a successful (`{ok: true}`) `kt_sync_to_github`/`kt_sync_to_linear` call. | + +**`SYNC_DRIFT` is not implementable against the current schema as written.** It depends on `last_github_sync_at`/`last_linear_sync_at` columns that don't exist anywhere in the real schema — not on `tracks`, not on `adapters`, not anywhere (confirmed against `migrations/001_init.sql` and `docs/DATABASE_SCHEMA.md`). This is a genuine gap, not yet a bug, since the only two `kind` values `drift_flags` currently accepts (`out_of_sequence`, `orphan_file_change`) don't include a sync-drift kind either — `SYNC_DRIFT` is entirely unbuilt, tracked for T6. Before it can be built, it needs one of: adding the two `last_*_sync_at` columns (to `tracks`, or to `adapters` scoped by type) via a new migration, or redefining the rule against columns that already exist. That's a schema decision, not something to guess at here — tracked in `docs/ROADMAP.md`'s backlog for a decision before `T6` builds real drift heuristics. ---