diff --git a/README.md b/README.md index 0a9c8e1..122a771 100644 --- a/README.md +++ b/README.md @@ -16,18 +16,15 @@ harness — not just one vendor's tool. ## Status -Pre-release, v0.1.0. 12 of the 14 planned tools are fully implemented; 11 of +Pre-release, v0.1.0. 13 of the 14 planned tools are fully implemented; 12 of those are dogfooded (KnoTrack tracks its own build using itself — see [`scripts/seed-self.ts`](scripts/seed-self.ts) and `docs/ROADMAP.md`'s T1). -`kt_sync_to_github` is implemented as of T5.2 (2026-09-19) — idempotent -one-way sync of a track to a GitHub Issue — and **offline-verified**, but not -yet dogfooded: a real-repo dogfood run is its one remaining gate. The -remaining 2 ship as stub -slices: `kt_check_drift` returns a well-formed empty scan (real drift -heuristics land in T6), and `kt_sync_to_linear` validates its inputs and -the adapter precondition, returning a clear `CONFLICT` ("adapter not -configured") or — once an adapter is configured — a "sync is not available -in this build" error; Linear sync ships in T5.3. +`kt_sync_to_github` (T5.2) and `kt_sync_to_linear` (T5.3) are both +implemented as of 2026-09-19 — idempotent one-way sync of a track to a GitHub +Issue / Linear Issue — and **offline-verified**; `kt_sync_to_github` has also +been dogfooded against a real repo, and `kt_sync_to_linear`'s one remaining +gate is a real-workspace dogfood. The one remaining stub is `kt_check_drift`, +which returns a well-formed empty scan (real drift heuristics land in T6). Every change lands through a mandatory adversarial-review gate before it's considered reviewed: deterministic checks (build, lint, typecheck, unit, @@ -43,22 +40,22 @@ include full diffs and raw model output. ## Tools -| Tool | Status | Purpose | -| --------------------------- | ----------- | ------------------------------------------------------------------------------------------- | -| `kt_register_project` | implemented | Register (or upsert) a project by its source (`github`, `local`, etc.) | -| `kt_get_project_status` | implemented | Current status summary: tracks, items, drift flags | -| `kt_create_track` | implemented | Create a track (a sequenced line of work) under a project | -| `kt_create_item` | implemented | Create an item within a track, auto- or explicitly-sequenced | -| `kt_record_session_summary` | implemented | Record a session's summary and re-check for drift | -| `kt_list_tracks` | implemented | List a project's tracks, optionally filtered by status | -| `kt_get_track` | implemented | Track detail: items plus dependency graph | -| `kt_get_next_steps` | implemented | Suggested next items given current status and dependencies | -| `kt_record_decision` | implemented | Record a decision and the context behind it | -| `kt_update_item_status` | implemented | Move an item's status forward (or flag it blocked) | -| `kt_check_drift` | stub | On-demand drift scan (T2.11 stub: empty scan until heuristics ship in T6) | -| `kt_render_roadmap` | implemented | Render a roadmap view from tracked items | -| `kt_sync_to_github` | implemented | One-way idempotent sync of a track to a GitHub Issue (T5.2; real-repo dogfood gate pending) | -| `kt_sync_to_linear` | stub | One-way sync to Linear (T2.14 stub: adapter precondition only; sync ships in T5.3) | +| Tool | Status | Purpose | +| --------------------------- | ----------- | ------------------------------------------------------------------------------------------------ | +| `kt_register_project` | implemented | Register (or upsert) a project by its source (`github`, `local`, etc.) | +| `kt_get_project_status` | implemented | Current status summary: tracks, items, drift flags | +| `kt_create_track` | implemented | Create a track (a sequenced line of work) under a project | +| `kt_create_item` | implemented | Create an item within a track, auto- or explicitly-sequenced | +| `kt_record_session_summary` | implemented | Record a session's summary and re-check for drift | +| `kt_list_tracks` | implemented | List a project's tracks, optionally filtered by status | +| `kt_get_track` | implemented | Track detail: items plus dependency graph | +| `kt_get_next_steps` | implemented | Suggested next items given current status and dependencies | +| `kt_record_decision` | implemented | Record a decision and the context behind it | +| `kt_update_item_status` | implemented | Move an item's status forward (or flag it blocked) | +| `kt_check_drift` | stub | On-demand drift scan (T2.11 stub: empty scan until heuristics ship in T6) | +| `kt_render_roadmap` | implemented | Render a roadmap view from tracked items | +| `kt_sync_to_github` | implemented | One-way idempotent sync of a track to a GitHub Issue (T5.2; dogfooded against a real repo) | +| `kt_sync_to_linear` | implemented | One-way idempotent sync of a track to a Linear Issue (T5.3; real-workspace dogfood gate pending) | Full request/response contracts for every tool, implemented or planned, are in [`docs/TRD.md`](docs/TRD.md). diff --git a/docs/PRD.md b/docs/PRD.md index 067b37b..99065b6 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -130,8 +130,8 @@ Found KnoTrack on GitHub, is not a KnoTrack contributor, and just wants to run i |---|---|---|---| | `name` | string, 1–200 chars | yes | Display name. Not required to be unique — uniqueness is on `(source_type, source_ref)`, not `name` (see Business rules). | | `source_type` | enum: `"github" \| "linear" \| "local"` | yes | What kind of source `source_ref` identifies. | -| `source_ref` | string, 1–500 chars | yes | Repo URL, Linear project ID, or local filesystem path, depending on `source_type`. | -| `adapters` | object `{ github?, linear? }` | no | Per-adapter credentials, supplied directly in the call (not read from server env vars). `github: { personal_access_token, repo? }`; `linear: { api_key, team_id }`. | +| `source_ref` | string, 1–500 chars | yes | GitHub `owner/repo` (e.g. `acme/widgets`), Linear project ID, or local filesystem path, depending on `source_type`. Stored and used **unchanged** — for GitHub the adapter passes it straight to `/repos/{source_ref}/issues`, so a repo URL is invalid; and because the value is part of the `(source_type, source_ref)` upsert key, a URL and an `owner/repo` form register as different projects. For Linear this is the project identity; the team that issues sync into is `adapters.linear.team_id` (separate adapter metadata). | +| `adapters` | object `{ github?, linear? }` | no | Per-adapter credentials, supplied directly in the call (not read from server env vars). `github: { personal_access_token, repo? }`; `linear: { api_key, team_id, done_state_id?, open_state_id? }` (the two optional workflow-state overrides are T5.3 — see §4.14). | **Output:** `{ project_id }` @@ -199,7 +199,7 @@ Found KnoTrack on GitHub, is not a KnoTrack contributor, and just wants to run i **Output:** ``` { - track: { track_id, title, status, pivot_decision_id, source_doc_ref, depends_on_track_ids, created_at }, + track: { track_id, title, status, pivot_decision_id, source_doc_ref, depends_on_track_ids, github_issue_url, linear_issue_url, created_at }, items: [ { item_id, title, status, sequence_position, depends_on_item_ids } ], dependency_graph: { nodes: [ { item_id, title, status } ], @@ -210,6 +210,7 @@ Found KnoTrack on GitHub, is not a KnoTrack contributor, and just wants to run i **Business rules / edge cases:** - `track_id` must belong to `project_id`; if it belongs to a different project or doesn't exist, `NOT_FOUND`. +- `github_issue_url` / `linear_issue_url` are the URLs of the linked GitHub / Linear Issue this track is synced to (§4.13 / §4.14), or `null` when the track has no linked issue of that type. This is where "the sync tool records the issue URL on the track" is observable through the read contract. - **Track status is derived at read time, not a stored column (T2.16).** There is no `tracks.status` column. Every read of a track's status — this tool, `kt_list_tracks`, `kt_get_project_status`, `kt_render_roadmap` — selects from the `track_readiness` view, which computes `status` in this fixed order: an active pivot (`tracks.pivot_decision_id IS NOT NULL`) → `pivot_pending`; else any **direct** dependency not locally OK (that dependency's own `own_done` true and no active pivot — never that dependency's own `status`) → `blocked`; else the track itself `own_done` (≥1 item, all done-equivalent) → `done`; else `on_track`. No tool writes a track status value directly, and there is no tool analogous to `kt_update_item_status` for tracks. **This closes the gap this document used to flag as an open product problem (PR-review Finding 2): `kt_record_decision`'s `effect: "resolve_pivot"` (§4.9) is exactly the write path that moves a track back out of `pivot_pending`.** `blocked` still has no direct unblock tool — it clears automatically, at read time, once every direct dependency becomes locally OK, since it was never a stored value to unstick in the first place. - Because `status` only looks one hop deep, it can miss a problem further up the dependency chain: a track can read `status: "done"` while a dependency two-plus hops away is still broken. The separate `effective_done` boolean (`own_done` AND no active pivot AND every **transitive** dependency is `own_done` with no active pivot) is the safe-to-build-on signal — `own_done: true, effective_done: false` is a real, surfaced gap, not a bug. This tool's own output does not include `effective_done` (only `status` and `pivot_decision_id`); `effective_done` is what `kt_render_roadmap` (§4.13) and `kt_get_next_steps` (§4.5) key off instead, precisely because `status` alone can't see past one hop. An empty track (zero items) is never `done`. - `pivot_decision_id` (nullable) points at the `decisions` row that opened the currently-active pivot, or is `null` if there is none. Set and cleared only by `kt_record_decision` (§4.9) via its `effect` parameter — a plain `"note"` decision never touches it. @@ -396,7 +397,7 @@ There is no `advisory_notice` field echoed on every response and no `blocking_su | `SEQUENCE_SKIP` | `info` | An item at `sequence_position = k` is `done` while another item in the *same track* at a lower `sequence_position` is `pending` or `blocked` — work finished out of its declared order. Informational, not necessarily wrong. | | `UNDOCUMENTED_DECISION` | `warning` | A Decision exists for a track, and no Event for that same track has a `created_at` later than the Decision's — a pivot was logged but nothing since shows it was acted on (§4.9). | | `ORPHAN_ITEM` | `warning` | An item's `depends_on_item_id` points to an item in a *different* track. Defensive only — `kt_create_item` (§4.7) already restricts this at write time. | -| `SYNC_DRIFT` | `warning` | The project has credentials configured for an adapter, and a track's most recent activity is later than that adapter's last successful sync of it. **Schema exists (`tracks.last_github_sync_at`/`last_linear_sync_at`); the `SYNC_DRIFT` rule itself and the `kt_check_drift` scan are not yet built (T6).** `kt_sync_to_github` (§4.13) is implemented (T5.2) and writes `last_github_sync_at` on each successful sync; the Linear sync (§4.14) is still a stub (T5.3). | +| `SYNC_DRIFT` | `warning` | The project has credentials configured for an adapter, and a track's most recent activity is later than that adapter's last successful sync of it. **Schema exists (`tracks.last_github_sync_at`/`last_linear_sync_at`); the `SYNC_DRIFT` rule itself and the `kt_check_drift` scan are not yet built (T6).** `kt_sync_to_github` (§4.13, T5.2) and `kt_sync_to_linear` (§4.14, T5.3) are both implemented and write `last_github_sync_at` / `last_linear_sync_at` respectively on each successful sync. | **Business rules / edge cases:** - Exceeding the scan's track/item caps or its wall-clock time budget (all operator-configured via environment variables — `docs/TRD.md` §6.3/§7) is **not** an error: the call returns whatever was scanned so far with `truncated: true`, `scanned_track_count`, and `total_track_count`, so the caller can see exactly how partial the result is. @@ -448,18 +449,20 @@ There is no `advisory_notice` field echoed on every response and no `blocking_su ### 4.14 `kt_sync_to_linear` -**Description:** Identical shape and semantics to `kt_sync_to_github` (§4.13), mirrored for Linear. Also registered with its real input schema; in this build it ships the same T2.14 stub slice — project/track validation plus the missing-adapter `CONFLICT` precondition — with the operational Linear API path deferred to T5.3. +**Description:** Identical shape and semantics to `kt_sync_to_github` (§4.13), mirrored for Linear. As of T5.3 it ships the full operational Linear sync path — create/update a linked Linear Issue for a track, idempotently, over the Linear GraphQL API — with the same durable creation-intent and hidden-marker crash recovery as §4.13. The one Linear-specific difference is workflow state: Linear has no open/closed, so a done track's issue is moved to a `completed` workflow state (the team's lowest-position `completed` state, or an optional configured `done_state_id`), and a non-done track's issue is never auto-moved backward unless a valid `open_state_id` is configured (both optional fields on the Linear adapter config). **Inputs:** `project_id` (required), `track_id` (required). -**Output:** `{ ok: true }` on success; `{ ok: false, error: ": " }` on an operational failure. Prefixes: `LINEAR_AUTH_FAILED`, `LINEAR_NOT_FOUND`, `LINEAR_TIMEOUT` (exceeds `KNOTRACK_LINEAR_SYNC_TIMEOUT_MS`, default 8000ms), `LINEAR_UNKNOWN_ERROR`. +**Output:** `{ ok: true }` on success; `{ ok: false, error: ": " }` on an operational failure. Prefixes: `LINEAR_AUTH_FAILED`, `LINEAR_NOT_FOUND`, `LINEAR_RATE_LIMITED`, `LINEAR_TIMEOUT` (exceeds `KNOTRACK_LINEAR_SYNC_TIMEOUT_MS`, default 8000ms), `LINEAR_STATE_CONFIG` (an invalid `done_state_id`/`open_state_id`, or a done track whose team has no `completed` workflow state and no `done_state_id` configured), `LINEAR_UNKNOWN_ERROR`. **Business rules / edge cases (mirrors §4.13 exactly, Linear-specific specifics only):** -- `CONFLICT` for missing Linear credentials (no row in `adapters` for `(project_id, "linear")`); everything about the Linear API call itself is a non-error `{ok: false, ...}` result. +- `CONFLICT` for missing Linear credentials (no row in `adapters` for `(project_id, "linear")`) or a Linear adapter with no `team_id`; everything about the Linear API call itself is a non-error `{ok: false, ...}` result. +- Workflow-state mapping: a done track's Issue is moved to a `completed` state (the configured `done_state_id`, else the team's lowest-position `completed` state); an invalid override, or a done track whose team has no `completed` state and no `done_state_id`, returns `{ok:false, error:"LINEAR_STATE_CONFIG: ..."}`. A non-done track's Issue is never moved backward unless a valid `open_state_id` is configured and (on update) the Issue is currently in a `completed`/`canceled` state. **Acceptance criteria:** - **Given** a project without Linear credentials configured, **when** `kt_sync_to_linear` is called, **then** the call fails with `CONFLICT`. -- **Given** this build, **when** `kt_sync_to_linear` is called for an existing project and track with no Linear adapter configured, **then** the missing-adapter `CONFLICT` (§4.13's shape, Linear-specific) is the shipped behavior and no HTTP call is made; the operational Linear API path is not yet implemented (T5.3). An adapter configured via `kt_register_project` (which provisions adapters from inline credentials) yields an `INTERNAL_ERROR` (`linear sync is not available in this build`), never a false success. +- **Given** a configured Linear adapter, **when** `kt_sync_to_linear` is called for a track with no existing link, **then** a Linear Issue is created, the link is recorded, and `kt_get_track`'s `track.linear_issue_url` returns the Issue URL; a second call with unchanged content is a no-op and a call after the track changes updates the same Issue. A done track's Issue is set to a `completed` workflow state. +- **Given** a `pending` link left by a crashed prior create, **when** `kt_sync_to_linear` is called again, **then** it recovers via the hidden marker (adopting the existing Issue if found) rather than creating a duplicate. *Verified offline (fake `LinearClient`, no network in CI); the real-workspace run is the one remaining acceptance gate (docs/ROADMAP.md T5.3).* --- @@ -524,7 +527,7 @@ Because KnoTrack collects no central telemetry (§5.6), every metric below is so 2. **Drift trend, not drift count.** The ratio of `SEQUENCE_SKIP` flags (§4.11) that get resolved, or simply stop recurring as work catches up to its declared order, versus flags that persist open across repeated checks. A tool that's working sees this trend toward resolved, not toward a silently accumulating pile of open flags nobody looks at. 3. **`kt_get_next_steps` actually consulted before work starts.** Correlating ordinary HTTP/application access logs for `kt_get_next_steps` calls against the timestamps of subsequent Item status changes and Events — if items are consistently touched shortly after a next-steps call recommended them, the advisory loop is being used as intended. Note this is derived from access logs, not from the Event table, because `kt_get_next_steps` is deliberately side-effect-free (§4.5) and creates no Event of its own. 4. **Rendered-roadmap freshness, if the caller chooses to persist it.** `kt_render_roadmap` (§4.12) has no notion of a target file — it returns rendered content as a string — so freshness is only measurable if and however the calling agent chooses to persist that string (e.g. committing it into the repo as `ROADMAP.md`). Where an installer adopts that convention, the time elapsed between a DB state change and the next commit of the rendered content is still a meaningful trust signal for the snapshot; where they don't, this metric simply doesn't apply to that installation. -5. **Adapter usage where configured.** For a project with GitHub or Linear credentials configured (a row exists in `adapters` for that project/type — §5.3), the frequency of successful sync calls relative to manual, un-synced Item creation — if a project has an adapter configured but the sync tools are never exercised (for a Linear adapter, note §4.14 is still a stub in this build), that's a signal the adapter isn't earning its complexity for that installer. +5. **Adapter usage where configured.** For a project with GitHub or Linear credentials configured (a row exists in `adapters` for that project/type — §5.3), the frequency of successful sync calls relative to manual, un-synced Item creation — if a project has an adapter configured but the sync tools are never exercised, that's a signal the adapter isn't earning its complexity for that installer. 6. **Time-to-first-registered-project per deploy path (maintainer-run, manual QA).** Not automated telemetry — a manual checklist the maintainers run themselves against each of the three deploy targets in §5.5, timing from `git clone` to a successful first `kt_register_project` call. Target: under 30 minutes on each path, re-verified whenever a deploy guide or dependency changes. As of 2026-09-07 this has only actually been run for Railway (well under target — see `docs/ROADMAP.md` T3/T4); Render+Supabase and Fly.io have a draft runbook (`docs/deploy/client-verification-runbook.md`) but it hasn't actually been run end-to-end yet, so this metric doesn't apply to them until `T7.1`/`T7.2` close. --- @@ -534,7 +537,7 @@ Because KnoTrack collects no central telemetry (§5.6), every metric below is so - **Work dispatch or orchestration of any kind.** No tool in this document assigns, triggers, queues, or executes work against any agent, CI system, or external runner. `kt_get_next_steps` is advisory only (§4.5); this boundary is treated as permanent product identity, not a temporary v1 gap — see §2.2. - **Team/multi-user auth beyond a single shared bearer-token pool.** There is no role-based access control, no SSO/OAuth login flow, no per-device or per-user token identity, and no concept of a restricted or read-only token in v1 — every valid token accepted by an instance (§5.3) has full read/write access to every project it hosts, and the server has no way to tell which configured token a given call used. A small team (persona in §3.2) shares one instance and, in practice, one token per developer drawn from the same pool, all with equal privileges. Formal per-user roles and scoped/read-only tokens are a natural v2 addition once real multi-user usage shows which restrictions are actually wanted. - **Real-time push UI.** `kt_get_project_status` and all other read tools are pull/poll-based only. There is no WebSocket/SSE channel, no live-updating dashboard, and no in-app or external notification (Slack, email, etc.) fired when drift is detected — a human or agent must proactively call a status/drift tool to find out. A future web UI, if built, would poll these same MCP tools rather than requiring a new push mechanism. -- **Automatic conflict resolution for adapter sync, and bidirectional/bulk sync generally.** `kt_sync_to_github` (§4.13) and `kt_sync_to_linear` (§4.14 — the Linear adapter is still a stub in this build) are each a single, scoped, one-way push per call, with no bulk import, no direction choice, and no merge logic for the case where the same piece of work has diverged independently on both sides. +- **Automatic conflict resolution for adapter sync, and bidirectional/bulk sync generally.** `kt_sync_to_github` (§4.13) and `kt_sync_to_linear` (§4.14) are each a single, scoped, one-way push per call, with no bulk import, no direction choice, and no merge logic for the case where the same piece of work has diverged independently on both sides. - **Automatic parsing of arbitrary local roadmap/spec file formats, and importing existing GitHub/Linear issues.** As stated in §1.3, KnoTrack does not itself ingest free-text planning documents from a local folder; the calling agent reads them and populates Tracks/Items via `kt_create_track`/`kt_create_item`. The GitHub and Linear adapters are one-way pushes of existing KnoTrack state outward, not an import path — there is no tool in v1 that pulls GitHub Issues or Linear Issues into KnoTrack Items. - **Centralized, maintainer-run hosting or analytics.** There is no multi-tenant SaaS version of KnoTrack, and no telemetry collection of any kind (§5.6). "Open source, self-hosted" is the whole distribution model for v1, not a stepping stone the PRD assumes will change. - **A standalone chat or web-app interface.** KnoTrack in v1 is purely an MCP tool surface. Whatever conversational interface a user experiences is provided entirely by the calling agent harness (Claude Code, Windsurf, etc.), not by KnoTrack itself. @@ -544,13 +547,13 @@ Because KnoTrack collects no central telemetry (§5.6), every metric below is so ## 8. Glossary -- **Project** — The top-level entity representing one software project KnoTrack has been pointed at. Identified by a `source_type` (`github`, `linear`, or `local`) and a `source_ref` whose meaning depends on `source_type` (a repo, a Linear team, or a local filesystem path) — not by a `root_path`/`repo_url` pair. All other entities belong to exactly one Project. +- **Project** — The top-level entity representing one software project KnoTrack has been pointed at. Identified by a `source_type` (`github`, `linear`, or `local`) and a `source_ref` whose meaning depends on `source_type` (a repo, a Linear project, or a local filesystem path) — not by a `root_path`/`repo_url` pair. All other entities belong to exactly one Project. - **Track** — A grouping of related work within a Project (roughly: an epic or workstream), with a derived `status` of `on_track`, `pivot_pending`, `blocked`, or `done` (T2.16 — computed at read time from the track's own item completion, its direct dependencies' local state, and whether it has an active pivot; there is no `tracks.status` column), and optional declared dependencies on other Tracks. No tool writes `status` directly; `kt_record_decision`'s `effect: "open_pivot"`/`"resolve_pivot"` is the only thing that moves a track into or out of `pivot_pending`, and `blocked` clears itself automatically once every direct dependency becomes locally OK (§4.4, §4.9). - **Item** — A single, discrete piece of work inside a Track. Has a `sequence_position` (its declared order within the Track), a stored `status` (`pending`, `in_progress`, `blocked`, `done` — written only by `kt_update_item_status`), and optional dependencies on other Items, restricted in v1 to items within the same Track. There is no `file_patterns` field on Items in this build. - **Event** — An append-only log entry created by `kt_record_session_summary`, recording what happened in a session: a `summary_text`, the files touched, and any items touched. Events carry no per-caller identity (§5.3) and no self-reported drift opinion — drift is structural only. Events are never edited or deleted once written; recording one also re-runs the drift-detector's rules scoped to that Event's track. - **Decision** — An explicit, append-only record of an intentional pivot or plan change, scoped to one Track: a `title`, a `rationale` (why), a `what_changed` description (what concretely changed), and an `effect` (`note` | `open_pivot` | `resolve_pivot`, T2.16 — default `note`, which has no lifecycle side effect at all). Decisions are never inferred from a status change or a boolean flag — they only exist because `kt_record_decision` was deliberately called with real content. Only `open_pivot`/`resolve_pivot` move a Track into or out of `pivot_pending` (§4.9); a Decision never directly suppresses any drift flag, though it changes the input to the `UNDOCUMENTED_DECISION` flag specifically. - **Drift** — A structurally-computed mismatch between the declared plan and what actually happened, never from a self-reported opinion. Evaluated against a six-type flag catalog (`STALE_TRACK`, `DEPENDENCY_GAP`, `SEQUENCE_SKIP`, `UNDOCUMENTED_DECISION`, `ORPHAN_ITEM`, `SYNC_DRIFT` — full definitions in §4.11); as of this build, only `SEQUENCE_SKIP` is actually raised anywhere, via `kt_record_session_summary`'s scoped per-track re-check, since the full-project `kt_check_drift` scan is not yet implemented. -- **Adapter** — A per-project integration to an external source-of-truth system (GitHub or Linear) that provides one-way, per-track push sync, gated on the server holding the relevant credential for that project (a row in the `adapters` table) — not a separate "enable this adapter" flag. Adapter credentials are always server-side only, never passed through any MCP tool's output. `kt_sync_to_github` (§4.13) is implemented (T5.2 — a real one-way, per-track push that creates/updates the linked Issue idempotently); `kt_sync_to_linear` (§4.14) still ships only its precondition stub (project/track validation plus a `CONFLICT` when no adapter is configured), with its outward push deferred to T5.3. +- **Adapter** — A per-project integration to an external source-of-truth system (GitHub or Linear) that provides one-way, per-track push sync, gated on the server holding the relevant credential for that project (a row in the `adapters` table) — not a separate "enable this adapter" flag. Adapter credentials are always server-side only, never passed through any MCP tool's output. `kt_sync_to_github` (§4.13, T5.2) and `kt_sync_to_linear` (§4.14, T5.3) are both implemented — each a real one-way, per-track push that creates or updates the linked Issue idempotently. - **Advisory** — The general operating principle behind `kt_get_next_steps` and, in effect, every other KnoTrack tool: the system recommends, tracks, and reports, but never assigns, dispatches, or blocks a human/agent's actual actions. --- @@ -567,7 +570,7 @@ Track- and item-level dependencies are stored as **join tables** (`track_depende **Adapter** `id (uuid pk)`, `project_id (uuid fk)`, `type (text — "github" | "linear")`, `encrypted_credential (bytea — a packed iv(12 bytes) || authTag(16 bytes) || ciphertext blob, §5.3)`, `config (jsonb — non-secret metadata only, e.g. {"owner": "acme", "repo": "widgets"})`, `key_version (integer, default 1 — which generation of `KNOTRACK_ENCRYPTION_KEY` encrypted this row; bumped by the key-rotation process, §5.3)`, `created_at`. Unique on `(project_id, type)` — at most one adapter row per project per type. -**Track** — **there is no `status` column (T2.16).** `id (uuid pk)`, `project_id (uuid fk)`, `title (text)`, `pivot_decision_id (uuid, nullable, composite fk -> decisions(id, track_id, effect) — NULL means no active pivot, §4.4/§4.9)`, `pivot_effect (text, GENERATED ALWAYS AS ('open_pivot') STORED — migration 008)` — exists only so `pivot_decision_id`'s FK can be a 3-column composite that also pins the referenced decision's `effect` to `open_pivot`, not just its identity/track, `source_doc_ref (text, nullable)`, `last_github_sync_at (timestamptz, nullable)`, `last_linear_sync_at (timestamptz, nullable — both added for the SYNC_DRIFT rule, §4.11; `last_github_sync_at` is now written by `kt_sync_to_github` on each successful sync (T5.2), stamped to the payload snapshot time; `last_linear_sync_at` is not yet written, pending the Linear sync (T5.3))`, `created_at`, `updated_at`. `status` (`on_track|pivot_pending|blocked|done`) is computed at read time by the `track_readiness` database view from `pivot_decision_id`, the track's own item completion, and its direct dependencies' state — no column to write, so no tool writes it directly. Track-level dependencies: a `track_dependencies (track_id, depends_on_track_id, project_id, created_at)` join table, no self-dependency allowed; `project_id` (migration 008, backfilled from `tracks.project_id`, NOT NULL) is enforced by two composite FKs to `tracks(id, project_id)` so both sides of an edge are schema-guaranteed to belong to the same project — closing a gap that was previously only enforced at the application layer (`kt_create_track`). There is no `description` column. Also unique on `(id, project_id)` (migration 008, backing those composite FKs). +**Track** — **there is no `status` column (T2.16).** `id (uuid pk)`, `project_id (uuid fk)`, `title (text)`, `pivot_decision_id (uuid, nullable, composite fk -> decisions(id, track_id, effect) — NULL means no active pivot, §4.4/§4.9)`, `pivot_effect (text, GENERATED ALWAYS AS ('open_pivot') STORED — migration 008)` — exists only so `pivot_decision_id`'s FK can be a 3-column composite that also pins the referenced decision's `effect` to `open_pivot`, not just its identity/track, `source_doc_ref (text, nullable)`, `last_github_sync_at (timestamptz, nullable)`, `last_linear_sync_at (timestamptz, nullable — both added for the SYNC_DRIFT rule, §4.11; `last_github_sync_at` is now written by `kt_sync_to_github` on each successful sync (T5.2) and `last_linear_sync_at` by `kt_sync_to_linear` (T5.3), each stamped to the payload snapshot time)`, `created_at`, `updated_at`. `status` (`on_track|pivot_pending|blocked|done`) is computed at read time by the `track_readiness` database view from `pivot_decision_id`, the track's own item completion, and its direct dependencies' state — no column to write, so no tool writes it directly. Track-level dependencies: a `track_dependencies (track_id, depends_on_track_id, project_id, created_at)` join table, no self-dependency allowed; `project_id` (migration 008, backfilled from `tracks.project_id`, NOT NULL) is enforced by two composite FKs to `tracks(id, project_id)` so both sides of an edge are schema-guaranteed to belong to the same project — closing a gap that was previously only enforced at the application layer (`kt_create_track`). There is no `description` column. Also unique on `(id, project_id)` (migration 008, backing those composite FKs). **Item** `id (uuid pk)`, `track_id (uuid fk)`, `title (text)`, `sequence_position (integer, not unique per track_id — see §4.7's shift-on-insert behavior)`, `status (text, CHECK-constrained: pending|in_progress|done|blocked, default pending — written only by kt_update_item_status, §4.10)`, `created_at`, `updated_at`. Item-level dependencies: an `item_dependencies (item_id, depends_on_item_id, created_at)` join table restricted at the application layer (§4.7) to pairs within the same track. There is no `description`, `file_patterns`, or `external_ref` column. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 59085dc..ecb421c 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -952,9 +952,10 @@ this Track. **Status:** `on_track` (corrected 2026-09-19 — the two blockers named below are both cleared: `T4` went `done` 2026-09-07 and `T2.16` shipped 2026-09-08, so the stale `blocked` no longer holds). `T5.1` is **done + -verified 2026-09-19** and `T5.2` is **implemented + offline-verified -2026-09-19** (real-repo dogfood gate pending — see those items); `T5.3` / -`T5.4` remain open. +verified 2026-09-19** and `T5.2` is **implemented + dogfooded against a real repo +2026-09-19** (Issue #24); `T5.3` is +**implemented + offline-verified 2026-09-19** (real-workspace dogfood gate +pending); `T5.4` remains open. **depends_on:** `T4`, `T2.16` (added 2026-08-29 — `SYNC_DRIFT`'s notion of "the track's most recent change" and any future logic that reads track completion to decide what to push should be built against the @@ -980,8 +981,8 @@ corrected track-status model, not the currently-broken one). "stored value is ciphertext with a correct decrypt round-trip" proof at the DB layer). No code change was needed; this item was already satisfied by the `T2.15` register-project work and is now confirmed. -2. **T5.2 — `kt_sync_to_github` implemented (offline-verified 2026-09-19; - real-repo dogfood gate pending).** Acceptance: given a stored encrypted +2. **T5.2 — `kt_sync_to_github` implemented + dogfooded against a real repo + (2026-09-19, Issue #24).** Acceptance: given a stored encrypted GitHub credential, calling `kt_sync_to_github` creates/updates a linked GitHub Issue for a **track** (per `docs/PRD.md` §4.13 — the tool is track-scoped, `{project_id, track_id}`, with **no @@ -1017,15 +1018,37 @@ corrected track-status model, not the currently-broken one). self-corrects on the next sync (one redundant PATCH). Revisit only if real concurrent-update contention is observed; a future option is a per-(track,adapter) advisory lock scoped to just the finalize, or an - `operation_id` compare-and-set on the hash write. The **one - remaining acceptance gate** is the real-repo verification against - `SathiaAI/KnoTrack` (dogfood) with a Paul-approved fine-grained PAT — not - part of the code/CI deliverable; see the handoff. `migrations/009_track_external_links.sql`. + `operation_id` compare-and-set on the hash write. The real-repo acceptance + gate is **complete**: dogfooded against `SathiaAI/KnoTrack` on 2026-09-19 + with a Paul-approved fine-grained PAT (Issue #24). `migrations/009_track_external_links.sql`. 3. **T5.3 — `kt_sync_to_linear` fully implemented.** Acceptance: given a stored encrypted Linear credential, calling `kt_sync_to_linear` - creates/updates a linked Linear issue for a KnoTrack item and records - the issue URL on the item, verified against one real test Linear - workspace. depends_on: `T2.13`, `T5.1`. + creates/updates a linked Linear Issue for a **track** (track-scoped + `{project_id, track_id}`, mirroring A4.13/T5.2 — no item-level sync + target in v1) and records the issue URL on the track (surfaced on + `kt_get_track`'s `track.linear_issue_url`), verified against one real + test Linear workspace. depends_on: `T2.13`, `T5.1`. + **Status (2026-09-19): implemented + offline-verified.** Mirrors the + T5.2 GitHub design — persisted `track_external_links` link as the + authoritative idempotency key, durable `pending` creation-intent before + the outbound GraphQL mutation, hidden-marker crash recovery that adopts + but never auto-recreates, injectable `LinearClient` (no network in CI), + and no new migration (the T5.2 table and `tracks.last_linear_sync_at` + already generalize). Linear-specific design (frontier panel 2026-09-19, + unanimous `auto_lookup_with_config_override`): a done track's issue is + moved to a `completed` workflow state — the team's lowest-position one, + or the optional `done_state_id` override — while a non-done track's issue + is never auto-moved backward unless a valid `open_state_id` is + configured (two optional fields added to the Linear adapter config). + Operational failures return `{ok:false, error}` with fixed prefixes + (`LINEAR_AUTH_FAILED` / `LINEAR_NOT_FOUND` / `LINEAR_RATE_LIMITED` / + `LINEAR_TIMEOUT` / `LINEAR_STATE_CONFIG` / `LINEAR_UNKNOWN_ERROR`). + Same deliberate v1 bounds as T5.2 (no retries/backoff; no update-path + lock; no auto-recreate on a stale link). Covered by offline unit + + integration tests (`tests/unit/linear-payload.test.ts`, + `tests/unit/linear-client.test.ts`, + `tests/integration/sync-to-linear.test.ts`). The one remaining + acceptance gate is the real-workspace dogfood. 4. **T5.4 — Credential revocation path implemented + unit-tested.** Acceptance: deleting a stored GitHub/Linear credential causes the next sync call to fail with a clear "credential not configured" error diff --git a/docs/TEST_CASES.md b/docs/TEST_CASES.md index 4bfedb4..6d5d5c1 100644 --- a/docs/TEST_CASES.md +++ b/docs/TEST_CASES.md @@ -447,7 +447,7 @@ section tests the *detection logic itself*, which is triggered both inline by `kt_sync_to_linear(project_id, track_id) -> {ok} | {ok:false, error}` — only valid if a linear adapter is configured (a missing adapter is a `CONFLICT` error, not `{ok:false}`). Mirror of §15. -**This build (T2.14 stub):** a _configured_ linear adapter (provisioned via `kt_register_project`) currently returns `INTERNAL_ERROR` (`linear sync is not available in this build`) — the success (`LNSY-01`) and operational-failure (`LNSY-04`/`LNSY-05`) rows below describe the T5.3 behavior once external sync ships; the missing-adapter `CONFLICT` rows are current. +**This build (T5.3, 2026-09-19):** external Linear sync is implemented — a configured linear adapter creates or updates a Linear Issue (`LNSY-01`), operational Linear failures return `{ok:false, error}` (`LNSY-04`/`LNSY-05`), and the missing-adapter `CONFLICT` rows hold. Verified offline (fake `LinearClient`, no network in CI); a real-workspace dogfood is the one remaining gate. | Test ID | Tool/Area | Type | Preconditions | Input | Expected Result | |---|---|---|---|---|---| diff --git a/docs/TRD.md b/docs/TRD.md index dc75a8e..2863a5c 100644 --- a/docs/TRD.md +++ b/docs/TRD.md @@ -117,7 +117,7 @@ knotrack/ **This tree describes the target layout, not yet the current one on two fronts (updated 2026-09-07 — see `docs/ROADMAP.md`'s T2/T3 status for the underlying build state):** -Tool files — `record-decision.ts` and `update-item-status.ts` are now real, dedicated files too (T2 build-out, second slice, 2026-08-26), joining `list-tracks.ts`, `get-track.ts`, `get-next-steps.ts`, and `render-roadmap.ts` from the first slice. All 14 tools listed above are real files: `check-drift.ts`, `sync-to-github.ts`, and `sync-to-linear.ts` each got their own file (2026-09-18, PR #22, with the two sync tools sharing `sync-shared.ts`), and the old `stubs.ts` was removed. `src/db/queries/decisions.ts` now exists (shipped alongside `record-decision.ts`). `src/adapters/` still doesn't exist at all — no code path uses it until `T5` starts. +Tool files — `record-decision.ts` and `update-item-status.ts` are now real, dedicated files too (T2 build-out, second slice, 2026-08-26), joining `list-tracks.ts`, `get-track.ts`, `get-next-steps.ts`, and `render-roadmap.ts` from the first slice. All 14 tools listed above are real files: `check-drift.ts`, `sync-to-github.ts`, and `sync-to-linear.ts` each got their own file (2026-09-18, PR #22), and the old `stubs.ts` was removed. `src/db/queries/decisions.ts` now exists (shipped alongside `record-decision.ts`). The T5 sync work has since shipped: the real adapter clients live in `src/github/github-client.ts` (T5.2) and `src/linear/linear-client.ts` (T5.3), and the interim `sync-shared.ts` stub helper was removed once both sync tools shipped their real implementations. `src/adapters/` was never created. Deploy config — `render.yaml`, `railway.toml`, and `fly.toml` do **not** exist in this repo and never have; only `Dockerfile` does. Railway is configured directly through its own dashboard/API rather than a committed `railway.toml` (see `docs/deploy/railway.md`), and this is the only one of the three deploy targets that has actually been built, deployed, and verified (`docs/ROADMAP.md` T3, done as of 2026-09-05) — Render+Supabase and Fly.io are undeployed, unverified target designs (`docs/ROADMAP.md` T7.1/T7.2, both still `blocked`). §7 below describes all three platforms' migration mechanics for planning purposes, but only the Railway entry reflects a real, exercised deployment. @@ -179,7 +179,7 @@ Input schema: "type": "string", "minLength": 1, "maxLength": 500, - "description": "github: 'owner/repo'. linear: team key or team UUID. local: absolute or repo-relative filesystem path." + "description": "github: 'owner/repo'. linear: the Linear project identity (the team that issues sync into is adapters.linear.team_id, separate adapter metadata). local: absolute or repo-relative filesystem path." }, "adapters": { "type": "object", @@ -197,7 +197,9 @@ Input schema: "type": "object", "properties": { "api_key": { "type": "string", "minLength": 1, "maxLength": 512 }, - "team_id": { "type": "string", "minLength": 1, "maxLength": 200 } + "team_id": { "type": "string", "minLength": 1, "maxLength": 200 }, + "done_state_id": { "type": "string", "minLength": 1, "maxLength": 200 }, + "open_state_id": { "type": "string", "minLength": 1, "maxLength": 200 } }, "required": ["api_key", "team_id"], "additionalProperties": false @@ -330,6 +332,8 @@ Example output: "pivot_decision_id": null, "source_doc_ref": "docs/auth-spec.md", "depends_on_track_ids": [], + "github_issue_url": "https://github.com/acme/widgets/issues/12", + "linear_issue_url": null, "created_at": "2026-08-01T12:00:00.000Z" }, "items": [ @@ -742,7 +746,7 @@ Example operational-failure output: ```json { "ok": false, "error": "LINEAR_AUTH_FAILED: API key rejected" } ``` -`error` string prefixes: `LINEAR_AUTH_FAILED`, `LINEAR_NOT_FOUND` (team or issue not found), `LINEAR_TIMEOUT` (exceeded `KNOTRACK_LINEAR_SYNC_TIMEOUT_MS`, default 8000ms), `LINEAR_UNKNOWN_ERROR`. +`error` string prefixes: `LINEAR_AUTH_FAILED`, `LINEAR_NOT_FOUND` (team or issue not found), `LINEAR_RATE_LIMITED`, `LINEAR_TIMEOUT` (exceeded `KNOTRACK_LINEAR_SYNC_TIMEOUT_MS`, default 8000ms), `LINEAR_STATE_CONFIG` (invalid `done_state_id`/`open_state_id`, or a done track whose team has no `completed` workflow state and no configured `done_state_id`), `LINEAR_UNKNOWN_ERROR`. Errors (tool-level, via `isError`): `401`; `404` (project or track not found); `409` (no Linear credentials configured for this project); `422` (malformed uuid); `500`. @@ -787,7 +791,7 @@ Errors (tool-level, via `isError`): `401`; `404` (project or track not found); ` 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 `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), `key_version integer NOT NULL DEFAULT 1` (added by `migrations/004_adapters_key_version.sql` — see the rotation bullet below), 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 the first three secret components into the one `encrypted_credential` column instead. `key_version` itself does exist, just on `adapters` directly rather than a separate table. `src/crypto/credential-cipher.ts`'s header comment documents the packed-blob choice 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. +- **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`. As of T5.2/T5.3 the sync path is implemented: `kt_sync_to_github`/`kt_sync_to_linear` read the adapter row via `getAdapterForProject` (`src/db/queries/adapters.ts`) and decrypt it with `decryptCredential` (`src/crypto/credential-cipher.ts`) immediately before the outbound API call, then pass the plaintext token only to the injectable client — the `encrypted_credential` column never appears in any tool's output type. The real clients live at `src/github/github-client.ts` (T5.2) and `src/linear/linear-client.ts` (T5.3); there is no `src/adapters/` directory. `scripts/rotate-encryption-key.ts` reads every row via `listAllAdapters` (`src/db/queries/adapters.ts`) — the only credential reader outside the sync path. (`listAdaptersForProject`, a project-scoped `SELECT *` helper in the same file, is currently unused.) - **Key rotation:** `scripts/rotate-encryption-key.ts` rotates every stored adapter credential to a new `KNOTRACK_ENCRYPTION_KEY`. It reads the current key from `KNOTRACK_ENCRYPTION_KEY` and the new key from `KNOTRACK_ENCRYPTION_KEY_NEW` (same base64/32-byte shape), decrypts every `adapters` row with the current key, re-encrypts with the new key, and bumps `key_version` — all inside one transaction, so a failure partway through (wrong current key, a corrupted row) rolls back every row rather than leaving some on the old key and some on the new one. It also prints the new key value back on success, since a rotation that completes without the operator retaining that value makes every just-rotated credential permanently undecryptable — the same "print secrets to stdout only" convention `scripts/generate-token.ts` already uses. This is a manual, operator-run, offline-key-swap rotation: KnoTrack v1 never supports more than one *active* encryption key on a running server, so the operator runs this script, then updates `KNOTRACK_ENCRYPTION_KEY` to the new value and redeploys — restarting the server with the old key still configured, in between, would leave it unable to decrypt what the script just re-encrypted. `key_version` exists specifically so a future rotation implementation (or an operator auditing rotation history) can tell which rows are on which generation of key; nothing in v1 currently branches on its value beyond incrementing it. Invocation differs by target, same split as `scripts/migrate.ts` (§2): `npm run rotate-encryption-key` locally (via `tsx`), or `node dist/scripts/rotate-encryption-key.js` in the Docker runtime image, where `tsx` and `scripts/*.ts` are both absent (only `dist` and production dependencies are copied in). --- @@ -957,7 +961,7 @@ similar name — it's never raised anywhere in this build. | `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 `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. | -**Schema decided (Paul, 2026-08-25); rule still unbuilt.** `last_github_sync_at`/`last_linear_sync_at` now exist as nullable `timestamptz` columns directly on `tracks` (`migrations/005_tracks_sync_timestamps.sql`) — scoped per track, not on `adapters`, since `uq_adapters_project_type` allows only one adapter row per `(project_id, type)` and a project can have more than one track syncing through that same adapter; a `tracks`-scoped column has no such sharing problem, while an `adapters`-scoped one would make syncing track A silently mark track B "up to date" too. See that migration's header comment for the full reasoning, including why a `track_id`+`adapter_id` join table was considered and rejected (no real second-adapter-per-type case exists to join against, given `uq_adapters_project_type`). This closes the schema gap only: `SYNC_DRIFT` itself is still unbuilt (the only two `kind` values `drift_flags` currently accepts, `out_of_sequence` and `orphan_file_change`, still don't include a sync-drift kind — that plus `kt_sync_to_github`/`kt_sync_to_linear`'s actual implementation remain T6/T5 work, not started). +**Schema decided (Paul, 2026-08-25); rule still unbuilt.** `last_github_sync_at`/`last_linear_sync_at` now exist as nullable `timestamptz` columns directly on `tracks` (`migrations/005_tracks_sync_timestamps.sql`) — scoped per track, not on `adapters`, since `uq_adapters_project_type` allows only one adapter row per `(project_id, type)` and a project can have more than one track syncing through that same adapter; a `tracks`-scoped column has no such sharing problem, while an `adapters`-scoped one would make syncing track A silently mark track B "up to date" too. See that migration's header comment for the full reasoning, including why a `track_id`+`adapter_id` join table was considered and rejected (no real second-adapter-per-type case exists to join against, given `uq_adapters_project_type`). This closes the schema gap only: `SYNC_DRIFT` itself is still unbuilt (the only two `kind` values `drift_flags` currently accepts, `out_of_sequence` and `orphan_file_change`, still don't include a sync-drift kind — that remains T6 work, not started). `kt_sync_to_github` and `kt_sync_to_linear` themselves are now implemented (T5.2 / T5.3) and write `last_github_sync_at` / `last_linear_sync_at` on each successful sync — so the watermark side of `SYNC_DRIFT` is already populated; only the drift rule that reads it is outstanding. --- diff --git a/src/db/queries/track-external-links.ts b/src/db/queries/track-external-links.ts index 3bf9e27..735384c 100644 --- a/src/db/queries/track-external-links.ts +++ b/src/db/queries/track-external-links.ts @@ -114,13 +114,21 @@ export async function updateLinkedContentHash( trackId: string; adapterType: 'github' | 'linear'; contentHash: string; + /** Optional refreshed external URL. When provided, external_url is + * updated too — a successful update returns the issue's current URL, which + * can change while the id stays stable (e.g. a Linear team-key/workspace + * slug change), so a linked row would otherwise keep exposing a stale URL + * through kt_get_track (Codex PR #25). Omitted (undefined) leaves the + * stored URL unchanged, preserving existing GitHub behavior. */ + externalUrl?: string; }, ): Promise { await client.query( `UPDATE track_external_links - SET content_hash = $3 + SET content_hash = $3, + external_url = COALESCE($4, external_url) WHERE track_id = $1 AND adapter_type = $2 AND sync_state = 'linked'`, - [input.trackId, input.adapterType, input.contentHash], + [input.trackId, input.adapterType, input.contentHash, input.externalUrl ?? null], ); } @@ -159,3 +167,19 @@ export async function getGithubIssueUrlForTrack( ); return result.rows[0]?.external_url ?? null; } + +/** The linked Linear issue URL for a track, or null when unlinked (T5.3). + * Sibling of getGithubIssueUrlForTrack; kept adapter-specific for the same + * reason — callers ask for one destination's URL, never "some" URL. */ +export async function getLinearIssueUrlForTrack( + db: Queryable, + trackId: string, +): Promise { + const result = await db.query<{ external_url: string | null }>( + `SELECT external_url FROM track_external_links + WHERE track_id = $1 AND adapter_type = 'linear' AND sync_state = 'linked' + LIMIT 1`, + [trackId], + ); + return result.rows[0]?.external_url ?? null; +} diff --git a/src/db/queries/tracks.ts b/src/db/queries/tracks.ts index ba1544d..673fd93 100644 --- a/src/db/queries/tracks.ts +++ b/src/db/queries/tracks.ts @@ -382,3 +382,15 @@ export async function touchGithubSyncWatermark( // pushed, not the later "now()". await db.query(`UPDATE tracks SET last_github_sync_at = $2 WHERE id = $1`, [trackId, at]); } + +/** Stamps `tracks.last_linear_sync_at` after a successful Linear sync (T5.3). + * Same snapshot-time semantics as the GitHub watermark above: `at` is the + * payload snapshot time, not the completion time, so a change made after the + * snapshot still reads as drift on the next scan. */ +export async function touchLinearSyncWatermark( + db: Queryable, + trackId: string, + at: Date, +): Promise { + await db.query(`UPDATE tracks SET last_linear_sync_at = $2 WHERE id = $1`, [trackId, at]); +} diff --git a/src/linear/linear-client.ts b/src/linear/linear-client.ts new file mode 100644 index 0000000..009ae58 --- /dev/null +++ b/src/linear/linear-client.ts @@ -0,0 +1,478 @@ +// Linear GraphQL client for kt_sync_to_linear (docs/PRD.md §4.14, T5.3). +// +// Mirrors src/github/github-client.ts by design: a thin native-fetch +// wrapper (no @linear/sdk), one HTTP call per method, a discriminated +// result that NEVER throws for an API-level outcome and NEVER logs or +// echoes the api_key. The frontier panel (2026-09-19) chose the same +// smallest-auditable-dependency approach as GitHub for this self-hosted +// server. +// +// Linear differences handled here: +// - Transport is GraphQL: one endpoint, POST { query, variables }. A +// GraphQL request can return HTTP 200 with a top-level `errors` array. +// On a MUTATION that is always ambiguous ({ok:false}, ambiguous:true): a 200 +// can carry partial data + errors, so a GraphQL error never proves no write +// happened. HTTP 4xx (auth / rate-limit / not-found) stays definitive in +// mapHttpError. Reads are always definitive. +// - Auth header is the raw api_key ("Authorization: "), NOT +// "Bearer " (that form is only for OAuth access tokens). +// - There is no open/closed; a workflow state (stateId) is resolved by +// the caller and passed in. This client only sets what it is given. +// +// Operational failures map to fixed prefixes +// (LINEAR_AUTH_FAILED / LINEAR_NOT_FOUND / LINEAR_RATE_LIMITED / +// LINEAR_TIMEOUT / LINEAR_UNKNOWN_ERROR). `ambiguous` tells the caller +// whether a create mutation may have reached Linear despite the failure +// (timeout / 5xx / network / malformed-2xx) so it can keep a durable +// `pending` link for marker recovery instead of recreating a duplicate. + +const LINEAR_API = 'https://api.linear.app/graphql'; + +export interface LinearIssuePayload { + title: string; + description: string; +} + +export interface LinearIssueRef { + /** Linear issue UUID — issueUpdate targets this. */ + id: string; + /** Human identifier, e.g. "ENG-123" (informational). */ + identifier: string; + url: string; +} + +export interface LinearWorkflowState { + id: string; + name: string; + /** backlog | unstarted | started | completed | canceled */ + type: string; + position: number; +} + +export type LinearResult = + { ok: true; value: T } | { ok: false; error: string; ambiguous: boolean }; + +export interface LinearClient { + /** issueCreate — creates an issue in `teamId`; sets `stateId` when given. */ + createIssue( + teamId: string, + payload: LinearIssuePayload, + stateId?: string, + ): Promise>; + /** issueUpdate — reconciles title/description (and stateId when given). */ + updateIssue( + issueId: string, + payload: LinearIssuePayload, + stateId?: string, + ): Promise>; + /** Recovery-only: find an existing issue in `teamId` whose description + * carries `marker`. Returns null when none is found. */ + findIssueByMarker(teamId: string, marker: string): Promise>; + /** The team's workflow states, for the state resolver. */ + getWorkflowStates(teamId: string): Promise>; + /** The current workflow-state TYPE of an issue (e.g. 'completed'), used to + * decide whether a reopen should move it backward. Null when the issue or + * its state is absent. */ + getIssueStateType(issueId: string): Promise>; +} + +export interface LinearClientOptions { + timeoutMs: number; + userAgent: string; +} + +export type LinearClientFactory = (apiKey: string, opts: LinearClientOptions) => LinearClient; + +function isRateLimitedMessage(text: string): boolean { + return /ratelimit|rate limit|too many requests/i.test(text); +} + +function isAuthMessage(text: string): boolean { + return /authentication|not authenticated|invalid api key|unauthorized|forbidden/i.test(text); +} + +function isNotFoundMessage(text: string): boolean { + return /entity_?not_?found|not found|could not find|does not exist|no such/i.test(text); +} + +/** Removes the api_key from any string before it can surface in a returned + * error or a log line (adversarial review, GPT-6, PR #25). Error paths + * interpolate upstream response bodies and native-fetch exception messages + * (which can quote a malformed Authorization header value), so redaction — not + * truncation — is what keeps invariant 3 (the key never appears in output). + * Applied to the FULL upstream text inside each map function, before any + * length truncation, so a boundary-straddling secret cannot survive. */ +function redactSecret(text: string, secret: string): string { + if (!secret) return text; + return text.split(secret).join('[REDACTED]'); +} + +/** Maps a completed HTTP response (non-2xx) to a prefixed error + ambiguity. + * 4xx means Linear rejected the request, so a create that got one did NOT + * write (ambiguous=false). 5xx may have written then failed to respond, so a + * create that got a 5xx is AMBIGUOUS — the caller keeps its pending link. */ +function mapHttpError( + status: number, + headers: Headers, + bodyText: string, + secret: string, +): { error: string; ambiguous: boolean } { + // Redact the FULL upstream body BEFORE truncating: a secret that straddles + // the 300-char boundary would otherwise survive as a partial the later + // exact-string redact cannot match (Codex PR #25). + // Classify on the FULL redacted text; truncate only the detail we return, so an + // identifying keyword past char 300 is not lost before classification (Codex PR #25). + const fullDetail = redactSecret(bodyText, secret).replace(/\s+/g, ' ').trim(); + const detail = fullDetail.slice(0, 300); + // 5xx is checked FIRST: a 5xx may have committed the write server-side and + // then failed to respond, so it is ALWAYS ambiguous — even when it carries a + // Retry-After header (Codex PR #25: a 5xx+Retry-After must not be reclassified + // as a definitive rate-limit, or createFresh would clear the pending claim + // and the next sync could duplicate the issue). + if (status >= 500) + return { + error: `LINEAR_UNKNOWN_ERROR: HTTP ${status}${detail ? `: ${detail}` : ''}`, + ambiguous: true, + }; + if (status === 401 || status === 403) + return { error: `LINEAR_AUTH_FAILED: ${detail || 'unauthorized'}`, ambiguous: false }; + if (status === 429 || headers.get('retry-after')) + return { error: `LINEAR_RATE_LIMITED: ${detail || 'rate limit exceeded'}`, ambiguous: false }; + if (status === 404) + return { error: `LINEAR_NOT_FOUND: ${detail || 'not found'}`, ambiguous: false }; + if (status === 400 && isAuthMessage(fullDetail)) + return { error: `LINEAR_AUTH_FAILED: ${detail}`, ambiguous: false }; + if (status === 400 && isRateLimitedMessage(fullDetail)) + return { error: `LINEAR_RATE_LIMITED: ${detail}`, ambiguous: false }; + // Any other completed 4xx is a definitive rejection (no write) for a create. + return { + error: `LINEAR_UNKNOWN_ERROR: HTTP ${status}${detail ? `: ${detail}` : ''}`, + ambiguous: false, + }; +} + +/** Maps a GraphQL top-level `errors` array (returned with HTTP 200) to a + * prefixed error. Auth/rate-limit errors are DEFINITIVE (Linear rejected the + * request before applying it) → ambiguous:false. But a generic GraphQL error + * on a MUTATION is NOT proof that no write occurred — a response may carry both + * partial `data` and top-level `errors` once execution has begun (Codex PR #25) + * — so an opaque mutation error is ambiguous, keeping the pending link for + * marker recovery instead of clearing it and risking a duplicate. */ +function mapGraphqlErrors( + errors: unknown[], + isMutation: boolean, + secret: string, +): { error: string; ambiguous: boolean } { + // Redact the FULL joined message BEFORE truncating (see mapHttpError): a + // boundary-straddling key must not survive as an unmatched partial (Codex PR #25). + // Redact the FULL joined message; classify on all of it, truncate only what we + // return, so a keyword past char 300 is not dropped before classification + // (Codex PR #25). A boundary-straddling key still cannot survive (redact first). + const fullMsg = redactSecret( + errors + .map((e) => + e && typeof e === 'object' && typeof (e as Record).message === 'string' + ? ((e as Record).message as string) + : '', + ) + .filter(Boolean) + .join('; '), + secret, + ); + const msg = fullMsg.slice(0, 300); + const code = errors + .map((e) => { + const ext = + e && typeof e === 'object' ? (e as Record).extensions : undefined; + if (!ext || typeof ext !== 'object') return ''; + const c = (ext as Record).code; + return typeof c === 'string' ? c : ''; + }) + .join(' '); + const haystack = `${fullMsg} ${code}`; + // On a MUTATION, a top-level GraphQL `errors` array on an HTTP 200 does NOT + // prove the write did not happen: Linear (like any GraphQL server) can return + // partial `data` alongside `errors` once execution has begun, and a text match + // over the message cannot establish a pre-execution rejection. So every mutation + // GraphQL error stays AMBIGUOUS (ambiguous:isMutation) and createFresh keeps its + // pending link for marker recovery rather than clearing it and risking a + // DUPLICATE issue -- honoring the invariant "duplicate prevention takes priority + // over auto-retry when the outcome cannot be established" (CodeRabbit + frontier + // panel, PR #25). The category prefix is still surfaced. HTTP-status auth + // (401/403), rate-limit (429/400) and not-found (404) stay DEFINITIVE in + // mapHttpError, which covers the common expired-key / stale-team case. A READ + // writes nothing, so its errors are always definitive. + // FOLLOW-UP (T5.3, needs the real-workspace dogfood): tighten to a three-way + // classifier -- definitive on a mutation ONLY for an allowlisted pre-execution + // extensions.code (e.g. AUTHENTICATION_ERROR / FORBIDDEN) with no usable + // data.issueCreate.issue; ambiguous otherwise. Requires real Linear response + // bodies as golden-file tests, which the live dogfood will provide. + if (isRateLimitedMessage(haystack)) + return { error: `LINEAR_RATE_LIMITED: ${msg || 'rate limited'}`, ambiguous: isMutation }; + if (isAuthMessage(haystack)) + return { + error: `LINEAR_AUTH_FAILED: ${msg || 'authentication failed'}`, + ambiguous: isMutation, + }; + if (isNotFoundMessage(haystack)) + return { error: `LINEAR_NOT_FOUND: ${msg || 'entity not found'}`, ambiguous: isMutation }; + return { error: `LINEAR_UNKNOWN_ERROR: ${msg || 'GraphQL error'}`, ambiguous: isMutation }; +} + +function mapThrown(err: unknown, secret: string): { error: string; ambiguous: boolean } { + if (err instanceof Error && err.name === 'AbortError') { + return { error: 'LINEAR_TIMEOUT: request exceeded the configured timeout', ambiguous: true }; + } + const msg = redactSecret(err instanceof Error ? err.message : String(err), secret); + // A request that never got a response may still have been received and + // acted on by Linear -> ambiguous for the create path. + return { error: `LINEAR_UNKNOWN_ERROR: ${msg}`, ambiguous: true }; +} + +export function createFetchLinearClient(apiKey: string, opts: LinearClientOptions): LinearClient { + const headers: Record = { + // Linear personal API keys are sent as the RAW Authorization value — no + // "Bearer " prefix (that form is for OAuth tokens only). + Authorization: apiKey, + 'Content-Type': 'application/json', + 'User-Agent': opts.userAgent, + }; + + /** One GraphQL round-trip. `isMutation` drives the ambiguity of a + * malformed 2xx body (a write may have landed; a read cannot have). */ + async function send( + query: string, + variables: Record, + isMutation: boolean, + ): Promise>> { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), opts.timeoutMs); + try { + const res = await fetch(LINEAR_API, { + method: 'POST', + headers, + body: JSON.stringify({ query, variables }), + signal: controller.signal, + }); + const text = await res.text(); + if (!res.ok) { + const mapped = mapHttpError(res.status, res.headers, text, apiKey); + return { + ok: false, + error: mapped.error, + ambiguous: mapped.ambiguous, + }; + } + let parsed: unknown; + try { + parsed = text.length > 0 ? JSON.parse(text) : {}; + } catch { + return { + ok: false, + error: 'LINEAR_UNKNOWN_ERROR: malformed JSON in Linear response', + // A 2xx mutation may already have written; a read cannot have. + ambiguous: isMutation, + }; + } + const obj = (parsed ?? {}) as Record; + if (Array.isArray(obj.errors) && obj.errors.length > 0) { + const mapped = mapGraphqlErrors(obj.errors, isMutation, apiKey); + return { + ok: false, + error: mapped.error, + ambiguous: mapped.ambiguous, + }; + } + const data = obj.data; + if (data === null || typeof data !== 'object') { + return { + ok: false, + error: 'LINEAR_UNKNOWN_ERROR: Linear response missing data', + ambiguous: isMutation, + }; + } + return { ok: true, value: data as Record }; + } catch (err) { + const mapped = mapThrown(err, apiKey); + return { ok: false, error: mapped.error, ambiguous: mapped.ambiguous }; + } finally { + clearTimeout(timer); + } + } + + function toRef(node: unknown): LinearIssueRef | null { + if (node === null || typeof node !== 'object') return null; + const rec = node as Record; + const id = rec.id; + const url = rec.url; + const identifier = rec.identifier; + if (typeof id !== 'string' || typeof url !== 'string') return null; + return { id, url, identifier: typeof identifier === 'string' ? identifier : '' }; + } + + function refFromMutation( + data: Record, + field: 'issueCreate' | 'issueUpdate', + ): LinearResult { + const payload = data[field]; + if (payload === null || typeof payload !== 'object') { + return { + ok: false, + error: `LINEAR_UNKNOWN_ERROR: ${field} returned no payload`, + // The mutation reached Linear (2xx, no errors) but the shape is + // unexpected — treat as ambiguous so a create keeps its pending link. + ambiguous: true, + }; + } + const rec = payload as Record; + // An explicit `success: false` in a well-formed payload is a DEFINITIVE + // rejection — Linear applied nothing — so it is not ambiguous; clearing the + // pending claim lets the next sync retry cleanly instead of wedging forever + // (Codex PR #25). Only an unexpected/incomplete shape stays ambiguous. + if (rec.success === false) { + return { + ok: false, + error: `LINEAR_UNKNOWN_ERROR: ${field} returned success:false (mutation rejected)`, + ambiguous: false, + }; + } + const ref = toRef(rec.issue); + if (rec.success !== true || !ref) { + return { + ok: false, + error: `LINEAR_UNKNOWN_ERROR: ${field} did not return success/issue`, + ambiguous: true, + }; + } + return { ok: true, value: ref }; + } + + const ISSUE_FIELDS = 'id identifier url'; + + return { + async createIssue(teamId, payload, stateId) { + const input: Record = { + teamId, + title: payload.title, + description: payload.description, + }; + if (stateId) input.stateId = stateId; + const res = await send( + `mutation KtCreate($input: IssueCreateInput!) { + issueCreate(input: $input) { success issue { ${ISSUE_FIELDS} } } + }`, + { input }, + true, + ); + if (!res.ok) return res; + return refFromMutation(res.value, 'issueCreate'); + }, + + async updateIssue(issueId, payload, stateId) { + const input: Record = { + title: payload.title, + description: payload.description, + }; + if (stateId) input.stateId = stateId; + const res = await send( + `mutation KtUpdate($id: String!, $input: IssueUpdateInput!) { + issueUpdate(id: $id, input: $input) { success issue { ${ISSUE_FIELDS} } } + }`, + { id: issueId, input }, + true, + ); + if (!res.ok) return res; + return refFromMutation(res.value, 'issueUpdate'); + }, + + async findIssueByMarker(teamId, marker) { + // Recovery-only. Linear's `description: { contains }` filter is a + // server-side substring match; we still re-check the exact marker on + // each node (defence in depth) and treat a truncated page as + // unconfirmed so recovery never recreates on a false miss. + const res = await send( + `query KtFind($teamId: ID!, $marker: String!) { + issues( + filter: { team: { id: { eq: $teamId } }, description: { contains: $marker } } + first: 20 + ) { nodes { ${ISSUE_FIELDS} description } pageInfo { hasNextPage } } + }`, + { teamId, marker }, + false, + ); + if (!res.ok) return res; + const issues = (res.value.issues ?? {}) as Record; + const nodes = Array.isArray(issues.nodes) ? (issues.nodes as unknown[]) : []; + for (const node of nodes) { + if (node && typeof node === 'object') { + const rec = node as Record; + if (typeof rec.description === 'string' && rec.description.includes(marker)) { + const ref = toRef(node); + if (ref) return { ok: true, value: ref }; + } + } + } + const pageInfo = (issues.pageInfo ?? {}) as Record; + if (pageInfo.hasNextPage === true) { + return { + ok: false, + error: + 'LINEAR_UNKNOWN_ERROR: issue search returned more results than inspected; could not confirm', + ambiguous: true, + }; + } + return { ok: true, value: null }; + }, + + async getWorkflowStates(teamId) { + const res = await send( + `query KtStates($teamId: String!) { + team(id: $teamId) { states { nodes { id name type position } } } + }`, + { teamId }, + false, + ); + if (!res.ok) return res; + const team = res.value.team; + if (team === null || typeof team !== 'object') { + return { ok: false, error: `LINEAR_NOT_FOUND: team ${teamId} not found`, ambiguous: false }; + } + const states = ((team as Record).states ?? {}) as Record; + const nodes = Array.isArray(states.nodes) ? (states.nodes as unknown[]) : []; + const out: LinearWorkflowState[] = []; + for (const node of nodes) { + if (node && typeof node === 'object') { + const rec = node as Record; + if ( + typeof rec.id === 'string' && + typeof rec.type === 'string' && + typeof rec.position === 'number' + ) { + out.push({ + id: rec.id, + name: typeof rec.name === 'string' ? rec.name : '', + type: rec.type, + position: rec.position, + }); + } + } + } + return { ok: true, value: out }; + }, + + async getIssueStateType(issueId) { + const res = await send( + `query KtIssueState($id: String!) { issue(id: $id) { state { type } } }`, + { id: issueId }, + false, + ); + if (!res.ok) return res; + const issue = res.value.issue; + if (issue === null || typeof issue !== 'object') return { ok: true, value: null }; + const state = (issue as Record).state; + if (state === null || typeof state !== 'object') return { ok: true, value: null }; + const type = (state as Record).type; + return { ok: true, value: typeof type === 'string' ? type : null }; + }, + }; +} diff --git a/src/mcp/tools/get-track.ts b/src/mcp/tools/get-track.ts index 0a1328d..e9db1a8 100644 --- a/src/mcp/tools/get-track.ts +++ b/src/mcp/tools/get-track.ts @@ -6,7 +6,10 @@ import { getTrackInputSchema, type GetTrackInput } from '../../schemas/tools.js' import { findActiveProjectById } from '../../db/queries/projects.js'; import { findTrackById, getDependsOnTrackIds } from '../../db/queries/tracks.js'; import { getItemDependencyEdgesForTrack, listItemsByTrack } from '../../db/queries/items.js'; -import { getGithubIssueUrlForTrack } from '../../db/queries/track-external-links.js'; +import { + getGithubIssueUrlForTrack, + getLinearIssueUrlForTrack, +} from '../../db/queries/track-external-links.js'; import { withReadSnapshot } from '../../db/tx.js'; import { notFound } from '../errors.js'; import { runTool } from '../tool-helpers.js'; @@ -27,6 +30,9 @@ export interface GetTrackOutput extends Record { * This is where "kt_sync_to_github records the issue URL on the track" * (docs/PRD.md §4.13) is observable through the read contract. */ github_issue_url: string | null; + /** T5.3: the URL of the Linear Issue this track is synced to, or null — + * the read-contract equivalent of github_issue_url (docs/PRD.md §4.14). */ + linear_issue_url: string | null; created_at: string; }; items: Array<{ @@ -67,6 +73,7 @@ export async function getTrackService( const items = await listItemsByTrack(client, track.id); const itemEdges = await getItemDependencyEdgesForTrack(client, track.id); const githubIssueUrl = await getGithubIssueUrlForTrack(client, track.id); + const linearIssueUrl = await getLinearIssueUrlForTrack(client, track.id); const dependsOnByItemId = new Map(); for (const edge of itemEdges) { @@ -87,6 +94,7 @@ export async function getTrackService( source_doc_ref: track.source_doc_ref, depends_on_track_ids: dependsOnTrackIds, github_issue_url: githubIssueUrl, + linear_issue_url: linearIssueUrl, created_at: track.created_at.toISOString(), }, items: items.map((item) => ({ diff --git a/src/mcp/tools/issue-payload.ts b/src/mcp/tools/issue-payload.ts index 352b3a7..aae6fe0 100644 --- a/src/mcp/tools/issue-payload.ts +++ b/src/mcp/tools/issue-payload.ts @@ -51,7 +51,7 @@ export function trackMarker(trackId: string): string { return ``; } -function truncate(value: string, max: number): string { +export function truncate(value: string, max: number): string { return value.length <= max ? value : value.slice(0, max); } @@ -65,7 +65,7 @@ function itemCheckbox(status: string): string { * an unrelated issue — nor open a stray hidden HTML comment. HTML-escaping * the delimiters keeps the text readable (GitHub renders `<`/`>` as * literal `<`/`>`) while making the marker syntax impossible to form. */ -function sanitize(value: string): string { +export function sanitize(value: string): string { return value.replace(//g, '-->'); } diff --git a/src/mcp/tools/linear-payload.ts b/src/mcp/tools/linear-payload.ts new file mode 100644 index 0000000..76189af --- /dev/null +++ b/src/mcp/tools/linear-payload.ts @@ -0,0 +1,204 @@ +// Pure track -> Linear Issue payload mapping, content hashing, and workflow- +// state resolution for kt_sync_to_linear (docs/PRD.md §4.14, docs/ROADMAP.md +// T5.3). Side-effect-free and free of any DB/HTTP import (the LinearWorkflowState +// type is imported type-only) so the mapping, the no-op hash, and the state +// resolver are unit-testable in isolation. +// +// KnoTrack owns exactly the issue title, description, and — for a done track — +// its workflow state. The description carries the same hidden recovery marker +// as the GitHub body (reused via issue-payload.ts), used ONLY for crash +// recovery, never as the authoritative create-vs-update key (that is the +// track_external_links row). +import { createHash } from 'node:crypto'; +import { sanitize, truncate, trackMarker } from './issue-payload.js'; +import type { LinearWorkflowState } from '../../linear/linear-client.js'; + +// Linear's issue title limit; we stay under it rather than letting the API +// reject the mutation. +const TITLE_MAX = 255; +// Linear descriptions allow large markdown; bound worst-case body size the +// same way GitHub does so a huge track can't render an unbounded checklist. +const DESC_MAX = 65_536; +const ITEM_RENDER_CAP = 300; + +export interface TrackForLinear { + id: string; + title: string; + /** Derived track status: on_track | pivot_pending | blocked | done. */ + status: string; +} + +export interface ItemForLinear { + id: string; + title: string; + status: string; + sequence_position: number; +} + +export interface LinearPayload { + title: string; + description: string; +} + +/** 'done' when the track is terminal, else 'open'. Folded into the content + * hash so flipping a track's done-ness always forces a re-sync (which lets + * the state resolver move the Linear issue to/from a completed state). */ +export function stateIntentFor(trackStatus: string): 'done' | 'open' { + return trackStatus === 'done' ? 'done' : 'open'; +} + +function itemCheckbox(status: string): string { + return status === 'done' ? '[x]' : '[ ]'; +} + +/** Deterministic mapping — pure function of (track, items). Mirrors the + * GitHub body (issue-payload.ts) but targets Linear's {title, description} + * and omits the open/closed field (Linear state is a separate stateId + * resolved by resolveLinearStateId). */ +export function buildLinearPayload(track: TrackForLinear, items: ItemForLinear[]): LinearPayload { + // Escape FIRST, then enforce the limit (sanitize expands `` into + // longer entities), matching the GitHub title fix. + const title = truncate(sanitize(track.title), TITLE_MAX); + + const ordered = [...items].sort( + (a, b) => a.sequence_position - b.sequence_position || (a.id < b.id ? -1 : a.id > b.id ? 1 : 0), + ); + + const header = [ + '_Synced from KnoTrack — KnoTrack owns this issue’s title, description, and (when the track is done) its workflow state, and will overwrite manual edits to them._', + '', + `**Track status:** \`${track.status}\``, + '', + '### Items', + '', + ].join('\n'); + + const markerBlock = `\n\n${trackMarker(track.id)}`; + const NOTICE_RESERVE = 64; + const budget = Math.max(0, DESC_MAX - markerBlock.length); + + const lines: string[] = []; + let used = header.length; + let shownCount = 0; + for (const item of ordered) { + if (shownCount >= ITEM_RENDER_CAP) break; + const line = `- ${itemCheckbox(item.status)} ${sanitize(item.title)}\n`; + const reserve = shownCount + 1 < ordered.length ? NOTICE_RESERVE : 0; + if (used + line.length + reserve > budget) break; + lines.push(line); + used += line.length; + shownCount += 1; + } + + const omitted = ordered.length - shownCount; + const checklist = + shownCount > 0 ? lines.join('') : ordered.length === 0 ? '_No items yet._\n' : ''; + const notice = omitted > 0 ? `\n_… ${omitted} more item(s) not shown._` : ''; + + const description = truncate(header + checklist + notice, budget) + markerBlock; + + return { title, description }; +} + +/** sha256 of exactly what KnoTrack pushes to Linear: title, description, the + * abstract state intent (done|open), and the configured state overrides. The + * intent (not an auto-resolved stateId) keeps a team's auto-picked state UUIDs + * out of the hash — those differ per team and would cause needless churn — while + * a done/undone flip still re-syncs; the operator-configured done_state_id/ + * open_state_id ARE included so changing them re-syncs. Detects KnoTrack-side + * changes only. */ +export function linearPayloadContentHash( + payload: LinearPayload, + stateIntent: 'done' | 'open', + stateConfig: { doneStateId?: string; openStateId?: string } = {}, +): string { + const canonical = JSON.stringify({ + title: payload.title, + description: payload.description, + state_intent: stateIntent, + // The configured overrides are part of what a sync would push: if a project + // is re-registered with a different done_state_id/open_state_id, the hash + // must change so the linked issue is re-synced (and the new override + // re-validated) instead of taking a false no-op path (Codex PR #25). + done_state_id: stateConfig.doneStateId ?? null, + open_state_id: stateConfig.openStateId ?? null, + }); + return createHash('sha256').update(canonical, 'utf8').digest('hex'); +} + +export type StateResolution = { stateId?: string } | { error: string }; + +function lowestPosition(states: LinearWorkflowState[]): LinearWorkflowState | undefined { + return states.reduce((best, s) => { + if (!best) return s; + if (s.position < best.position) return s; + if (s.position === best.position && s.id < best.id) return s; + return best; + }, undefined); +} + +/** The single shared workflow-state resolver (frontier checklist item 15). + * + * - done track: set the configured `done_state_id`, or (default) the team's + * lowest-position 'completed' state. Fails LINEAR_STATE_CONFIG when the + * override is unknown/not 'completed', or when no completed state exists + * and none is configured. + * - non-done track: NEVER auto-moves the issue (frontier checklist item 2). + * It moves backward only when an explicit, valid `open_state_id` is + * configured AND (on update) the issue is currently in a completed/canceled + * state. On create with `open_state_id` set, new issues are placed there; + * with none set, Linear assigns the team default. + * + * Returns `{}` (no stateId) to mean "leave the state as-is / let Linear + * decide", or `{ error }` for an invalid configuration. */ +export function resolveLinearStateId(args: { + states: LinearWorkflowState[]; + doneStateId?: string; + openStateId?: string; + trackStatus: string; + mode: 'create' | 'update'; + currentStateType?: string | null; +}): StateResolution { + const { states, doneStateId, openStateId, trackStatus, mode, currentStateType } = args; + const byId = new Map(states.map((s) => [s.id, s])); + + if (doneStateId !== undefined) { + const s = byId.get(doneStateId); + if (!s) + return { + error: `LINEAR_STATE_CONFIG: configured done_state_id is not a workflow state of this team`, + }; + if (s.type !== 'completed') + return { + error: `LINEAR_STATE_CONFIG: configured done_state_id is a '${s.type}' state, not 'completed'`, + }; + } + if (openStateId !== undefined) { + const s = byId.get(openStateId); + if (!s) + return { + error: `LINEAR_STATE_CONFIG: configured open_state_id is not a workflow state of this team`, + }; + if (s.type === 'completed' || s.type === 'canceled') + return { + error: `LINEAR_STATE_CONFIG: configured open_state_id is a '${s.type}' (terminal) state; choose a non-terminal (backlog/unstarted/started) state to reopen into`, + }; + } + + if (trackStatus === 'done') { + const chosen = doneStateId ?? lowestPosition(states.filter((s) => s.type === 'completed'))?.id; + if (!chosen) + return { + error: `LINEAR_STATE_CONFIG: this team has no 'completed' workflow state; set done_state_id on the Linear adapter`, + }; + return { stateId: chosen }; + } + + // Non-done: never auto-move. Only an explicit open_state_id can set state. + if (openStateId === undefined) return {}; + if (mode === 'create') return { stateId: openStateId }; + // Update: only reopen (move backward) if the issue is currently done/canceled. + if (currentStateType === 'completed' || currentStateType === 'canceled') + return { stateId: openStateId }; + return {}; +} diff --git a/src/mcp/tools/register-project.ts b/src/mcp/tools/register-project.ts index a7abb8e..e1cd4f3 100644 --- a/src/mcp/tools/register-project.ts +++ b/src/mcp/tools/register-project.ts @@ -76,6 +76,14 @@ export async function registerProjectService( encryptedCredential: encrypted, config: { team_id: input.adapters.linear.team_id, + // Optional workflow-state overrides (T5.3); only stored when set, + // so an adapter registered before T5.3 keeps auto-resolution. + ...(input.adapters.linear.done_state_id + ? { done_state_id: input.adapters.linear.done_state_id } + : {}), + ...(input.adapters.linear.open_state_id + ? { open_state_id: input.adapters.linear.open_state_id } + : {}), connected: true, }, }); diff --git a/src/mcp/tools/sync-shared.ts b/src/mcp/tools/sync-shared.ts deleted file mode 100644 index fb44323..0000000 --- a/src/mcp/tools/sync-shared.ts +++ /dev/null @@ -1,62 +0,0 @@ -// Shared precondition path for kt_sync_to_github / kt_sync_to_linear -// (T2.13 / T2.14 stub slice — docs/PRD.md §4.13/§4.14). -import type { Pool } from 'pg'; -import { findActiveProjectById } from '../../db/queries/projects.js'; -import { trackExistsInProject } from '../../db/queries/tracks.js'; -import { adapterConfigured } from '../../db/queries/adapters.js'; -import { conflict, internalError, notFound } from '../errors.js'; - -export interface SyncOutput extends Record { - ok: true; -} - -export interface SyncInput { - project_id: string; - track_id: string; -} - -/** - * Validates the project and track exist (project-scoped track lookup, so - * a `track_id` from another project is a 404, never a cross-project - * leak), then checks the adapter precondition: - * - no adapter row for (project, type) -> CONFLICT " adapter not - * configured" (the caller must configure credentials first; PRD - * §4.13 acceptance). Makes no outbound HTTP call. - * - an adapter row exists -> the real GitHub/Linear push is not built - * in this build, so this returns a clear " sync is not - * available in this build" INTERNAL_ERROR rather than a false success - * or a misleading "not configured". This is a normal, client-reachable - * state: kt_register_project provisions an adapter from inline - * credentials (register-project.ts), so any client that registers a - * project with a GitHub/Linear credential reaches it. The T5.2 / T5.3 - * API path replaces this branch and the success return below. - */ -export async function syncAdapterStub( - pool: Pool, - type: 'github' | 'linear', - input: SyncInput, -): Promise { - const project = await findActiveProjectById(pool, input.project_id); - if (!project) { - throw notFound('project not found', { project_id: input.project_id }); - } - const trackExists = await trackExistsInProject(pool, input.project_id, input.track_id); - if (!trackExists) { - throw notFound('track not found in this project', { - project_id: input.project_id, - track_id: input.track_id, - }); - } - const configured = await adapterConfigured(pool, input.project_id, type); - if (!configured) { - throw conflict(`${type} adapter not configured`, { - project_id: input.project_id, - adapter: type, - }); - } - throw internalError(`${type} sync is not available in this build`, { - project_id: input.project_id, - track_id: input.track_id, - adapter: type, - }); -} diff --git a/src/mcp/tools/sync-to-linear.ts b/src/mcp/tools/sync-to-linear.ts index cd0cd07..79060f9 100644 --- a/src/mcp/tools/sync-to-linear.ts +++ b/src/mcp/tools/sync-to-linear.ts @@ -1,17 +1,414 @@ -// kt_sync_to_linear — docs/PRD.md §4.14 (T2.14 stub slice). +// kt_sync_to_linear — docs/PRD.md §4.14, docs/ROADMAP.md T5.3. +// +// Syncs ONE track to a Linear Issue, idempotently. Deliberately a close +// mirror of sync-to-github.ts: the authoritative create-vs-update key is a +// track_external_links row (never a Linear search), idempotency is hardened +// with a durable `pending` creation-intent committed BEFORE the outbound +// mutation, and a crash recovers via the hidden body marker instead of +// blindly recreating. See that file for the shared rationale. +// +// Linear-specific (frontier panel 2026-09-19, unanimous +// auto_lookup_with_config_override): +// - Transport is GraphQL (src/linear/linear-client.ts). +// - `repo_identity` on the link stores the bound Linear team_id. +// - external_id stores the Linear issue UUID (issueUpdate targets it). +// - There is no open/closed: a done track's issue is moved to a completed +// workflow state (configured done_state_id, or the team's lowest-position +// 'completed' state); a non-done track's issue is never auto-moved back +// unless a valid open_state_id is configured. All in resolveLinearStateId. +// +// Failure surfaces, per §4.14: +// - Missing/unusable adapter precondition -> CONFLICT (a thrown KtError). +// - Everything about actually talking to Linear (and an invalid state +// configuration) -> a SUCCESSFUL tool call returning +// { ok: false, error: ": ..." } with a fixed prefix +// (LINEAR_AUTH_FAILED / LINEAR_NOT_FOUND / LINEAR_RATE_LIMITED / +// LINEAR_TIMEOUT / LINEAR_STATE_CONFIG / LINEAR_UNKNOWN_ERROR). +import { randomUUID } from 'node:crypto'; import type { Pool } from 'pg'; import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import type { Config } from '../../config/env.js'; import { syncToLinearInputSchema, type SyncToLinearInput } from '../../schemas/tools.js'; import { runTool } from '../tool-helpers.js'; -import { syncAdapterStub, type SyncOutput } from './sync-shared.js'; +import { conflict, notFound } from '../errors.js'; +import { findActiveProjectById } from '../../db/queries/projects.js'; +import { findTrackById, touchLinearSyncWatermark } from '../../db/queries/tracks.js'; +import { listItemsByTrack } from '../../db/queries/items.js'; +import { getAdapterForProject } from '../../db/queries/adapters.js'; +import { decryptCredential } from '../../crypto/credential-cipher.js'; +import { trackMarker } from './issue-payload.js'; +import { + buildLinearPayload, + linearPayloadContentHash, + resolveLinearStateId, + stateIntentFor, + type LinearPayload, +} from './linear-payload.js'; +import { withReadSnapshot, withTransaction } from '../../db/tx.js'; +import { + claimPendingLink, + clearPendingLink, + finalizeLinked, + getLinkForUpdate, + updateLinkedContentHash, + type TrackExternalLinkRow, +} from '../../db/queries/track-external-links.js'; +import { + createFetchLinearClient, + type LinearClient, + type LinearClientFactory, +} from '../../linear/linear-client.js'; + +const ADAPTER = 'linear' as const; +const USER_AGENT = 'knotrack-mcp-server'; + +export interface LinearSyncOutput extends Record { + ok: boolean; + error?: string; +} + +export interface SyncToLinearDeps { + linearClientFactory?: LinearClientFactory; +} + +interface StateConfig { + doneStateId?: string; + openStateId?: string; +} export async function syncToLinearService( pool: Pool, - _config: Config, + config: Config, input: SyncToLinearInput, -): Promise { - return syncAdapterStub(pool, 'linear', input); + deps: SyncToLinearDeps = {}, +): Promise { + const clientFactory = deps.linearClientFactory ?? createFetchLinearClient; + + // 1. Project + project-scoped track existence. + const project = await findActiveProjectById(pool, input.project_id); + if (!project) { + throw notFound('project not found', { project_id: input.project_id }); + } + const track = await findTrackById(pool, input.project_id, input.track_id); + if (!track) { + throw notFound('track not found', { + project_id: input.project_id, + track_id: input.track_id, + }); + } + + // 2. Adapter precondition -> CONFLICT (no HTTP). §4.14. + const adapter = await getAdapterForProject(pool, input.project_id, ADAPTER); + if (!adapter) { + throw conflict('linear adapter not configured', { + project_id: input.project_id, + adapter: ADAPTER, + }); + } + const teamRaw = adapter.config['team_id']; + const teamId = typeof teamRaw === 'string' ? teamRaw.trim() : ''; + if (!teamId) { + throw conflict('linear adapter has no team configured', { + project_id: input.project_id, + adapter: ADAPTER, + }); + } + const stateConfig: StateConfig = { + doneStateId: readOptionalString(adapter.config['done_state_id']), + openStateId: readOptionalString(adapter.config['open_state_id']), + }; + + // 3. Decrypt the stored credential (never leaks the key). + const apiKey = decryptCredential(adapter.encrypted_credential, config.encryptionKey); + + // 4. Deterministic payload + no-op hash from ONE consistent snapshot. + const snap = await withReadSnapshot(pool, async (c) => { + const asOf = (await c.query<{ t: Date }>('SELECT now() AS t')).rows[0]!.t; + const t = await findTrackById(c, input.project_id, input.track_id); + return { asOf, track: t, items: t ? await listItemsByTrack(c, t.id) : [] }; + }); + if (!snap.track) { + throw notFound('track not found', { + project_id: input.project_id, + track_id: input.track_id, + }); + } + const syncedAt = snap.asOf; + const payload = buildLinearPayload( + { id: snap.track.id, title: snap.track.title, status: snap.track.status }, + snap.items.map((i) => ({ + id: i.id, + title: i.title, + status: i.status, + sequence_position: i.sequence_position, + })), + ); + const trackStatus = snap.track.status; + const stateIntent = stateIntentFor(trackStatus); + const hash = linearPayloadContentHash(payload, stateIntent, stateConfig); + + const client = clientFactory(apiKey, { + timeoutMs: config.linearSyncTimeoutMs, + userAgent: USER_AGENT, + }); + + // 5. Decide under a row lock; claim a fresh pending slot if none exists. + const decision = await withTransaction(pool, async (c): Promise => { + const existing = await getLinkForUpdate(c, track.id, ADAPTER); + if (existing) { + if (existing.sync_state === 'pending') return { kind: 'pending', row: existing }; + if (existing.repo_identity !== teamId) return { kind: 'team_changed', row: existing }; + if (existing.content_hash === hash) return { kind: 'noop' }; + return { kind: 'update', row: existing }; + } + // No link yet -> a create. We deliberately do NOT claim the pending row + // here: the Linear workflow-state read happens first (in createFresh), so a + // crash during that read cannot leave a wedged pending row for a create + // that was never attempted. The claim (INSERT ... ON CONFLICT DO NOTHING) + // still commits BEFORE the outbound mutation, preserving the durable + // creation-intent, and its ON CONFLICT is the cross-process race guard. + return { kind: 'create' }; + }); + + const ctx: SyncCtx = { + pool, + client, + teamId, + trackId: track.id, + payload, + hash, + syncedAt, + trackStatus, + stateConfig, + }; + + switch (decision.kind) { + case 'noop': + return okSynced(pool, track.id, syncedAt); + case 'team_changed': + return { + ok: false, + error: `LINEAR_UNKNOWN_ERROR: this track is linked to an issue in Linear team ${decision.row.repo_identity}, but the adapter is now configured for team ${teamId}; refusing to update a different team`, + }; + case 'update': + return updateExisting(ctx, decision.row); + case 'create': + return createFresh(ctx); + case 'pending': + return recoverPending(ctx, decision.row); + } +} + +function readOptionalString(value: unknown): string | undefined { + if (typeof value !== 'string') return undefined; + const trimmed = value.trim(); + return trimmed.length > 0 ? trimmed : undefined; +} + +interface SyncCtx { + pool: Pool; + client: LinearClient; + teamId: string; + trackId: string; + payload: LinearPayload; + hash: string; + syncedAt: Date; + trackStatus: string; + stateConfig: StateConfig; +} + +type Decision = + | { kind: 'noop' } + | { kind: 'team_changed'; row: TrackExternalLinkRow } + | { kind: 'update'; row: TrackExternalLinkRow } + | { kind: 'create' } + | { kind: 'pending'; row: TrackExternalLinkRow }; + +async function okSynced(pool: Pool, trackId: string, syncedAt: Date): Promise { + await withTransaction(pool, (c) => touchLinearSyncWatermark(c, trackId, syncedAt)); + return { ok: true }; +} + +/** Resolves the target stateId for a sync, fetching the team's workflow + * states (and, only for a non-done reopen with an override, the issue's + * current state) as needed. Returns { ok:false } with a LINEAR_* error for a + * transport failure or an invalid state configuration. `stateId` undefined + * means "leave the workflow state as-is / let Linear default it". */ +async function resolveState( + ctx: SyncCtx, + mode: 'create' | 'update', + issueId?: string, +): Promise<{ ok: true; stateId?: string } | { ok: false; error: string }> { + // Fetch states when a stateId might be set (done track, or an open_state_id + // reopen) OR when ANY override is configured — so a misconfigured + // done_state_id/open_state_id is validated and surfaced eagerly (Codex + // PR #25), not silently accepted until the track later becomes done. + const needState = + ctx.trackStatus === 'done' || + ctx.stateConfig.openStateId !== undefined || + ctx.stateConfig.doneStateId !== undefined; + if (!needState) return { ok: true }; + + const statesRes = await ctx.client.getWorkflowStates(ctx.teamId); + if (!statesRes.ok) return { ok: false, error: statesRes.error }; + + let currentStateType: string | null | undefined; + if ( + mode === 'update' && + ctx.trackStatus !== 'done' && + ctx.stateConfig.openStateId !== undefined && + issueId + ) { + const st = await ctx.client.getIssueStateType(issueId); + if (!st.ok) return { ok: false, error: st.error }; + currentStateType = st.value; + } + + const resolved = resolveLinearStateId({ + states: statesRes.value, + doneStateId: ctx.stateConfig.doneStateId, + openStateId: ctx.stateConfig.openStateId, + trackStatus: ctx.trackStatus, + mode, + currentStateType, + }); + if ('error' in resolved) return { ok: false, error: resolved.error }; + return { ok: true, stateId: resolved.stateId }; +} + +async function createFresh(ctx: SyncCtx): Promise { + // Resolve the target workflow state BEFORE committing the durable pending + // intent. This Linear-only network read has no side effect, so a crash or a + // config/transport failure during it leaves NO pending row for a create that + // was never attempted (Codex PR #25) — no wedge, clean retry. + const state = await resolveState(ctx, 'create'); + if (!state.ok) return { ok: false, error: state.error }; + + // Claim the durable pending intent now, BEFORE the outbound mutation. The + // UNIQUE(track_id, adapter_type) ON CONFLICT DO NOTHING is the cross-process + // race guard: exactly one concurrent sync wins the claim. + const operationId = randomUUID(); + const claimed = await withTransaction(ctx.pool, (c) => + claimPendingLink(c, { + trackId: ctx.trackId, + adapterType: ADAPTER, + repoIdentity: ctx.teamId, + operationId, + }), + ); + if (!claimed) { + return { + ok: false, + error: 'LINEAR_UNKNOWN_ERROR: another sync for this track is in progress; retry shortly', + }; + } + + const created = await ctx.client.createIssue(ctx.teamId, ctx.payload, state.stateId); + if (!created.ok) { + if (!created.ambiguous) { + await withTransaction(ctx.pool, (c) => + clearPendingLink(c, { trackId: ctx.trackId, adapterType: ADAPTER, operationId }), + ); + } + return { ok: false, error: created.error }; + } + const ref = created.value; + const finalized = await withTransaction(ctx.pool, (c) => + finalizeLinked(c, { + trackId: ctx.trackId, + adapterType: ADAPTER, + operationId, + externalId: ref.id, + externalUrl: ref.url, + contentHash: ctx.hash, + }), + ); + if (!finalized) { + return { + ok: false, + error: + 'LINEAR_UNKNOWN_ERROR: issue created but its link could not be finalized; reconcile manually', + }; + } + return okSynced(ctx.pool, ctx.trackId, ctx.syncedAt); +} + +async function updateExisting(ctx: SyncCtx, row: TrackExternalLinkRow): Promise { + const issueId = row.external_id; + if (!issueId) { + return { + ok: false, + error: 'LINEAR_UNKNOWN_ERROR: linked row is missing its issue id; reconcile manually', + }; + } + const state = await resolveState(ctx, 'update', issueId); + if (!state.ok) return { ok: false, error: state.error }; + + // As with GitHub, we do NOT hold a DB lock across this network call: the + // decision-transaction row lock is already released. issueUpdate is + // idempotent, there is no duplicate/lost issue, and a momentarily-stale + // content_hash self-corrects on the next sync. + const res = await ctx.client.updateIssue(issueId, ctx.payload, state.stateId); + if (!res.ok) return { ok: false, error: res.error }; + await withTransaction(ctx.pool, (c) => + updateLinkedContentHash(c, { + trackId: ctx.trackId, + adapterType: ADAPTER, + contentHash: ctx.hash, + // Refresh the stored URL: a Linear issue's URL can change while its id + // stays stable (team-key/workspace-slug change). + externalUrl: res.value.url, + }), + ); + return okSynced(ctx.pool, ctx.trackId, ctx.syncedAt); +} + +async function recoverPending(ctx: SyncCtx, row: TrackExternalLinkRow): Promise { + if (row.repo_identity !== ctx.teamId) { + return { + ok: false, + error: `LINEAR_UNKNOWN_ERROR: a sync is pending for this track against Linear team ${row.repo_identity}, but the adapter is now configured for team ${ctx.teamId}; resolve the team change before retrying`, + }; + } + const found = await ctx.client.findIssueByMarker(ctx.teamId, trackMarker(ctx.trackId)); + if (!found.ok) return { ok: false, error: found.error }; + + if (found.value) { + // The earlier create DID happen. Reconcile the found issue to current + // content (and state), then adopt it as linked. + const ref = found.value; + const state = await resolveState(ctx, 'update', ref.id); + if (!state.ok) return { ok: false, error: state.error }; + const res = await ctx.client.updateIssue(ref.id, ctx.payload, state.stateId); + if (!res.ok) return { ok: false, error: res.error }; + const finalized = await withTransaction(ctx.pool, (c) => + finalizeLinked(c, { + trackId: ctx.trackId, + adapterType: ADAPTER, + operationId: row.operation_id, + externalId: res.value.id, + externalUrl: res.value.url, + contentHash: ctx.hash, + }), + ); + if (!finalized) { + return { + ok: false, + error: 'LINEAR_UNKNOWN_ERROR: adopted issue could not be finalized; reconcile manually', + }; + } + return okSynced(ctx.pool, ctx.trackId, ctx.syncedAt); + } + + // Marker not found. A pending link only survives an AMBIGUOUS create, so + // the outcome is unknown by construction; per the T5.2 design decision + // (duplicate prevention takes priority over automatic retry when the + // outcome cannot be established), we do NOT auto-recreate. + return { + ok: false, + error: + 'LINEAR_UNKNOWN_ERROR: a previous sync did not complete and no matching issue could be found; its outcome cannot be safely confirmed, so this track is not auto-recreated (that would risk a duplicate). Clear the pending link (or use a relink tool) to resync.', + }; } export function registerSyncToLinearTool( @@ -25,7 +422,7 @@ export function registerSyncToLinearTool( { title: 'Sync to Linear', description: - 'Pushes a track to a linked Linear Issue. Requires a configured Linear adapter; external sync is not available in this build.', + 'Creates or updates a Linear Issue for a track (idempotent). Requires a configured Linear adapter with a team; operational Linear failures return { ok: false, error } rather than throwing.', inputSchema: syncToLinearInputSchema, }, async (rawArgs: unknown) => { diff --git a/src/schemas/tools.ts b/src/schemas/tools.ts index 5f7a9b5..26d86e9 100644 --- a/src/schemas/tools.ts +++ b/src/schemas/tools.ts @@ -23,6 +23,18 @@ const linearAdapterInput = z .object({ api_key: z.string().min(1).max(512), team_id: z.string().min(1).max(200), + // Optional workflow-state overrides (T5.3). When omitted, sync auto-picks + // the team's lowest-position 'completed' state for a done track and never + // moves a non-done track's issue backward. When set, they pin exact Linear + // WorkflowState UUIDs: done_state_id must be a 'completed'-type state; + // open_state_id is the state a reopened (no-longer-done) track's issue is + // moved to and must be a non-terminal (backlog/unstarted/started) state + // (a 'completed' or 'canceled' state is rejected with LINEAR_STATE_CONFIG). + // `.trim()` before `.min(1)` so a whitespace-only override is rejected at + // registration (a blank string is not a valid state id) rather than being + // silently coerced to "unset" and falling back to auto behavior (Codex PR #25). + done_state_id: z.string().trim().min(1).max(200).optional(), + open_state_id: z.string().trim().min(1).max(200).optional(), }) .strict(); diff --git a/tests/integration/sync-to-linear.test.ts b/tests/integration/sync-to-linear.test.ts index 9d012d9..da37107 100644 --- a/tests/integration/sync-to-linear.test.ts +++ b/tests/integration/sync-to-linear.test.ts @@ -1,97 +1,468 @@ +import { randomUUID } from 'node:crypto'; import { afterAll, beforeEach, describe, expect, it } from 'vitest'; -import { syncToLinearService } from '../../src/mcp/tools/sync-to-linear.js'; +import { syncToLinearService, type SyncToLinearDeps } from '../../src/mcp/tools/sync-to-linear.js'; +import { getTrackService } from '../../src/mcp/tools/get-track.js'; import { registerProjectService } from '../../src/mcp/tools/register-project.js'; import { createTrackService } from '../../src/mcp/tools/create-track.js'; +import { createItemService } from '../../src/mcp/tools/create-item.js'; +import { updateItemStatusService } from '../../src/mcp/tools/update-item-status.js'; import { upsertAdapter } from '../../src/db/queries/adapters.js'; +import { encryptCredential } from '../../src/crypto/credential-cipher.js'; +import type { + LinearClient, + LinearIssuePayload, + LinearIssueRef, + LinearResult, + LinearWorkflowState, +} from '../../src/linear/linear-client.js'; import { closeTestPool, getTestConfig, getTestPool, truncateAll, UNKNOWN_UUID } from './helpers.js'; const pool = getTestPool(); const config = getTestConfig(); +const TEAM = 'team-1'; -async function makeProjectAndTrack(): Promise<{ project_id: string; track_id: string }> { +const DEFAULT_STATES: LinearWorkflowState[] = [ + { id: 's-todo', name: 'Todo', type: 'unstarted', position: 1 }, + { id: 's-doing', name: 'Doing', type: 'started', position: 2 }, + { id: 's-done', name: 'Done', type: 'completed', position: 3 }, + { id: 's-released', name: 'Released', type: 'completed', position: 5 }, + { id: 's-cancel', name: 'Cancelled', type: 'canceled', position: 6 }, +]; + +// ---- fake Linear client ------------------------------------------------ +interface FakeState { + createCalls: Array<{ teamId: string; stateId?: string; payload: LinearIssuePayload }>; + updateCalls: Array<{ issueId: string; stateId?: string }>; + findCalls: Array<{ teamId: string; marker: string }>; + statesCalls: number; + nextId: number; +} +interface FakeOverrides { + createIssue?: (s: FakeState) => LinearResult; + updateIssue?: (s: FakeState, issueId: string) => LinearResult; + findIssueByMarker?: (s: FakeState) => LinearResult; + getWorkflowStates?: () => LinearResult; + getIssueStateType?: () => LinearResult; +} +function makeFake(overrides: FakeOverrides = {}) { + const state: FakeState = { + createCalls: [], + updateCalls: [], + findCalls: [], + statesCalls: 0, + nextId: 1, + }; + const client: LinearClient = { + createIssue(teamId, payload, stateId) { + state.createCalls.push({ teamId, stateId, payload }); + const r: LinearResult = overrides.createIssue + ? overrides.createIssue(state) + : { + ok: true, + value: { + id: `iss-${state.nextId++}`, + identifier: `ENG-${state.nextId - 1}`, + url: `https://linear.app/x/issue/ENG-${state.nextId - 1}`, + }, + }; + return Promise.resolve(r); + }, + updateIssue(issueId, _payload, stateId) { + state.updateCalls.push({ issueId, stateId }); + const r: LinearResult = overrides.updateIssue + ? overrides.updateIssue(state, issueId) + : { + ok: true, + value: { + id: issueId, + identifier: 'ENG-1', + url: `https://linear.app/x/issue/${issueId}`, + }, + }; + return Promise.resolve(r); + }, + findIssueByMarker(teamId, marker) { + state.findCalls.push({ teamId, marker }); + const r: LinearResult = overrides.findIssueByMarker + ? overrides.findIssueByMarker(state) + : { ok: true, value: null }; + return Promise.resolve(r); + }, + getWorkflowStates() { + state.statesCalls += 1; + const r: LinearResult = overrides.getWorkflowStates + ? overrides.getWorkflowStates() + : { ok: true, value: DEFAULT_STATES }; + return Promise.resolve(r); + }, + getIssueStateType() { + const r: LinearResult = overrides.getIssueStateType + ? overrides.getIssueStateType() + : { ok: true, value: 'completed' }; + return Promise.resolve(r); + }, + }; + const deps: SyncToLinearDeps = { linearClientFactory: () => client }; + return { state, deps }; +} + +async function makeProjectTrack( + opts: { + withLinear?: boolean; + teamId?: string | null; + doneStateId?: string; + openStateId?: string; + } = {}, +) { + const withLinear = opts.withLinear ?? true; + const adapters = withLinear + ? { + linear: { + api_key: 'lin_test_secret', + team_id: opts.teamId ?? TEAM, + done_state_id: opts.doneStateId, + open_state_id: opts.openStateId, + }, + } + : undefined; const { project_id } = await registerProjectService(pool, config, { name: 'P', source_type: 'local', - source_ref: `/tmp/${crypto.randomUUID()}`, - adapters: undefined, + source_ref: `/tmp/${randomUUID()}`, + adapters, }); const { track_id } = await createTrackService(pool, config, { project_id, - title: 'T', + title: 'Auth overhaul', depends_on: [], source_doc_ref: undefined, }); return { project_id, track_id }; } +async function markTrackDone(project_id: string, track_id: string) { + const item = await createItemService(pool, config, { + project_id, + track_id, + title: 'only item', + sequence_position: undefined, + depends_on: [], + }); + await updateItemStatusService(pool, config, { + project_id, + item_id: item.item_id, + status: 'done', + }); +} + +async function linkRow(trackId: string) { + const r = await pool.query( + `SELECT * FROM track_external_links WHERE track_id = $1 AND adapter_type = 'linear'`, + [trackId], + ); + return r.rows[0] as + { sync_state: string; external_id: string | null; external_url: string | null } | undefined; +} + beforeEach(async () => { await truncateAll(); }); - afterAll(async () => { await closeTestPool(); }); -describe('kt_sync_to_linear (T2.14 stub)', () => { - it('negative: CONFLICT (linear-specific) when no linear adapter is configured, even with a github adapter present', async () => { - const { project_id, track_id } = await makeProjectAndTrack(); +describe('kt_sync_to_linear — preconditions (PRD §4.14)', () => { + it('CONFLICT when no linear adapter is configured', async () => { + const { project_id, track_id } = await makeProjectTrack({ withLinear: false }); + await expect(syncToLinearService(pool, config, { project_id, track_id })).rejects.toMatchObject( + { + code: 'CONFLICT', + message: 'linear adapter not configured', + details: { adapter: 'linear' }, + }, + ); + }); + + it('CONFLICT when the linear adapter has no team configured', async () => { + const { project_id, track_id } = await makeProjectTrack({ withLinear: false }); await upsertAdapter(pool, { projectId: project_id, - type: 'github', - encryptedCredential: Buffer.from('github-secret'), + type: 'linear', + encryptedCredential: encryptCredential('lin_test_secret', config.encryptionKey), config: {}, }); await expect(syncToLinearService(pool, config, { project_id, track_id })).rejects.toMatchObject( { code: 'CONFLICT', - message: 'linear adapter not configured', - details: { adapter: 'linear' }, + message: 'linear adapter has no team configured', }, ); }); - it('negative: 404 when the project does not exist', async () => { - const { track_id } = await makeProjectAndTrack(); + it('404 when the project does not exist', async () => { + const { track_id } = await makeProjectTrack(); await expect( syncToLinearService(pool, config, { project_id: UNKNOWN_UUID, track_id }), ).rejects.toMatchObject({ code: 'NOT_FOUND' }); }); - it('negative: 404 when the track does not exist in the project', async () => { - const { project_id } = await makeProjectAndTrack(); + it('404 when the track belongs to another project (no cross-project leak)', async () => { + const other = await makeProjectTrack(); + const { project_id } = await makeProjectTrack(); await expect( - syncToLinearService(pool, config, { project_id, track_id: UNKNOWN_UUID }), + syncToLinearService(pool, config, { project_id, track_id: other.track_id }), ).rejects.toMatchObject({ code: 'NOT_FOUND' }); }); +}); - it('negative: 404 when the track belongs to a different project (no cross-project leak)', async () => { - const other = await makeProjectAndTrack(); - const { project_id } = await makeProjectAndTrack(); - await expect( - syncToLinearService(pool, config, { project_id, track_id: other.track_id }), - ).rejects.toMatchObject({ code: 'NOT_FOUND' }); +describe('kt_sync_to_linear — create / update / no-op', () => { + it('creates an issue, links it, and surfaces the URL on kt_get_track', async () => { + const { project_id, track_id } = await makeProjectTrack(); + const { state, deps } = makeFake(); + + const res = await syncToLinearService(pool, config, { project_id, track_id }, deps); + expect(res).toEqual({ ok: true }); + expect(state.createCalls).toHaveLength(1); + // non-done track, no override -> no workflow-state lookup, no stateId set + expect(state.statesCalls).toBe(0); + expect(state.createCalls[0]!.stateId).toBeUndefined(); + + const row = await linkRow(track_id); + expect(row?.sync_state).toBe('linked'); + expect(row?.external_id).toBe('iss-1'); + + const track = await getTrackService(pool, config, { project_id, track_id }); + expect(track.track.linear_issue_url).toBe('https://linear.app/x/issue/ENG-1'); + + const wm = await pool.query(`SELECT last_linear_sync_at FROM tracks WHERE id = $1`, [track_id]); + expect(wm.rows[0].last_linear_sync_at).not.toBeNull(); + }); + + it('is a no-op on an unchanged re-sync (no second mutation)', async () => { + const { project_id, track_id } = await makeProjectTrack(); + const { state, deps } = makeFake(); + await syncToLinearService(pool, config, { project_id, track_id }, deps); + const res = await syncToLinearService(pool, config, { project_id, track_id }, deps); + expect(res).toEqual({ ok: true }); + expect(state.createCalls).toHaveLength(1); + expect(state.updateCalls).toHaveLength(0); }); - // Reachable through the public kt_register_project path (it provisions - // the adapter from inline credentials), not just fixtures/manual SQL. - it('edge: INTERNAL_ERROR "not available in this build" when a linear adapter is configured via kt_register_project', async () => { - const { project_id } = await registerProjectService(pool, config, { - name: 'P', - source_type: 'local', - source_ref: `/tmp/${crypto.randomUUID()}`, - adapters: { linear: { api_key: 'lin_test_secret', team_id: 'team_123' } }, + it('updates the existing issue when the track content changes', async () => { + const { project_id, track_id } = await makeProjectTrack(); + const { state, deps } = makeFake(); + await syncToLinearService(pool, config, { project_id, track_id }, deps); + await createItemService(pool, config, { + project_id, + track_id, + title: 'New item', + sequence_position: undefined, + depends_on: [], }); - const { track_id } = await createTrackService(pool, config, { + const res = await syncToLinearService(pool, config, { project_id, track_id }, deps); + expect(res).toEqual({ ok: true }); + expect(state.createCalls).toHaveLength(1); + expect(state.updateCalls).toHaveLength(1); + expect(state.updateCalls[0]!.issueId).toBe('iss-1'); + }); + + it('refreshes the stored linear_issue_url when an update returns a changed URL', async () => { + const { project_id, track_id } = await makeProjectTrack(); + await syncToLinearService(pool, config, { project_id, track_id }, makeFake().deps); + // change content so an update runs, and have the update return a new URL + await createItemService(pool, config, { project_id, - title: 'T', + track_id, + title: 'New item', + sequence_position: undefined, depends_on: [], - source_doc_ref: undefined, }); - await expect(syncToLinearService(pool, config, { project_id, track_id })).rejects.toMatchObject( - { - code: 'INTERNAL_ERROR', - message: 'linear sync is not available in this build', - }, + const { deps } = makeFake({ + updateIssue: (_s, issueId) => ({ + ok: true, + value: { id: issueId, identifier: 'ENG-1', url: 'https://linear.app/new-slug/issue/ENG-1' }, + }), + }); + const res = await syncToLinearService(pool, config, { project_id, track_id }, deps); + expect(res).toEqual({ ok: true }); + const track = await getTrackService(pool, config, { project_id, track_id }); + expect(track.track.linear_issue_url).toBe('https://linear.app/new-slug/issue/ENG-1'); + }); +}); + +describe('kt_sync_to_linear — workflow-state resolution', () => { + it('sets the lowest-position completed state on create for a done track (auto)', async () => { + const { project_id, track_id } = await makeProjectTrack(); + await markTrackDone(project_id, track_id); + const { state, deps } = makeFake(); + const res = await syncToLinearService(pool, config, { project_id, track_id }, deps); + expect(res).toEqual({ ok: true }); + expect(state.statesCalls).toBeGreaterThanOrEqual(1); + // 's-done' (position 3) beats 's-released' (position 5) + expect(state.createCalls[0]!.stateId).toBe('s-done'); + }); + + it('honors a configured done_state_id override', async () => { + const { project_id, track_id } = await makeProjectTrack({ doneStateId: 's-released' }); + await markTrackDone(project_id, track_id); + const { state, deps } = makeFake(); + const res = await syncToLinearService(pool, config, { project_id, track_id }, deps); + expect(res).toEqual({ ok: true }); + expect(state.createCalls[0]!.stateId).toBe('s-released'); + }); + + it('fails LINEAR_STATE_CONFIG when done_state_id is not a completed state (and clears pending)', async () => { + const { project_id, track_id } = await makeProjectTrack({ doneStateId: 's-doing' }); + await markTrackDone(project_id, track_id); + const { state, deps } = makeFake(); + const res = await syncToLinearService(pool, config, { project_id, track_id }, deps); + expect(res.ok).toBe(false); + expect(String(res.error)).toMatch(/^LINEAR_STATE_CONFIG/); + expect(state.createCalls).toHaveLength(0); + expect(await linkRow(track_id)).toBeUndefined(); // pending cleared + }); + + it('validates an override eagerly even for a non-done track (no pending row wedged)', async () => { + // A non-done track with a bad done_state_id must be rejected up front, not + // silently accepted until the track later becomes done. + const { project_id, track_id } = await makeProjectTrack({ doneStateId: 's-doing' }); + const { state, deps } = makeFake(); + const res = await syncToLinearService(pool, config, { project_id, track_id }, deps); + expect(res.ok).toBe(false); + expect(String(res.error)).toMatch(/^LINEAR_STATE_CONFIG/); + expect(state.createCalls).toHaveLength(0); + // resolution failed BEFORE any claim -> no pending row left behind + expect(await linkRow(track_id)).toBeUndefined(); + }); + + it('fails LINEAR_STATE_CONFIG when the team has no completed state', async () => { + const { project_id, track_id } = await makeProjectTrack(); + await markTrackDone(project_id, track_id); + const { deps } = makeFake({ + getWorkflowStates: () => ({ + ok: true, + value: DEFAULT_STATES.filter((s) => s.type !== 'completed'), + }), + }); + const res = await syncToLinearService(pool, config, { project_id, track_id }, deps); + expect(res.ok).toBe(false); + expect(String(res.error)).toMatch(/no 'completed' workflow state/); + }); +}); + +describe('kt_sync_to_linear — operational failures ({ok:false}, never throw)', () => { + it('passes through a definitive failure and clears the pending claim', async () => { + const { project_id, track_id } = await makeProjectTrack(); + const { deps } = makeFake({ + createIssue: () => ({ + ok: false, + error: 'LINEAR_UNKNOWN_ERROR: bad input', + ambiguous: false, + }), + }); + const res = await syncToLinearService(pool, config, { project_id, track_id }, deps); + expect(res.ok).toBe(false); + expect(String(res.error)).toMatch(/^LINEAR_UNKNOWN_ERROR/); + expect(await linkRow(track_id)).toBeUndefined(); + }); + + it('keeps the pending claim after an ambiguous (timeout) failure', async () => { + const { project_id, track_id } = await makeProjectTrack(); + const { deps } = makeFake({ + createIssue: () => ({ ok: false, error: 'LINEAR_TIMEOUT: too slow', ambiguous: true }), + }); + const res = await syncToLinearService(pool, config, { project_id, track_id }, deps); + expect(res.ok).toBe(false); + expect(String(res.error)).toMatch(/^LINEAR_TIMEOUT/); + expect((await linkRow(track_id))?.sync_state).toBe('pending'); + }); + + it('refuses to update when the adapter team changed under an existing link', async () => { + const { project_id, track_id } = await makeProjectTrack(); + const first = makeFake(); + await syncToLinearService(pool, config, { project_id, track_id }, first.deps); + await upsertAdapter(pool, { + projectId: project_id, + type: 'linear', + encryptedCredential: encryptCredential('lin_test_secret', config.encryptionKey), + config: { team_id: 'team-OTHER' }, + }); + const { state, deps } = makeFake(); + const res = await syncToLinearService(pool, config, { project_id, track_id }, deps); + expect(res.ok).toBe(false); + expect(String(res.error)).toMatch(/refusing to update a different team/); + expect(state.updateCalls).toHaveLength(0); + expect(state.createCalls).toHaveLength(0); + }); +}); + +describe('kt_sync_to_linear — durable creation-intent / recovery', () => { + it('concurrent syncs create exactly one issue (no duplicate)', async () => { + const { project_id, track_id } = await makeProjectTrack(); + const { state, deps } = makeFake(); + const [a, b] = await Promise.all([ + syncToLinearService(pool, config, { project_id, track_id }, deps), + syncToLinearService(pool, config, { project_id, track_id }, deps), + ]); + expect(state.createCalls).toHaveLength(1); + const rows = await pool.query( + `SELECT count(*)::int AS n FROM track_external_links WHERE track_id = $1 AND adapter_type='linear'`, + [track_id], + ); + expect(rows.rows[0].n).toBe(1); + expect([a.ok, b.ok]).toContain(true); + }); + + it('recovers a crashed pending link by adopting the issue the marker finds', async () => { + const { project_id, track_id } = await makeProjectTrack(); + await pool.query( + `INSERT INTO track_external_links (track_id, adapter_type, sync_state, repo_identity, operation_id) + VALUES ($1, 'linear', 'pending', $2, $3)`, + [track_id, TEAM, randomUUID()], + ); + const { state, deps } = makeFake({ + findIssueByMarker: () => ({ + ok: true, + value: { id: 'iss-42', identifier: 'ENG-42', url: 'https://linear.app/x/issue/ENG-42' }, + }), + }); + const res = await syncToLinearService(pool, config, { project_id, track_id }, deps); + expect(res).toEqual({ ok: true }); + expect(state.findCalls).toHaveLength(1); + expect(state.createCalls).toHaveLength(0); // adopted, not recreated + const row = await linkRow(track_id); + expect(row?.sync_state).toBe('linked'); + expect(row?.external_id).toBe('iss-42'); + }); + + it('never recreates on a marker miss — surfaces the pending link (no duplicate)', async () => { + const { project_id, track_id } = await makeProjectTrack(); + await pool.query( + `INSERT INTO track_external_links (track_id, adapter_type, sync_state, repo_identity, operation_id, created_at) + VALUES ($1, 'linear', 'pending', $2, $3, now() - interval '2 hours')`, + [track_id, TEAM, randomUUID()], + ); + const { state, deps } = makeFake(); // findIssueByMarker default -> null + const res = await syncToLinearService(pool, config, { project_id, track_id }, deps); + expect(res.ok).toBe(false); + expect(String(res.error)).toMatch(/not auto-recreated/); + expect(state.findCalls).toHaveLength(1); + expect(state.createCalls).toHaveLength(0); + expect((await linkRow(track_id))?.sync_state).toBe('pending'); + }); + + it('refuses recovery when the adapter team changed while a sync was pending', async () => { + const { project_id, track_id } = await makeProjectTrack(); + await pool.query( + `INSERT INTO track_external_links (track_id, adapter_type, sync_state, repo_identity, operation_id, created_at) + VALUES ($1, 'linear', 'pending', 'team-OLD', $2, now() - interval '5 minutes')`, + [track_id, randomUUID()], ); + const { state, deps } = makeFake(); + const res = await syncToLinearService(pool, config, { project_id, track_id }, deps); + expect(res.ok).toBe(false); + expect(String(res.error)).toMatch(/resolve the team change/); + expect(state.findCalls).toHaveLength(0); // never searched the wrong team + expect((await linkRow(track_id))?.sync_state).toBe('pending'); }); }); diff --git a/tests/unit/linear-client.test.ts b/tests/unit/linear-client.test.ts new file mode 100644 index 0000000..8f1d5bb --- /dev/null +++ b/tests/unit/linear-client.test.ts @@ -0,0 +1,490 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { createFetchLinearClient } from '../../src/linear/linear-client.js'; + +const KEY = 'lin_api_super_secret_value'; +const OPTS = { timeoutMs: 8000, userAgent: 'knotrack-mcp-server' }; +const TEAM = 'team-123'; + +function client() { + return createFetchLinearClient(KEY, OPTS); +} + +interface FakeResponse { + ok: boolean; + status: number; + headers: Headers; + text: () => Promise; +} +function resp(status: number, body: string, headers: Record = {}): FakeResponse { + return { + ok: status >= 200 && status < 300, + status, + headers: new Headers(headers), + text: () => Promise.resolve(body), + }; +} +interface FetchInit { + method: string; + headers: Record; + body: string; +} + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe('createFetchLinearClient — request shape', () => { + it('createIssue sends issueCreate with the RAW Authorization header (no Bearer)', async () => { + const fetchMock = vi.fn((_url: string, _init: FetchInit) => + Promise.resolve( + resp( + 200, + JSON.stringify({ + data: { + issueCreate: { success: true, issue: { id: 'i1', identifier: 'ENG-1', url: 'u1' } }, + }, + }), + ), + ), + ); + vi.stubGlobal('fetch', fetchMock); + + const res = await client().createIssue(TEAM, { title: 'T', description: 'D' }, 's-done'); + expect(res).toEqual({ ok: true, value: { id: 'i1', identifier: 'ENG-1', url: 'u1' } }); + + const [url, init] = fetchMock.mock.calls[0]!; + expect(url).toBe('https://api.linear.app/graphql'); + expect(init.method).toBe('POST'); + // RAW key, not "Bearer " + expect(init.headers.Authorization).toBe(KEY); + expect(init.headers.Authorization).not.toContain('Bearer'); + const parsed = JSON.parse(init.body); + expect(parsed.query).toContain('issueCreate'); + expect(parsed.variables.input).toEqual({ + teamId: TEAM, + title: 'T', + description: 'D', + stateId: 's-done', + }); + }); + + it('createIssue omits stateId when none is given', async () => { + const fetchMock = vi.fn((_url: string, _init: FetchInit) => + Promise.resolve( + resp( + 200, + JSON.stringify({ + data: { + issueCreate: { success: true, issue: { id: 'i1', identifier: 'E-1', url: 'u' } }, + }, + }), + ), + ), + ); + vi.stubGlobal('fetch', fetchMock); + await client().createIssue(TEAM, { title: 'T', description: 'D' }); + const parsed = JSON.parse(fetchMock.mock.calls[0]![1].body); + expect(parsed.variables.input).not.toHaveProperty('stateId'); + }); + + it('updateIssue sends issueUpdate at the issue id with stateId', async () => { + const fetchMock = vi.fn((_url: string, _init: FetchInit) => + Promise.resolve( + resp( + 200, + JSON.stringify({ + data: { + issueUpdate: { success: true, issue: { id: 'i1', identifier: 'E-1', url: 'u' } }, + }, + }), + ), + ), + ); + vi.stubGlobal('fetch', fetchMock); + await client().updateIssue('i1', { title: 'T', description: 'D' }, 's-done'); + const parsed = JSON.parse(fetchMock.mock.calls[0]![1].body); + expect(parsed.query).toContain('issueUpdate'); + expect(parsed.variables).toEqual({ + id: 'i1', + input: { title: 'T', description: 'D', stateId: 's-done' }, + }); + }); +}); + +describe('createFetchLinearClient — error mapping', () => { + it('maps a GraphQL 200-with-errors on a MUTATION to ambiguous (may have committed)', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => + Promise.resolve(resp(200, JSON.stringify({ errors: [{ message: 'bad input' }] }))), + ), + ); + const res = await client().createIssue(TEAM, { title: 'T', description: 'D' }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.error).toMatch(/^LINEAR_UNKNOWN_ERROR/); + // A mutation's opaque GraphQL error is NOT proof no write happened. + expect(res.ambiguous).toBe(true); + } + }); + + it('maps a GraphQL 200-with-errors on a READ to non-ambiguous', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => + Promise.resolve(resp(200, JSON.stringify({ errors: [{ message: 'bad query' }] }))), + ), + ); + const res = await client().getWorkflowStates(TEAM); + expect(res.ok).toBe(false); + if (!res.ok) expect(res.ambiguous).toBe(false); + }); + + it('keeps a 5xx AMBIGUOUS even when it carries a Retry-After header', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => Promise.resolve(resp(503, 'unavailable', { 'retry-after': '5' }))), + ); + const res = await client().createIssue(TEAM, { title: 'T', description: 'D' }); + expect(res.ok).toBe(false); + if (!res.ok) { + // Must NOT be reclassified as a definitive rate-limit. + expect(res.error).toMatch(/^LINEAR_UNKNOWN_ERROR/); + expect(res.ambiguous).toBe(true); + } + }); + + it('treats an explicit success:false mutation payload as DEFINITIVE (not ambiguous)', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => + Promise.resolve( + resp(200, JSON.stringify({ data: { issueCreate: { success: false, issue: null } } })), + ), + ), + ); + const res = await client().createIssue(TEAM, { title: 'T', description: 'D' }); + expect(res.ok).toBe(false); + if (!res.ok) expect(res.ambiguous).toBe(false); + }); + + it('maps a GraphQL entity-not-found error on a MUTATION to LINEAR_NOT_FOUND, ambiguous', async () => { + // The NOT_FOUND prefix is still surfaced, but on a MUTATION a 200 GraphQL + // error cannot prove no write occurred, so it stays ambiguous and the pending + // link is kept for marker recovery, not cleared (CodeRabbit + panel, PR #25). + vi.stubGlobal( + 'fetch', + vi.fn(() => + Promise.resolve( + resp( + 200, + JSON.stringify({ + errors: [ + { message: 'Entity not found: Issue', extensions: { code: 'ENTITY_NOT_FOUND' } }, + ], + }), + ), + ), + ), + ); + const res = await client().updateIssue('i1', { title: 'T', description: 'D' }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.error).toMatch(/^LINEAR_NOT_FOUND/); + expect(res.ambiguous).toBe(true); + } + }); + + it('keeps mutation GraphQL rate-limit/auth/not-found AMBIGUOUS (200 cannot prove no write -> no duplicate)', async () => { + // A 200 GraphQL error on a mutation is never proof of pre-execution rejection, + // so the pending claim is kept (CodeRabbit + frontier panel, PR #25). The + // prefix is still surfaced. HTTP-status forms of these stay definitive in + // mapHttpError (see the HTTP 401/429 tests). + for (const message of ['rate limited', 'authentication failed', 'Entity not found']) { + vi.stubGlobal( + 'fetch', + vi.fn(() => Promise.resolve(resp(200, JSON.stringify({ errors: [{ message }] })))), + ); + const res = await client().createIssue(TEAM, { title: 'T', description: 'D' }); + expect(res.ok).toBe(false); + if (!res.ok) expect(res.ambiguous).toBe(true); + } + }); + + it('keeps a mutation GraphQL OPAQUE error ambiguous (may have followed a side effect)', async () => { + // The reconciliation: only a generic/unrecognized mutation error can legally + // follow a write, so it alone stays ambiguous and keeps the pending link. + vi.stubGlobal( + 'fetch', + vi.fn(() => + Promise.resolve( + resp(200, JSON.stringify({ errors: [{ message: 'internal server error' }] })), + ), + ), + ); + const res = await client().createIssue(TEAM, { title: 'T', description: 'D' }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.error).toMatch(/^LINEAR_UNKNOWN_ERROR/); + expect(res.ambiguous).toBe(true); + } + }); + + it('leaves the same GraphQL errors NON-ambiguous on a read (a read writes nothing)', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => + Promise.resolve(resp(200, JSON.stringify({ errors: [{ message: 'Entity not found' }] }))), + ), + ); + const res = await client().getWorkflowStates(TEAM); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.error).toMatch(/^LINEAR_NOT_FOUND/); + expect(res.ambiguous).toBe(false); + } + }); + + it('redacts a boundary-straddling secret BEFORE truncation (HTTP body and GraphQL message)', async () => { + // The secret sits so the 300-char cut lands mid-key. Redaction must run on + // the FULL upstream text first, or a partial credential survives truncation + // (Codex PR #25). 'lin_api_su' is the prefix an unredacted slice would keep. + const pad = 'x'.repeat(290); + // HTTP body path (5xx). + vi.stubGlobal( + 'fetch', + vi.fn(() => Promise.resolve(resp(500, pad + KEY))), + ); + let res = await client().createIssue(TEAM, { title: 'T', description: 'D' }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.error).not.toContain(KEY); + expect(res.error).not.toContain('lin_api_su'); + expect(res.error).toContain('[REDACTED]'); + } + // GraphQL message path (200 with errors). + vi.stubGlobal( + 'fetch', + vi.fn(() => Promise.resolve(resp(200, JSON.stringify({ errors: [{ message: pad + KEY }] })))), + ); + res = await client().createIssue(TEAM, { title: 'T', description: 'D' }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.error).not.toContain(KEY); + expect(res.error).not.toContain('lin_api_su'); + expect(res.error).toContain('[REDACTED]'); + } + }); + + it('classifies a GraphQL error by a keyword PAST char 300 (classify full, truncate only detail)', async () => { + // The identifying keyword sits after the 300-char cut; classification must run + // on the full message so it is not misread as LINEAR_UNKNOWN_ERROR (Codex PR #25). + const message = 'x'.repeat(320) + ' authentication failed'; + vi.stubGlobal( + 'fetch', + vi.fn(() => Promise.resolve(resp(200, JSON.stringify({ errors: [{ message }] })))), + ); + const res = await client().getWorkflowStates(TEAM); + expect(res.ok).toBe(false); + if (!res.ok) expect(res.error).toMatch(/^LINEAR_AUTH_FAILED/); + }); + + it('redacts the api_key from any returned error string', async () => { + // An upstream body or a native-fetch exception could echo the key; it must + // never reach tool output (GPT-6 adversarial review, invariant 3). + vi.stubGlobal( + 'fetch', + vi.fn(() => Promise.resolve(resp(500, `upstream error, header was ${KEY} oops`))), + ); + const res = await client().createIssue(TEAM, { title: 'T', description: 'D' }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.error).not.toContain(KEY); + expect(res.error).toContain('[REDACTED]'); + } + }); + + it('redacts the api_key from a thrown-exception error', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => Promise.reject(new Error(`connect failed for Authorization: ${KEY}`))), + ); + const res = await client().createIssue(TEAM, { title: 'T', description: 'D' }); + expect(res.ok).toBe(false); + if (!res.ok) expect(res.error).not.toContain(KEY); + }); + + it('maps a GraphQL rate-limit error to LINEAR_RATE_LIMITED', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => + Promise.resolve( + resp( + 200, + JSON.stringify({ + errors: [{ message: 'ratelimited', extensions: { code: 'RATELIMITED' } }], + }), + ), + ), + ), + ); + const res = await client().createIssue(TEAM, { title: 'T', description: 'D' }); + expect(res.ok).toBe(false); + if (!res.ok) expect(res.error).toMatch(/^LINEAR_RATE_LIMITED/); + }); + + it('maps HTTP 401 to LINEAR_AUTH_FAILED (not ambiguous)', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => Promise.resolve(resp(401, 'unauthorized'))), + ); + const res = await client().createIssue(TEAM, { title: 'T', description: 'D' }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.error).toMatch(/^LINEAR_AUTH_FAILED/); + expect(res.ambiguous).toBe(false); + } + }); + + it('maps HTTP 429 to LINEAR_RATE_LIMITED', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => Promise.resolve(resp(429, 'slow down'))), + ); + const res = await client().createIssue(TEAM, { title: 'T', description: 'D' }); + expect(res.ok).toBe(false); + if (!res.ok) expect(res.error).toMatch(/^LINEAR_RATE_LIMITED/); + }); + + it('treats a 5xx on create as AMBIGUOUS (the write may have landed)', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => Promise.resolve(resp(502, 'bad gateway'))), + ); + const res = await client().createIssue(TEAM, { title: 'T', description: 'D' }); + expect(res.ok).toBe(false); + if (!res.ok) expect(res.ambiguous).toBe(true); + }); + + it('maps an aborted (timed-out) request to LINEAR_TIMEOUT, ambiguous', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => { + const e = new Error('aborted'); + e.name = 'AbortError'; + return Promise.reject(e); + }), + ); + const res = await client().createIssue(TEAM, { title: 'T', description: 'D' }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.error).toMatch(/^LINEAR_TIMEOUT/); + expect(res.ambiguous).toBe(true); + } + }); +}); + +describe('createFetchLinearClient — reads', () => { + it('findIssueByMarker returns only a node whose description contains the marker', async () => { + const marker = ''; + vi.stubGlobal( + 'fetch', + vi.fn(() => + Promise.resolve( + resp( + 200, + JSON.stringify({ + data: { + issues: { + nodes: [ + { id: 'x', identifier: 'E-1', url: 'ux', description: 'unrelated' }, + { id: 'y', identifier: 'E-2', url: 'uy', description: `has ${marker}` }, + ], + pageInfo: { hasNextPage: false }, + }, + }, + }), + ), + ), + ), + ); + const res = await client().findIssueByMarker(TEAM, marker); + expect(res).toEqual({ ok: true, value: { id: 'y', identifier: 'E-2', url: 'uy' } }); + }); + + it('findIssueByMarker reports an unconfirmed miss when more pages exist', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => + Promise.resolve( + resp( + 200, + JSON.stringify({ data: { issues: { nodes: [], pageInfo: { hasNextPage: true } } } }), + ), + ), + ), + ); + const res = await client().findIssueByMarker(TEAM, 'm'); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.error).toMatch(/could not confirm/); + expect(res.ambiguous).toBe(true); + } + }); + + it('getWorkflowStates parses the team states', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => + Promise.resolve( + resp( + 200, + JSON.stringify({ + data: { + team: { + states: { + nodes: [ + { id: 's1', name: 'Todo', type: 'unstarted', position: 1 }, + { id: 's2', name: 'Done', type: 'completed', position: 3 }, + ], + }, + }, + }, + }), + ), + ), + ), + ); + const res = await client().getWorkflowStates(TEAM); + expect(res).toEqual({ + ok: true, + value: [ + { id: 's1', name: 'Todo', type: 'unstarted', position: 1 }, + { id: 's2', name: 'Done', type: 'completed', position: 3 }, + ], + }); + }); + + it('getWorkflowStates returns LINEAR_NOT_FOUND for an unknown team', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => Promise.resolve(resp(200, JSON.stringify({ data: { team: null } })))), + ); + const res = await client().getWorkflowStates(TEAM); + expect(res.ok).toBe(false); + if (!res.ok) expect(res.error).toMatch(/^LINEAR_NOT_FOUND/); + }); + + it('getIssueStateType returns the state type', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => + Promise.resolve( + resp(200, JSON.stringify({ data: { issue: { state: { type: 'completed' } } } })), + ), + ), + ); + const res = await client().getIssueStateType('i1'); + expect(res).toEqual({ ok: true, value: 'completed' }); + }); +}); diff --git a/tests/unit/linear-payload.test.ts b/tests/unit/linear-payload.test.ts new file mode 100644 index 0000000..b1e0a57 --- /dev/null +++ b/tests/unit/linear-payload.test.ts @@ -0,0 +1,226 @@ +import { describe, expect, it } from 'vitest'; +import { + buildLinearPayload, + linearPayloadContentHash, + resolveLinearStateId, + stateIntentFor, + type ItemForLinear, +} from '../../src/mcp/tools/linear-payload.js'; +import type { LinearWorkflowState } from '../../src/linear/linear-client.js'; + +const TRACK_ID = '11111111-1111-1111-1111-111111111111'; + +function items(...specs: Array<[string, string, number]>): ItemForLinear[] { + return specs.map(([title, status, sequence_position], i) => ({ + id: `item-${i}`, + title, + status, + sequence_position, + })); +} + +describe('buildLinearPayload', () => { + it('maps title + a checklist in sequence order and embeds the hidden marker', () => { + const payload = buildLinearPayload( + { id: TRACK_ID, title: 'Auth overhaul', status: 'on_track' }, + items(['B second', 'pending', 2], ['A first', 'done', 1]), + ); + expect(payload.title).toBe('Auth overhaul'); + expect(payload.description).toContain('- [x] A first'); + expect(payload.description).toContain('- [ ] B second'); + // ordered by sequence_position: A (1) before B (2) + expect(payload.description.indexOf('A first')).toBeLessThan( + payload.description.indexOf('B second'), + ); + expect(payload.description).toContain(``); + }); + + it('renders a placeholder when the track has no items', () => { + const payload = buildLinearPayload({ id: TRACK_ID, title: 'T', status: 'on_track' }, []); + expect(payload.description).toContain('_No items yet._'); + }); + + it('keeps the title within 255 chars even when escaping delimiters expands it', () => { + const dense = ''.repeat(80); + const payload = buildLinearPayload({ id: TRACK_ID, title: dense, status: 'on_track' }, []); + expect(payload.title.length).toBeLessThanOrEqual(255); + expect(payload.title).not.toContain('`; + const payload = buildLinearPayload( + { id: TRACK_ID, title: 'T', status: 'on_track' }, + items([`sneaky ${fake}`, 'pending', 1]), + ); + // the only real marker is KnoTrack's own; the injected one is escaped + const real = ``; + expect(payload.description.split('