From fcc7a8f2f3749eb7a2a99ec32ed06f0aa0fd3794 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 4 Aug 2026 15:17:14 +0700 Subject: [PATCH] fix(lanes): stop archive/activate from clobbering feature title and links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit archiveActiveFeature was overwriting a feature's own title with the LIVE lane's title on every clear/switch — a --title set on activation silently disappeared. activateFeature restored stage/status/gate/CI/ stages/notes onto the live lane when switching back to a past feature, but never links, so they vanished on reactivation. Both are fixed, with a regression test for each (full activate/archive/reactivate round trip for links). Also fixes server/routes/lanes.js: the doc comment explaining GET /:id/git's rationale had been left sitting above the newly-inserted /:id/features routes instead of its own route. docs/API.md's Lane features section described fields and behavior that don't exist in the real routes (an "active" boolean, a POST response containing "archivedPrevious", a "409 ESTALE" concurrency response) — rewritten to match the actual request/response shapes exactly. --- docs/API.md | 87 ++++++++++++-------------- server/__tests__/lane-features.test.js | 20 ++++++ server/lib/lane-features.js | 6 +- server/routes/lanes.js | 33 ++++++---- 4 files changed, 84 insertions(+), 62 deletions(-) diff --git a/docs/API.md b/docs/API.md index 91e6834..19bc967 100644 --- a/docs/API.md +++ b/docs/API.md @@ -400,76 +400,71 @@ with no slot returns `{"available": false}`. #### Lane features +Per-feature state and archive (`server/lib/lane-features.js`) — a lane's +`stage`/`status`/etc. is always the LIVE view of whichever feature it's +currently on; these routes read and switch between a lane's feature history. + ```http GET /api/lanes/:id/features ``` -List every feature this lane has activated, archived or live: +Every feature this lane has ever activated, archived or live, most recently +touched first: ```json { "features": [ - { "slug": "auth-redesign", "active": true, "title": "Auth redesign (v2)", "stage": "implement", "status": "running", "created_at": 1722702012 }, - { "slug": "migration", "active": false, "title": null, "stage": "done", "status": "idle", "created_at": 1722700000 }, + { + "id": 5, "lane_id": 12, "slug": "auth-redesign", "title": "auth-redesign", + "branch": "feat/auth", "pipeline": "default", "stage": "implement", + "stage_since": "2026-08-04T09:00:00.000Z", "status": "running", + "gate_decision": null, "ci_status": null, "qc_dev": null, + "stages": {}, "links": {}, "notes": null, "archived_at": null, + "created_at": "2026-08-04T09:00:00.000Z", "updated_at": "2026-08-04T09:00:00.000Z", + "pipeline_name": "Default", "pipeline_nodes": [ /* same shape as a live lane's */ ], "progress": 40 + } ] } ``` -Each feature carries a canonicalized `slug` (drops leading `feat/`, `/` → `-`, preserves case, keeps `[A-Za-z0-9._-]` only — **not the same rule as `worktree.js:slugify`**, which lowercases). `active` is `true` for the currently live feature. `title` is the display name (human-chosen via `ccam feature activate --title`; omitted/`null` if never set or identical to slug). `stage`, `status`, `notes` reflect the **saved** state when this feature was last archived; `active:true` shows the **live** lane's current stage instead. +Each row is the hydrated `lane_features` row (`stages`/`links` parsed from +JSON) plus `pipeline_name`/`pipeline_nodes`/`progress`, computed the exact +same way a live lane's are — the client renders an archived feature with the +same `PipelineMap` component, no special-casing. `archived_at` is `null` for +the currently-active feature, an ISO timestamp for every past one. `slug` is +always the **canonicalized** form (drops a leading `feat/`, turns `/` and +whitespace into `-`, keeps `[A-Za-z0-9._-]`, preserves case — **not the same +rule as `worktree.js:slugify`**, which lowercases everything). ```http GET /api/lanes/:id/features/:slug ``` -Show one feature's saved pipeline: - -```json -{ - "slug": "auth-redesign", - "active": true, - "title": "Auth redesign (v2)", - "stage": "implement", - "status": "running", - "notes": "Testing with OAuth...", - "created_at": 1722702012 -} -``` - -Works on both archived and active features. If the slug has never been activated, returns `404 ENOFEAT`. +One feature by its canonicalized slug, same shape as an entry above, wrapped +as `{ "feature": {...} }`. `404 ENOFEATURE` if that slug was never activated +on this lane; `404 ENOLANE` if the lane itself doesn't exist. ```http POST /api/lanes/:id/features/activate -{ "slug": "auth-redesign", "title": "Auth redesign (v2)" } +{ "slug": "feat/Auth Redesign", "title": "Auth redesign (optional)" } ``` -Activate a feature by slug. Request body: -- `slug` (required, string) — the canonicalized slug (or raw slug; the route canonicalizes it before lookup) -- `title` (optional, string) — human-friendly name to save with this feature +Switches the lane to a feature by slug: +- If a **different** feature is currently active, it is archived first with + its exact live state (stage, status, gate/CI fields, stages, links, notes) + — its own `title` is never overwritten by this. +- If the target slug was activated before, its saved state is restored onto + the live lane (stage, status, gate/CI fields, stages, links, notes) — this + is what makes switching back to a past feature resume where it left off. +- If the slug is new, a fresh feature row is created (`title` defaults to the + slug itself when omitted). +- Re-activating the **currently** active slug is a no-op. -Response: +Response: `{ "lane": , "feature": }`. -```json -{ - "slug": "auth-redesign", - "active": true, - "title": "Auth redesign (v2)", - "stage": "implement", - "status": "running", - "created_at": 1722702012, - "archivedPrevious": { "slug": "migration", "stage": "done" } -} -``` - -Behavior: -- If the target slug has been activated before, restores its saved stage/status/notes onto the live lane -- If the slug is new, creates a fresh feature row with empty stage/status/notes -- If a different feature is currently active, archives it first (copies live stage/status/notes to its row) — echoed in `archivedPrevious` -- Returns **200** on success; **409 ESTALE** if another request changed the lane between read and write - -Status codes: -- **200** — feature activated -- **409** — the lane's stage changed concurrently (rare with single-session lanes) -- **400** — missing/invalid request body +- **200** — activated (or already active). +- **400** `{ "error": { "code": "EBADSLUG", "message": "slug is required" } }` — missing/empty `slug`, or the canonicalized result is empty (e.g. `"feat/"` alone). +- **404** `{ "error": { "code": "ENOLANE", "message": "lane not found" } }`. ### Locks diff --git a/server/__tests__/lane-features.test.js b/server/__tests__/lane-features.test.js index 1b610fd..2616062 100644 --- a/server/__tests__/lane-features.test.js +++ b/server/__tests__/lane-features.test.js @@ -112,6 +112,26 @@ describe("activateFeature / archiveActiveFeature", () => { const { feature } = features.activateFeature(lane.id, "feat/Weird Input/"); assert.equal(feature.slug, "Weird-Input"); }); + + it("a feature's own title survives being archived (never clobbered by the lane's title)", () => { + const lane = makeLane(); + features.activateFeature(lane.id, "one", { title: "Custom title" }); + features.activateFeature(lane.id, "two"); // archives "one" + assert.equal(features.getFeature(lane.id, "one").title, "Custom title"); + }); + + it("links survive a full activate -> archive -> reactivate round trip", () => { + const lane = makeLane(); + features.activateFeature(lane.id, "one"); + lanesLib.updateLane(lane.id, { links: { pr: "https://example.com/pr/1" } }); + features.activateFeature(lane.id, "two"); // archives "one" with its links + + const archived = features.getFeature(lane.id, "one"); + assert.deepEqual(archived.links, { pr: "https://example.com/pr/1" }); + + const { lane: reactivated } = features.activateFeature(lane.id, "one"); + assert.deepEqual(reactivated.links, { pr: "https://example.com/pr/1" }); + }); }); describe("listFeatures / getFeature", () => { diff --git a/server/lib/lane-features.js b/server/lib/lane-features.js index 8d77b1a..5a3207b 100644 --- a/server/lib/lane-features.js +++ b/server/lib/lane-features.js @@ -99,12 +99,11 @@ function archiveActiveFeature(laneId) { db.prepare( `UPDATE lane_features SET - title = ?, branch = ?, pipeline = ?, stage = ?, stage_since = ?, status = ?, + branch = ?, pipeline = ?, stage = ?, stage_since = ?, status = ?, gate_decision = ?, ci_status = ?, qc_dev = ?, stages = ?, links = ?, notes = ?, archived_at = ?, updated_at = ? WHERE id = ?` ).run( - lane.title, lane.branch, lane.pipeline, lane.stage, @@ -172,7 +171,7 @@ function activateFeature(laneId, rawSlug, options = {}) { db.prepare( `UPDATE lanes SET stage = ?, stage_since = ?, status = ?, gate_decision = ?, ci_status = ?, - stages = ?, notes = ?, active_feature_id = ?, updated_at = ? + stages = ?, links = ?, notes = ?, active_feature_id = ?, updated_at = ? WHERE id = ?` ).run( target.stage, @@ -181,6 +180,7 @@ function activateFeature(laneId, rawSlug, options = {}) { target.gate_decision, target.ci_status, JSON.stringify(target.stages || {}), + JSON.stringify(target.links || {}), target.notes, target.id, nowIso(), diff --git a/server/routes/lanes.js b/server/routes/lanes.js index 2501379..925f90a 100644 --- a/server/routes/lanes.js +++ b/server/routes/lanes.js @@ -239,19 +239,11 @@ router.get("/:id/preflight", async (req, res) => { }); /** - * A lane's working-copy facts: branch, short HEAD, that commit's subject, and - * the uncommitted counts. Read-only, so no same-origin guard — that guard - * exists for the destructive actions. - * - * Deliberately NOT part of `GET /api/lanes`: this shells out to git three - * times, and that payload is polled and re-broadcast on every hook-driven - * lane_update. Any failure — no such directory, not a repo, git itself - * erroring — is reported as `available: false` rather than a 500, because a - * lane pointing at a plain directory is a normal state, not a fault. - * - * The `/:id/:action` catch-all below cannot shadow this one — that route is a - * POST and Express matches on method as well as path. Verified by moving this - * registration after it: the suite stayed green. + * Per-feature state and archive (B) — `lib/lane-features.js`. `GET + * /:id/features` lists every feature the lane has ever activated (archived + * or live); `GET /:id/features/:slug` shows one, including an archived + * one's saved pipeline; `POST /:id/features/activate` switches the live + * lane to a feature by slug, archiving whichever one was active first. */ router.get("/:id/features", (req, res) => { const lane = lanesLib.getLane(req.params.id); @@ -290,6 +282,21 @@ router.post("/:id/features/activate", sameOriginGuard, (req, res) => { } }); +/** + * A lane's working-copy facts: branch, short HEAD, that commit's subject, and + * the uncommitted counts. Read-only, so no same-origin guard — that guard + * exists for the destructive actions. + * + * Deliberately NOT part of `GET /api/lanes`: this shells out to git three + * times, and that payload is polled and re-broadcast on every hook-driven + * lane_update. Any failure — no such directory, not a repo, git itself + * erroring — is reported as `available: false` rather than a 500, because a + * lane pointing at a plain directory is a normal state, not a fault. + * + * The `/:id/:action` catch-all below cannot shadow this one — that route is a + * POST and Express matches on method as well as path. Verified by moving this + * registration after it: the suite stayed green. + */ router.get("/:id/git", async (req, res) => { const lane = lanesLib.getLane(req.params.id); if (!lane) return res.status(404).json({ error: { code: "ENOLANE", message: "lane not found" } });