docs(3524): CJS↔SDK hard-seam ADR + phased PRD (#3529)
* docs(3524): propose CJS↔SDK hard-seam ADR + phased PRD Adds docs/adr/3524-cjs-sdk-hard-seam.md (Proposed) and docs/prd/3524-cjs-sdk-hard-seam.md (Reference) tracking #3524. Updates the ADR and PRD index READMEs. The ADR defines one canonical owner per responsibility across the CJS (bin/lib/*.cjs) and SDK (sdk/src/**/*.ts) sides, eliminating the recurring drift bug class (#1535, #1542, #2047, #2638, #2653, #2687, #2798, #3055, #3523). Three layers: shared data (sdk/shared/*.json), shared core logic (sdk/src/core/ → dual CJS+ESM build), thin adapters. Enforcement is layered: build-time grep, type-level contract test, mutation parity test, CODEOWNERS gate, in-file banner. The PRD phases the migration in five independently shippable steps — shared data first (closes constant drift), then config consolidation (closes #3523 class), then project-root + path projection, then state and verify handlers, then enforcement hardening + retrospective. * docs(3524): revise seam ADR + PRD after architecture review Architecture-review pass (via /improve-codebase-architecture) found seven deepening opportunities; this commit applies all of them. 1. Re-anchor on the existing generator precedent. The repo already has sdk/scripts/gen-command-aliases.ts emitting both .generated.ts and .generated.cjs from one TS source, with sdk/scripts/check-command-aliases-fresh.mjs as the CI freshness gate. The ADR's invented dual CJS+ESM bundler pipeline is dropped. Each Shared Module gets one generator script and one freshness check, modeled on that precedent. 2. Drop the generic sdk/src/core/ container. The canonical-owner table is now indexed by Module, using the CONTEXT.md domain vocabulary (STATE.md Document Module, Configuration Module, etc.) rather than file-path-based pseudo-modules. 3. Split the coarse "State management" row into three: the pure STATE.md Document Module (already a character-identical hand-synced pair — perfect Phase 1 target), the Planning Workspace Module (defer to ADR-0004), and per-side state I/O Adapters (legitimately differ sync vs async). 4. Defer to existing ADRs. Planning Path Projection (ADR-0006), Model Catalog (ADR-0003), Planning Workspace (ADR-0004), Dispatch Policy (ADR-0001), Shell Command Projection (ADR-0009 post-Phase 3-4 expansion which absorbed superseded ADR-0010). The stale ADR-0010 reference is fixed. 5. Define a Configuration Module entry in CONTEXT.md as a Phase 2 deliverable, with explicit Interface contract for loadConfig, normalizeLegacyKeys, mergeDefaults, migrateOnDisk. 6. Split the Workstream Inventory Module into a pure Builder (generated, shared) and per-side Reader Adapters (hand-authored, sync vs async). Same pattern generalizes to other paired Modules. 7. Match enforcement to existing scripts. Per-Module freshness checks (precedent: check-command-aliases-fresh.mjs), per-Module drift lints (precedent: lint-shell-command-projection-drift.cjs), and one hand-sync pair lint that blocks the #3523 anti-pattern at PR time. PRD phases reordered: STATE.md Document Module ships first as a proof of pattern (two identical files become one source plus one generated artifact). Configuration Module ships second, closing the #3523 class. Workstream Inventory Builder split third. Project-Root Resolution fourth. Enforcement and retrospective fifth. * docs(3524): expand scope — CJS router delegates to SDK runtime bridge User flagged that the original "Out of scope" list was my unilateral scoping call, not theirs. After review, the CJS router consolidation (formerly out-of-scope item #1) is brought into scope. ADR additions: - CJS Command Router Adapter Module row added to canonical-owner table. Existing Module (per CONTEXT.md) is amended so the per-family `handlers` map delegates to `QueryRuntimeBridge.execute()` in-process. Per-side CJS handler files for canonical families (state.cjs, verify.cjs, init.cjs, phase.cjs, etc.) shrink to delegates or are deleted. - Per-side I/O Adapter consequence updated to clarify the bridge preserves the in-process model. No subprocess hop is added. - "Out of scope" stripped of router item; CJS-only seam migration and verify-Module-first work remain out of scope. PRD additions: - New Phase 5: CJS Command Router Adapter delegates to SDK runtime bridge, family-by-family, with golden parity matrix per family gating each PR. - Old Phase 5 (enforcement) renumbered to Phase 6, expanded to cover Phase 5's parity matrix and runtime-bridge CODEOWNERS. - Open question #4 added for the synchronous-bridging mechanism (`deasync` vs `Atomics.wait` vs sync-handler refactor) — resolved in the Phase 5 spike before any family migration begins. - Open question #5 added for family migration order (recommended: smallest read-only family first). - Risks table expanded with three Phase 5 rows: bridging-mechanism uncertainty, observable-output regression, startup-time impact. - Done-when updated for six phases and five enforcement layers. Non-goals updated: CJS-only Module migration and verify-Module deepening remain out of scope. CJS CLI removal explicitly stays off the table — the external `gsd-tools` contract is preserved. * docs(3524): address CodeRabbit review * docs(3524): fix PRD issue reference markdown
This commit is contained in:
103
docs/adr/3524-cjs-sdk-hard-seam.md
Normal file
103
docs/adr/3524-cjs-sdk-hard-seam.md
Normal file
@@ -0,0 +1,103 @@
|
||||
# CJS↔SDK hard seam — one source of truth per Shared Module
|
||||
|
||||
- **Status:** Proposed
|
||||
- **Date:** 2026-05-14
|
||||
- **Tracking issue:** [#3524](https://github.com/gsd-build/get-shit-done/issues/3524)
|
||||
- **Related PRD:** [`docs/prd/3524-cjs-sdk-hard-seam.md`](../prd/3524-cjs-sdk-hard-seam.md)
|
||||
- **Extends:** ADR-0005 (seam map) — adds the **Shared-Module Source Policy** to the seam family
|
||||
- **Defers to:** ADR-0001 (Dispatch Policy Module), ADR-0003 (Model Catalog Module), ADR-0004 (Planning Workspace Module), ADR-0006 (Planning Path Projection Module), ADR-0009 (Shell Command Projection Module — post-Phase 3–4, also subsuming superseded ADR-0010)
|
||||
|
||||
We decided to harden the boundary between the CJS tooling layer (`get-shit-done/bin/lib/*.cjs`) and the SDK (`sdk/src/**/*.ts`) by making every Module that is conceptually shared between the two runtimes have exactly one hand-authored source of truth and at most one generated artifact per runtime. The trigger is the recurring drift bug class — #1535, #1542, #2047/#2052, #2638/#2655, #2653/#2670, #2687/#2706, #2798/#2816, #3055/#3116, #3523 — each of which was a fix landing on one side without the other.
|
||||
|
||||
The precedent shape is already in the repo. `sdk/scripts/gen-command-aliases.ts` emits `sdk/src/query/command-aliases.generated.ts` **and** `get-shit-done/bin/lib/command-aliases.generated.cjs` from one TypeScript source. `sdk/scripts/check-command-aliases-fresh.mjs` is the CI freshness gate. The two consuming sides are pure Adapters over the generated artifact. This ADR generalizes that pattern to the other Shared Modules and forbids the hand-synced-pair anti-pattern that produced #3523.
|
||||
|
||||
## Decision
|
||||
|
||||
### 1. Shared-Module Source Policy
|
||||
|
||||
A **Shared Module** is any Module whose Interface is consumed identically by both the CJS toolset and the SDK. The CONTEXT.md domain glossary already calls these out — e.g. `STATE.md Document Module` is explicitly typed as "Shared CJS/SDK pure transform Module."
|
||||
|
||||
For every Shared Module:
|
||||
|
||||
1. **Exactly one hand-authored source of truth.** Lives at `sdk/src/<module-name>/` as TypeScript when the Module has behavior, or `sdk/shared/<module-name>.manifest.json` when the Module is pure data.
|
||||
2. **Generated artifacts only.** The CJS-side file is `get-shit-done/bin/lib/<module-name>.generated.cjs` and is emitted mechanically. It is never hand-edited.
|
||||
3. **Per-Module freshness check.** A CI script `sdk/scripts/check-<module>-fresh.mjs` re-runs the generator and fails if the emitted artifact differs from the committed one. Precedent: `check-command-aliases-fresh.mjs`.
|
||||
4. **Per-Module drift lint** (when the source is data, not a generator output). Precedent: `scripts/lint-shell-command-projection-drift.cjs`. The lint asserts the canonical-owner invariants that aren't captured by file-equality.
|
||||
5. **Hand-synced pairs are forbidden.** A pre-merge `lint-shared-module-handsync.cjs` greps `get-shit-done/bin/lib/` for non-`.generated.*` files whose basename matches a `sdk/src/query/<same-name>.ts` source and fails the build unless the pair is explicitly allow-listed.
|
||||
|
||||
### 2. Module-indexed canonical-owner table
|
||||
|
||||
The table below indexes by Module, not by physical layer. Each row names the source of truth, the emitted artifacts, the Adapter sites, and either the new ADR section that defines the Module or the existing ADR that already owns it.
|
||||
|
||||
| Module | Status | Source of truth | Generated artifacts | Adapters |
|
||||
|---|---|---|---|---|
|
||||
| **STATE.md Document Module** | New under this ADR (Phase 1) — see CONTEXT.md "STATE.md Document Module" | `sdk/src/state-document/index.ts` (promoted from `sdk/src/query/state-document.ts`) | `sdk/src/query/state-document.generated.ts`, `get-shit-done/bin/lib/state-document.generated.cjs` | `bin/lib/state.cjs` and `sdk/src/query/state*.ts` import the generated form |
|
||||
| **Configuration Module** | New under this ADR (Phase 2) — definition added to CONTEXT.md as part of Phase 2 | `sdk/src/configuration/index.ts` plus data manifests `sdk/shared/config-schema.manifest.json` and `sdk/shared/config-defaults.manifest.json` | `sdk/src/query/config-schema.generated.ts`, `get-shit-done/bin/lib/config-schema.generated.cjs`, `get-shit-done/bin/lib/configuration.generated.cjs` | `bin/lib/config.cjs`, `bin/lib/core.cjs:loadConfig`, `sdk/src/config.ts` |
|
||||
| **Workstream Inventory Module** (Builder) | Amended under this ADR (Phase 3) — Builder split documented in CONTEXT.md update | `sdk/src/workstream-inventory/builder.ts` (pure projection from directory entries + STATE.md text + plan scan results → typed inventory) | `sdk/src/query/workstream-inventory-builder.generated.ts`, `get-shit-done/bin/lib/workstream-inventory-builder.generated.cjs` | Per-side fs Readers (`workstream-inventory.cjs` sync, `workstream-inventory.ts` async) call the Builder. Readers stay hand-authored because the fs idiom legitimately differs. |
|
||||
| **Project-Root Resolution Module** | New under this ADR (Phase 4) — short CONTEXT.md entry, behavior already de-facto shared | `sdk/src/project-root/index.ts` | `get-shit-done/bin/lib/project-root.generated.cjs` | `bin/lib/core.cjs` (`findProjectRoot`, `findEffectiveRoot`), `sdk/src/helpers.ts` |
|
||||
| **Frontmatter Module** | Conditional (Phase 3, only if drift catalogue confirms pair duplication) | `sdk/src/frontmatter/index.ts` | `get-shit-done/bin/lib/frontmatter.generated.cjs` | Existing handler call sites |
|
||||
| **Plan Scan Module** | Conditional (Phase 3 or later) | `sdk/src/plan-scan/index.ts` | `get-shit-done/bin/lib/plan-scan.generated.cjs` | Phase/roadmap routers |
|
||||
| **CJS Command Router Adapter Module** | Amended under this ADR (Phase 5). Existing Module (per CONTEXT.md) is extended so the per-family `handlers` map delegates to the SDK runtime bridge in-process instead of to parallel CJS handler implementations. | `sdk/src/query-runtime-bridge.ts` (already exists) + per-family delegate emitter | `get-shit-done/bin/lib/cjs-command-router-adapter.cjs` (existing, ~40 lines) plus per-family `handlers` maps that `require('../../sdk/dist/query-runtime-bridge.cjs')` and call `QueryRuntimeBridge.execute()` | `bin/gsd-tools.cjs` and the seven `bin/lib/*-command-router.cjs` files are the consumers. Per-family CJS handler files (`state.cjs`, `verify.cjs`, `init.cjs`, etc.) shrink to delegates or are deleted once the SDK handler is the only implementation. |
|
||||
| Command-Alias Module | **Already sealed** by this pattern's precedent — `sdk/scripts/gen-command-aliases.ts` + `check-command-aliases-fresh.mjs` | No change | No change | No change |
|
||||
| Dispatch Policy Module | **Defer — see ADR-0001** (and its 2026-05-05 SDK Runtime Bridge amendment) | n/a | n/a | n/a |
|
||||
| Model Catalog Module | **Defer — see ADR-0003**; the `sdk/shared/model-catalog.json` manifest already follows the source-of-truth policy | n/a | n/a | n/a |
|
||||
| Planning Workspace Module | **Defer — see ADR-0004**; `withPlanningLock`, workstream pointer policy, lock semantics stay where they are | n/a | n/a | n/a |
|
||||
| Planning Path Projection Module | **Defer — see ADR-0006**; SDK is canonical, CJS path resolution converges via Phase 4 if any divergence is found | n/a | n/a | n/a |
|
||||
| Shell Command Projection Module (incl. platform fs + subprocess after Phase 3–4 expansion) | **Defer — see ADR-0009**; this Module is the canonical owner for `platformWriteSync`, `platformReadSync`, `platformEnsureDir`, `execGit`, `execNpm`, `execTool`, `probeTty`, `normalizeContent` | n/a | n/a | n/a |
|
||||
| Skill Surface Budget Module | **Defer — see ADR-0011** (accepted, not the 0011-superseded draft) | n/a | n/a | n/a |
|
||||
|
||||
### 3. Out-of-seam Modules (per-runtime, no shared source)
|
||||
|
||||
These remain CJS-only. Drift cannot occur because there is no SDK counterpart. If any later needs an SDK port, that port is a new enhancement, not a parallel implementation.
|
||||
|
||||
- `bin/lib/graphify.cjs`
|
||||
- `bin/lib/gsd2-import.cjs`
|
||||
- `bin/lib/schema-detect.cjs`
|
||||
- `bin/lib/fallow-runner.cjs`
|
||||
- `bin/lib/intel.cjs`
|
||||
- `bin/lib/drift.cjs`
|
||||
- `bin/lib/installer-migrations.cjs` (installer runtime is CJS-native; SDK consumes via `sdk-package-compatibility.ts` Adapter)
|
||||
|
||||
### 4. Per-side I/O Adapters legitimately differ
|
||||
|
||||
The per-side state Adapter, verify Adapter, and similar handlers are **not** in the Shared-Module table. CJS callers use synchronous fs/exec; SDK callers use async I/O and the SDK observability decorators. The pure transforms behind them (parsing, projection, normalization) are extracted into Shared Modules per the table above; the I/O remains per-side. Golden parity tests in `sdk/src/golden/` pin observable behavior across the seam.
|
||||
|
||||
### 5. Enforcement (per existing repo precedents, not new conventions)
|
||||
|
||||
Drift is blocked at three layers, each modeled on an existing in-repo script:
|
||||
|
||||
1. **Per-Module freshness check** — `sdk/scripts/check-<module>-fresh.mjs`, one per Shared Module in the table. Precedent: `check-command-aliases-fresh.mjs`.
|
||||
2. **Per-Module drift lint** (when invariants are not pure file-equality) — `scripts/lint-<module>-drift.cjs`, one per data-manifest-backed Module. Precedent: `lint-shell-command-projection-drift.cjs`.
|
||||
3. **Hand-sync pair lint** — `scripts/lint-shared-module-handsync.cjs` rejects any pair of files at `get-shit-done/bin/lib/<name>.cjs` and `sdk/src/query/<name>.ts` (or `sdk/src/<name>.ts`) that are neither generated artifacts nor on an explicit allow-list. This blocks the #3523 anti-pattern at PR time.
|
||||
|
||||
CODEOWNERS extends to `sdk/src/<module>/` for each Shared Module. Architecture-team review is required for changes to a source of truth.
|
||||
|
||||
A top-of-file banner is auto-inserted by each generator into the emitted `.generated.cjs` / `.generated.ts` files. Banner pattern follows the existing `command-aliases.generated.*` files: a header noting "GENERATED FILE — Source: …". No additional banner tooling is introduced.
|
||||
|
||||
### 6. New CONTEXT.md entries added by this ADR's phases
|
||||
|
||||
- **Configuration Module** (added during Phase 2): Module owning config load, legacy-key normalization, defaults merge, and explicit on-disk migration for `.planning/config.json`. Interface: `loadConfig(cwd) → MergedConfig` (pure read, no disk write); `normalizeLegacyKeys(parsed) → { parsed, normalizations[] }` (idempotent, returns the list of normalizations applied for migration logging); `mergeDefaults(parsed) → MergedConfig`; `migrateOnDisk(cwd) → MigrationReport` (explicit, opt-in, called only by the installer and by `gsd-tools migrate-config`). Invariants: never mutates disk inside `loadConfig`; legacy top-level keys (`branching_strategy`, `sub_repos`, `multiRepo`, `depth`) are normalized into their canonical nested locations in the returned value; defaults come from the shared `config-defaults.manifest.json`.
|
||||
- **Project-Root Resolution Module** (added during Phase 4): Module owning project-root and effective-root resolution heuristics including own-`.planning` detection, parent-`sub_repos` traversal, legacy `multiRepo`, and `.git`-ancestor fallback.
|
||||
- **Workstream Inventory Module — Builder split** (CONTEXT.md amendment during Phase 3): the existing Module entry gains a sub-paragraph noting that the pure projection logic is the source of truth and the per-side Reader Adapters are hand-authored over the generated Builder.
|
||||
- **CJS Command Router Adapter Module — runtime-bridge delegation** (CONTEXT.md amendment during Phase 5): the existing Module entry gains a paragraph noting that the per-family `handlers` map delegates to `QueryRuntimeBridge.execute()` in-process via `require('../../sdk/dist/query-runtime-bridge.cjs')`. Per-side CJS handler files (`state.cjs`, `verify.cjs`, etc.) that previously held parallel implementations are reduced to delegates or deleted once their SDK counterpart is the only remaining implementation. CJS-only Module handlers (graphify, gsd2-import, schema-detect, fallow-runner, intel, drift) keep their in-process CJS implementations because no SDK counterpart exists.
|
||||
|
||||
## Consequences
|
||||
|
||||
- **The hand-synced-pair anti-pattern that produced #3523 becomes impossible to merge.** The `lint-shared-module-handsync.cjs` gate rejects any new pair that is not generated. The `check-<module>-fresh.mjs` gates reject any edit to a generated file that is out of sync with its source.
|
||||
- **The seam vocabulary stays inside the existing CONTEXT.md / LANGUAGE.md frame.** No new layer labels ("shared core", "shared data"); the unit of seam ownership is the Module, as it already is everywhere else in this repo.
|
||||
- **No new build tooling is introduced.** The generator pattern is the existing `gen-command-aliases.ts` shape. No dual CJS+ESM bundler, no `package.json` `exports` subpath change, no `tsup`/`rollup` decision.
|
||||
- **Each phase ships one Shared Module.** The smallest phase (STATE.md Document Module) ships first because both files are already character-identical — the deletion test passes on contact. The trigger bug class (#3523) is closed in Phase 2 by the Configuration Module. The seam becomes a real wall in Phase 5 when the CJS routers stop holding parallel handler implementations.
|
||||
- **CJS dispatch collapses onto the SDK runtime bridge.** Once Phase 5 lands, every canonical command running via `gsd-tools` executes the same SDK handler that `gsd-sdk query` executes — in-process, not subprocess. The per-side state/verify/init/phase/roadmap/validate handler implementations in CJS are replaced by thin delegates over `QueryRuntimeBridge.execute()`. The result-shape contract is preserved (`{ exitCode, stdoutChunks, stderrLines }` per the Query CLI Output Module, ADR-0001).
|
||||
- **Existing ADRs are deferred to, not restated.** Planning Path Projection (ADR-0006), Model Catalog (ADR-0003), Planning Workspace (ADR-0004), Dispatch Policy (ADR-0001), Shell Command Projection (ADR-0009) remain authoritative for their domains. The new ADR adds Shared-Module Source Policy, the per-Module entries above, and the CJS Command Router Adapter Module amendment.
|
||||
- **Per-side I/O Adapter divergence is preserved at the runtime-bridge boundary.** The CJS router's sync execution model is preserved: `QueryRuntimeBridge.execute()` exposes a sync entry point for CJS callers (or, when the underlying SDK handler is async, the bridge runs an in-process event loop step). No subprocess hop is added. Async SDK call sites continue to use the async bridge directly.
|
||||
- **Enforcement reuses existing scripts.** Three new lint/check primitives, all modeled on scripts already in `scripts/` and `sdk/scripts/`. CI wiring follows the existing precedent.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Migrating CJS-only Modules (graphify, gsd2-import, schema-detect, fallow-runner, intel, drift) to SDK handlers — each is its own enhancement.
|
||||
- Sync→async migration of CJS state/verify Adapters — leaves the per-side Adapter shape intact, which is the point.
|
||||
- Defining a Verify Module before the verify surface has a shared Interface — that is precondition work for a future enhancement, not this one.
|
||||
|
||||
## Amendments
|
||||
|
||||
_(Append-only. Use a dated header when the decision evolves.)_
|
||||
@@ -44,6 +44,7 @@ See **[CONTRIBUTING.md — "Proposing an ADR or PRD"](../../CONTRIBUTING.md#prop
|
||||
| [0011-skill-surface-budget-module.md](0011-skill-surface-budget-module.md) | Skill Surface Budget Module owns install-time profile staging and runtime surface control | Accepted |
|
||||
| [0011-review-default-reviewers.md](0011-review-default-reviewers.md) | Review default-reviewers selection policy for /gsd:review | Accepted |
|
||||
| [0011-review-default-reviewers-prd.md](0011-review-default-reviewers-prd.md) | PRD for review.default_reviewers feature (#3464) | Reference |
|
||||
| [3524-cjs-sdk-hard-seam.md](3524-cjs-sdk-hard-seam.md) | CJS↔SDK hard seam — single canonical owner per responsibility (#3524) | Proposed |
|
||||
|
||||
## Seam map
|
||||
|
||||
|
||||
240
docs/prd/3524-cjs-sdk-hard-seam.md
Normal file
240
docs/prd/3524-cjs-sdk-hard-seam.md
Normal file
@@ -0,0 +1,240 @@
|
||||
# PRD: CJS↔SDK hard seam — Shared-Module migration
|
||||
|
||||
- **Status:** Reference
|
||||
- **Date:** 2026-05-14
|
||||
- **Tracking issue:** [#3524](https://github.com/gsd-build/get-shit-done/issues/3524)
|
||||
- **Related ADR:** [`docs/adr/3524-cjs-sdk-hard-seam.md`](../adr/3524-cjs-sdk-hard-seam.md)
|
||||
|
||||
## Why this PRD exists
|
||||
|
||||
The ADR defines the target architecture — one source of truth per Shared Module, reusing the existing `command-aliases.generated.*` precedent. This PRD defines *how to get there* without breaking the running system. The migration is sequenced so the smallest, lowest-risk Shared Module ships first as a working proof of the pattern. Subsequent phases apply the same pattern to higher-stakes Modules. Each phase is independently shippable and independently reversible.
|
||||
|
||||
## Problem statement
|
||||
|
||||
The CJS↔SDK boundary in `gsd-build/get-shit-done` is structurally permeable. Multiple Shared Modules — STATE.md Document Module, Workstream Inventory Module, and several others — exist today as **hand-synced pairs** of `.cjs` and `.ts` files with character-identical implementations. Constants (`CONFIG_DEFAULTS`, `VALID_CONFIG_KEYS`) are likewise defined twice. The boundary is policed only by:
|
||||
|
||||
- A naming-parity test (`tests/config-schema-sdk-parity.test.cjs`)
|
||||
- Output-parity golden tests for read-only handlers (`sdk/src/golden/read-only-parity.integration.test.ts`)
|
||||
|
||||
These catch some drift but miss:
|
||||
- Structure drift under defaults (#3523: top-level `branching_strategy` returned as `'none'` by CJS, `'phase'` by SDK)
|
||||
- Warning/error-message drift (#3523: CJS warns falsely; SDK silently grafts)
|
||||
- Mutation-path drift (each side tested separately; no cross-side mutation fixture)
|
||||
- New-Module drift (a new constant added to one side and not the other is invisible)
|
||||
|
||||
Each of #1535, #1542, #2047/#2052, #2638/#2655, #2653/#2670, #2687/#2706, #2798/#2816, #3055/#3116, #3523 fits this shape.
|
||||
|
||||
The fix is mechanical: for every hand-synced pair, replace one side with a generated artifact derived from the other side as the source of truth, modeled on the existing `sdk/scripts/gen-command-aliases.ts` + `sdk/scripts/check-command-aliases-fresh.mjs` pattern.
|
||||
|
||||
## Goals
|
||||
|
||||
1. Eliminate the drift bug class. Concretely: zero new bugs with the `drift-recurrence` retroactive label in the four months following the seam landing.
|
||||
2. One source of truth per Shared Module, enforced by per-Module freshness checks at PR time.
|
||||
3. Hand-synced pairs of `.cjs`/`.ts` files become impossible to merge (lint gate).
|
||||
4. No new build tooling. The existing generator pattern scales.
|
||||
|
||||
## Non-goals
|
||||
|
||||
- Removing the CJS CLI. `gsd-tools` continues to exist for shell-script back-compat. (Its dispatcher delegates to the SDK runtime bridge after Phase 5; the external CLI contract is unchanged.)
|
||||
- Migrating CJS-only Modules (graphify, gsd2-import, schema-detect, fallow-runner, intel, drift) to SDK handlers.
|
||||
- Defining a Verify Module before the verify surface has a shared Interface. Verify-surface deepening is precondition work for a future enhancement.
|
||||
|
||||
## Approach
|
||||
|
||||
The repo already has a working precedent for shared CJS/SDK Modules: `sdk/scripts/gen-command-aliases.ts` emits both `sdk/src/query/command-aliases.generated.ts` and `get-shit-done/bin/lib/command-aliases.generated.cjs` from a single TypeScript source. `sdk/scripts/check-command-aliases-fresh.mjs` is the CI freshness gate that fails when either generated file drifts from the source. This PRD generalizes that pattern to every Shared Module.
|
||||
|
||||
For each Shared Module being migrated:
|
||||
|
||||
1. Promote one side to the source of truth (the TS source, because it already carries types).
|
||||
2. Write `sdk/scripts/gen-<module>.ts` that emits both `.generated.ts` and `.generated.cjs`.
|
||||
3. Write `sdk/scripts/check-<module>-fresh.mjs` modeled on `check-command-aliases-fresh.mjs`.
|
||||
4. Replace the hand-authored CJS file with a thin re-export from the generated file.
|
||||
5. Wire the freshness check into CI.
|
||||
6. Once green for one release cycle, delete the now-unreferenced hand-authored content from history's view by removing dead re-exports.
|
||||
|
||||
A separate, standing CI lint (`scripts/lint-shared-module-handsync.cjs`, introduced in Phase 6) blocks any new hand-synced pair from being merged.
|
||||
|
||||
## Phased plan
|
||||
|
||||
Phases are sized to ship in one to two PRs each. Each phase has its own GitHub issue, linked back to #3524, opened only after the previous phase ships.
|
||||
|
||||
---
|
||||
|
||||
### Phase 1 — STATE.md Document Module (smallest possible proof)
|
||||
|
||||
**Why first.** `bin/lib/state-document.cjs` and `sdk/src/query/state-document.ts` are already a character-identical hand-synced pair of pure transforms (the file headers explicitly say "Pure transforms for STATE.md text. This module does not read the filesystem and does not own persistence or locking."). Deletion test passes on contact: one side can be deleted as soon as the other becomes the generated artifact. This is the safest possible first step and the canonical proof that the generator pattern works for executable logic, not just alias tables.
|
||||
|
||||
**Scope:**
|
||||
- Promote `sdk/src/query/state-document.ts` to a source under `sdk/src/state-document/index.ts` (or keep in place — decided in implementation).
|
||||
- Write `sdk/scripts/gen-state-document.ts` that emits `get-shit-done/bin/lib/state-document.generated.cjs` (and optionally re-exports the TS form at its existing location).
|
||||
- Write `sdk/scripts/check-state-document-fresh.mjs` modeled on `check-command-aliases-fresh.mjs`.
|
||||
- Replace `bin/lib/state-document.cjs` content with a thin re-export from `state-document.generated.cjs`. Keep the existing filename so callers (e.g. `workstream-inventory.cjs:16`) don't need to update imports.
|
||||
- Wire `check-state-document-fresh.mjs` into CI alongside `check-command-aliases-fresh.mjs`.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [ ] `bin/lib/state-document.cjs` contains only a re-export from `state-document.generated.cjs`.
|
||||
- [ ] `sdk/scripts/check-state-document-fresh.mjs` passes in CI and fails when intentionally desynchronized.
|
||||
- [ ] All existing call sites (CJS: `state.cjs`, `workstream-inventory.cjs`; SDK: `state-mutation.ts`, `state-project-load.ts`, others importing `state-document`) work unchanged.
|
||||
- [ ] Existing STATE.md unit tests on both sides pass.
|
||||
- [ ] CONTEXT.md "STATE.md Document Module" entry is amended (one sentence) to note the source-of-truth file path.
|
||||
|
||||
**Rollback:** Revert the branch. Re-importing the deleted CJS file content from git history restores the prior hand-synced shape. No external consumer is broken.
|
||||
|
||||
---
|
||||
|
||||
### Phase 2 — Configuration Module (closes the #3523 class)
|
||||
|
||||
**Why second.** This is the Module that triggered the work. It is the highest-leverage drift surface and the test of whether the pattern scales from a pure-transform Module to a Module that consumes data manifests.
|
||||
|
||||
**Scope:**
|
||||
- Add a **Configuration Module** entry to `CONTEXT.md` first. Definition: "Module owning config load, legacy-key normalization, defaults merge, and explicit on-disk migration for `.planning/config.json`." Interface and invariants per ADR §6.
|
||||
- Extract `CONFIG_DEFAULTS`, `VALID_CONFIG_KEYS`, `DYNAMIC_KEY_PATTERNS`, `RUNTIME_STATE_KEYS` to two data manifests: `sdk/shared/config-schema.manifest.json` and `sdk/shared/config-defaults.manifest.json`. Precedent: `sdk/shared/model-catalog.json`.
|
||||
- Write the Configuration Module source at `sdk/src/configuration/index.ts`. Implementation imports the two manifests and exports `loadConfig`, `normalizeLegacyKeys`, `mergeDefaults`, `migrateOnDisk`.
|
||||
- Write `sdk/scripts/gen-configuration.ts` to emit `get-shit-done/bin/lib/configuration.generated.cjs` and (if needed) `sdk/src/query/config-schema.generated.ts`.
|
||||
- Write `sdk/scripts/check-configuration-fresh.mjs`.
|
||||
- Replace the inline implementations in `bin/lib/core.cjs:loadConfig` (lines 220–243, 434–449, 485) and `bin/lib/config.cjs` (the validation surface) with thin Adapters over the generated Module. Delete the inline `CONFIG_DEFAULTS`, the false-positive warning at `core.cjs:444-449`, and the duplicated `_deepMergeConfig`.
|
||||
- Replace `sdk/src/config.ts:mergeDefaults` (lines 192–218) with a re-export from the new Module.
|
||||
- Extend `sdk/src/golden/read-only-parity.integration.test.ts` with a fixture matrix for the four legacy-key normalizations: top-level `branching_strategy`, top-level `sub_repos`, `multiRepo: true`, top-level `depth`.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [ ] `CONTEXT.md` contains a Configuration Module entry with the Interface contract.
|
||||
- [ ] `bin/lib/core.cjs` and `bin/lib/config.cjs` contain no local `CONFIG_DEFAULTS` or `VALID_CONFIG_KEYS` literals; both load from the manifests via the generated Module.
|
||||
- [ ] Bug #3523 fixture matrix passes on both CJS and SDK paths; the false-positive warning at the old `core.cjs:444-449` site is gone.
|
||||
- [ ] Golden parity matrix green for all four legacy-key shapes.
|
||||
- [ ] Bug #3523 closed with a back-reference to this phase.
|
||||
|
||||
**Rollback:** Revert the branch; inline implementations restore from git history. The manifest files remain unreferenced.
|
||||
|
||||
---
|
||||
|
||||
### Phase 3 — Workstream Inventory Builder + remaining hand-synced pairs
|
||||
|
||||
**Why third.** Phase 1 proves the pattern for pure transforms. Phase 2 proves it for data-manifest-backed logic. Phase 3 generalizes across the remaining hand-synced pairs surfaced by the audit. The Workstream Inventory Module is the headline because it requires the **Builder/Reader split** — the projection logic is pure and shareable, but the directory traversal is legitimately sync (CJS) vs async (SDK). This is the pattern for every paired Module with mixed pure-and-I/O concerns.
|
||||
|
||||
**Scope:**
|
||||
- Write the Workstream Inventory Builder source at `sdk/src/workstream-inventory/builder.ts`. Pure function: takes a list of directory entries plus per-workstream STATE.md text plus plan-scan results and returns the typed `WorkstreamPhaseInventory`/`WorkstreamInventory` projection. No fs reads.
|
||||
- Write `sdk/scripts/gen-workstream-inventory-builder.ts` to emit `get-shit-done/bin/lib/workstream-inventory-builder.generated.cjs` and `sdk/src/query/workstream-inventory-builder.generated.ts`.
|
||||
- Write `sdk/scripts/check-workstream-inventory-builder-fresh.mjs`.
|
||||
- Refactor `bin/lib/workstream-inventory.cjs` to a sync Reader Adapter: does `fs.readdirSync` + `readFileSync` of STATE.md, calls the Builder. The projection logic is removed.
|
||||
- Refactor `sdk/src/query/workstream-inventory.ts` to an async Reader Adapter: same shape, async I/O, calls the Builder.
|
||||
- Amend the `CONTEXT.md` "Workstream Inventory Module" entry with a sub-paragraph documenting the Builder/Reader split.
|
||||
- Audit remaining likely pairs (`frontmatter.cjs`↔`frontmatter-mutation.ts`, `plan-scan.cjs`↔`plan-scan` SDK equivalents) for pure-transform sharability. For each confirmed-shareable pair, apply the same Builder pattern in this phase. For pairs whose duplication is structural (e.g. routing tables, sync vs async with different return shapes), document the decision in the phase issue and defer.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [ ] `bin/lib/workstream-inventory.cjs` and `sdk/src/query/workstream-inventory.ts` no longer share projection logic; both call the generated Builder.
|
||||
- [ ] CONTEXT.md "Workstream Inventory Module" entry reflects the split.
|
||||
- [ ] Workstream-related golden tests pass on both sides.
|
||||
- [ ] Every additional Module in scope has its own freshness check.
|
||||
- [ ] Each Module not migrated in this phase has a one-paragraph deferral note (in the phase issue, not in the ADR).
|
||||
|
||||
---
|
||||
|
||||
### Phase 4 — Project-Root Resolution Module
|
||||
|
||||
**Scope:**
|
||||
- Add a **Project-Root Resolution Module** entry to `CONTEXT.md`. Interface: `findProjectRoot(startDir)`, `findEffectiveRoot(startDir, options)`.
|
||||
- Source at `sdk/src/project-root/index.ts`. Pure function: takes a path and an injected fs probe (or just uses `node:fs` since both runtimes have it synchronously).
|
||||
- Generator at `sdk/scripts/gen-project-root.ts`.
|
||||
- Freshness check at `sdk/scripts/check-project-root-fresh.mjs`.
|
||||
- Replace `bin/lib/core.cjs:74-140` with a thin Adapter over the generated Module.
|
||||
- Replace `sdk/src/helpers.ts:497-630` with a thin Adapter over the same Module.
|
||||
- Extend parity tests for: standalone project, monorepo with `planning.sub_repos`, legacy `multiRepo: true`, deep nesting.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [ ] `findProjectRoot` is defined exactly once in source form.
|
||||
- [ ] Both sides import the generated Module.
|
||||
- [ ] Parity tests pass for the four configurations above.
|
||||
|
||||
---
|
||||
|
||||
### Phase 5 — CJS Command Router Adapter: delegate to the SDK runtime bridge
|
||||
|
||||
**Why fifth.** Phases 1–4 collapse drift in *shared* logic. Phase 5 collapses drift in *parallel* logic — the per-side state/verify/init/phase/roadmap/validate handler implementations on the CJS side. After Phase 5, every canonical command running via `gsd-tools` executes the same SDK handler that `gsd-sdk query` executes, in-process, with no subprocess hop. The seam becomes a real wall.
|
||||
|
||||
**Scope:**
|
||||
- Amend the existing `CJS Command Router Adapter Module` CONTEXT.md entry to document runtime-bridge delegation.
|
||||
- Expose a synchronous-friendly entry on `QueryRuntimeBridge` for CJS callers. Today `QueryRuntimeBridge.execute()` is async; the bridge gains a `executeForCjs(input) → { exitCode, stdoutChunks, stderrLines }` synchronous wrapper that runs the dispatch under `deasync` or a controlled `runUntil` semantic. (Toolchain choice resolved in the Phase 5 issue; if synchronous bridging is not viable, fall back to `Atomics.wait` on a worker channel — never `gsd-sdk` subprocess.)
|
||||
- Replace each canonical-family `handlers` map in `bin/lib/*-command-router.cjs` with a generated delegate emitter that, per subcommand, calls `executeForCjs({ canonical, argv, env, cwd })` and writes the result through the existing CJS output Adapter.
|
||||
- For each canonical command family in order — `state.*`, `verify.*`, `phase.*`, `phases.*`, `validate.*`, `roadmap.*`, `init.*`, `frontmatter.*`, `config.*`, plus the non-family commands listed in `sdk/src/query/command-manifest.non-family.ts` — migrate one family per sub-PR. Run the golden parity matrix per family before merging.
|
||||
- Delete CJS-side handler files (or shrink to delegates) for each migrated family: `state.cjs`, `verify.cjs`, `init.cjs`, `phase.cjs`, `phases.cjs`, `validate.cjs`, `roadmap.cjs`, `milestone.cjs`, `frontmatter.cjs`, `config.cjs` write paths, plan-scan handlers, etc. The pure-transform Shared Modules from Phases 1–4 remain untouched; only the per-family handler entry points are replaced.
|
||||
- CJS-only Module handlers (`graphify`, `gsd2-import`, `schema-detect`, `fallow-runner`, `intel`, `drift`, `installer-migrations`) keep their in-process CJS implementations. They are not in the canonical family registry and do not route through the SDK runtime bridge.
|
||||
- Extend `sdk/src/golden/golden.integration.test.ts` to verify identical exit code + stdout chunks + stderr lines between `gsd-tools <family> <subcommand>` (now delegated) and `gsd-sdk query <canonical>` for every canonical command in the manifest.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [ ] CONTEXT.md "CJS Command Router Adapter Module" entry documents runtime-bridge delegation.
|
||||
- [ ] `QueryRuntimeBridge.executeForCjs` (or equivalent) ships with the synchronous semantics resolved in the phase issue.
|
||||
- [ ] Every canonical command family in `command-manifest.*.ts` routes via `executeForCjs`. CJS-only commands continue to route via the existing CJS handler.
|
||||
- [ ] Each per-family CJS handler file (`state.cjs`, `verify.cjs`, …) contains no command-specific logic — only the delegate wiring or has been deleted entirely.
|
||||
- [ ] Golden parity matrix verifies output equivalence across `gsd-tools` and `gsd-sdk` for every canonical command. No regressions in workflow markdown that calls `gsd-tools`.
|
||||
- [ ] Subprocess overhead per `gsd-tools` invocation does not increase (the bridge is in-process, not a `gsd-sdk` subprocess).
|
||||
|
||||
**Rollback (per family):** Each family's PR is independently revertible. The CJS handler files for an un-migrated family remain on disk in git history; if a family's delegation regresses, revert that family's PR and the CJS-side handler is restored.
|
||||
|
||||
**Out-of-scope under Phase 5:** The CJS-only Modules (graphify, gsd2-import, etc.) and workflow markdown that calls them — those calls continue to hit the in-process CJS handler, no change. Migrating CJS-only Modules to SDK is a separate enhancement.
|
||||
|
||||
---
|
||||
|
||||
### Phase 6 — Enforcement hardening + retrospective
|
||||
|
||||
**Scope:**
|
||||
- Write `scripts/lint-shared-module-handsync.cjs`. Greps for any pair of files at `get-shit-done/bin/lib/<name>.cjs` and `sdk/src/query/<name>.ts` (or `sdk/src/<name>.ts`) where neither file matches `*.generated.*` and the pair is not on an explicit allow-list. Allow-list documents the cooperating-sibling exceptions (e.g. routing files where the implementations are structurally different).
|
||||
- Verify each Shared Module from Phases 1–4 has its own freshness check wired to CI.
|
||||
- Verify Phase 5's golden parity matrix covers every canonical command family.
|
||||
- Add CODEOWNERS rules for `sdk/src/<module>/**` for each Shared Module source-of-truth directory, for `sdk/shared/*.manifest.json`, and for `sdk/src/query-runtime-bridge.ts` (the Phase 5 boundary). Architecture-team review required.
|
||||
- Retrospectively walk the recurring-bug list (#1535 ... #3523). For each, document in `docs/agents/cjs-sdk-seam.md` which enforcement layer (handsync lint, freshness check, manifest data isolation, per-Module drift lint, runtime-bridge delegation) would have blocked it.
|
||||
- Write `docs/agents/cjs-sdk-seam.md` as a CONTRIBUTING-linked guide for adding a new Shared Module and for adding a new canonical command.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [ ] `lint-shared-module-handsync.cjs` runs in CI; demonstrated to block an intentional regression PR.
|
||||
- [ ] Every Shared Module from Phases 1–4 appears in a freshness-check workflow step.
|
||||
- [ ] Phase 5's golden parity matrix is in CI on every PR that touches `bin/lib/*` or `sdk/src/query/*`.
|
||||
- [ ] CODEOWNERS rules in place.
|
||||
- [ ] Retrospective document committed.
|
||||
- [ ] No PR can land that re-introduces the #3523 anti-pattern or that bypasses the runtime-bridge delegation for a canonical command.
|
||||
|
||||
---
|
||||
|
||||
## Cross-phase concerns
|
||||
|
||||
### Backwards compatibility
|
||||
|
||||
The CJS public CLI surface (`gsd-tools <subcommand>`) does not change. Flags, exit codes, stdout shapes preserved. Every phase replaces internal implementations behind the existing Module Interfaces; the external contracts are pinned by the existing golden parity suite plus the new fixture matrices.
|
||||
|
||||
### Performance
|
||||
|
||||
No subprocess overhead anywhere. The generated `.cjs` files are `require`-able CommonJS modules; the SDK consumes the TS source directly. Module load cost adds ≤ 10 ms per `require` across all phases combined.
|
||||
|
||||
Phase 5 specifically preserves the in-process model: `QueryRuntimeBridge.executeForCjs` runs the SDK handler in the same Node process as the CJS dispatcher. No `gsd-sdk` subprocess is invoked. Synchronous bridging adds at most a handful of microseconds per call vs the previous direct CJS handler invocation, dominated by the existing dispatch policy overhead.
|
||||
|
||||
### Build/install pipeline impact
|
||||
|
||||
- Each generator runs at build time on the developer machine (and in CI for the freshness check). No runtime generator execution.
|
||||
- The published `get-shit-done-cc` package already includes both `get-shit-done/bin/` and `sdk/dist/`. The generated `.cjs` files are committed to the repo (like `command-aliases.generated.cjs` today), so the install flow is unchanged — no on-install code generation.
|
||||
- `npm run build:sdk` continues to do what it does. Generators are invoked via `npm run gen:<module>` per the existing precedent.
|
||||
|
||||
### Risks
|
||||
|
||||
| Risk | Likelihood | Mitigation |
|
||||
|---|---|---|
|
||||
| Generator output drifts from source between commits | Medium | `check-<module>-fresh.mjs` per Module catches this at PR time. Precedent already in use for command-aliases. |
|
||||
| A Shared Module's TS source uses features not expressible in CommonJS output | Low | Generator emits a CJS-compatible subset (no ESM-only syntax in source). Existing `gen-command-aliases.ts` template covers this. |
|
||||
| Phase 2's removal of inline `_deepMergeConfig` changes a subtle merge semantic | Medium | Golden parity matrix is the test. If `_deepMergeConfig` and the new Module disagree on a fixture, the matrix fails and the new Module is amended before merge. |
|
||||
| `migrateOnDisk` rollout silently changes user-visible behavior on upgrade | Medium | `migrateOnDisk` is explicit and opt-in; installer calls it once on next upgrade, with a release-note entry. Standalone command `gsd-tools migrate-config` for manual invocation. |
|
||||
| CODEOWNERS rule slows down architecture-team responsiveness | Medium | Apply CODEOWNERS only to source-of-truth directories and manifests. Adapters and `.generated.*` files remain open. Architecture team commits to a ≤ 24 h SLA. |
|
||||
| Phase 3's audit surfaces more pairs than expected, scope creeps | Medium | Each non-Phase-1/2 Module is scope-checked in its phase issue. Pairs that don't fit cleanly are deferred with a documented reason. |
|
||||
| Phase 5's synchronous-bridging mechanism (`executeForCjs`) has no clean shape — `deasync` is C++-bound, `Atomics.wait` requires a Worker, refactoring every SDK handler to be sync is huge | High | Phase 5 spike resolves this before any family migration. If no clean mechanism exists, Phase 5 is descoped to the families whose SDK handlers are already synchronous, and the remainder shift to a follow-up enhancement. |
|
||||
| Phase 5 family migrations regress observable CJS output (exit codes, stdout/stderr shape) | Medium | Golden parity matrix per family is the gate. A family's PR cannot merge until the matrix is green across every canonical command in that family. |
|
||||
| Phase 5 changes startup time because the SDK runtime bridge eagerly loads more handlers than the previous CJS routers | Low | Lazy-load handlers behind the bridge (already the SDK's model). Measure `time gsd-tools state load` before/after migration; fail the family PR if median latency regresses >20 ms. |
|
||||
|
||||
### Open questions (resolved before the phase that depends on them)
|
||||
|
||||
1. **Phase 1 source location** — `sdk/src/state-document/index.ts` (move) vs `sdk/src/query/state-document.ts` (in place). Decided when Phase 1 PR is drafted.
|
||||
2. **Phase 2 manifest format** — JSON vs JSONC vs TypeScript-as-source. Decided in Phase 2. JSON wins unless we need comments for invariants documentation.
|
||||
3. **Phase 3 sibling-Module audit** — exact list of pairs that get Builder-split vs deferred. Decided as a deliverable of Phase 3's spike.
|
||||
4. **Phase 5 synchronous-bridging mechanism** — `executeForCjs` implementation strategy: `deasync` native module (battle-tested but C++ binding), `Atomics.wait` on a worker channel (zero-binding but spins a Worker), or refactor every async SDK handler to expose a sync entry point (cleanest but largest scope). Decided in the Phase 5 spike issue before any family migration begins.
|
||||
5. **Phase 5 family migration order** — which canonical family migrates first. Recommended order: smallest read-only family first (likely `frontmatter.*` or `config.* read paths`) as the proof of pattern, then state/verify/phase/roadmap/validate/init in increasing complexity. Decided in the Phase 5 issue.
|
||||
6. **Phase 6 retrospective format** — table vs prose. Decided when the retrospective document is drafted.
|
||||
|
||||
## Done when
|
||||
|
||||
`#3524` is closed when all six phases have shipped, each with its own merged PR closing its own phase issue, and the Phase 6 retrospective confirms every historical drift bug from the recurring list would have been blocked by one of the five enforcement layers (handsync lint, freshness check, manifest data isolation, per-Module drift lint, runtime-bridge delegation).
|
||||
@@ -24,4 +24,4 @@ The GitHub-assigned issue number is the prefix. Do not compute a sequential numb
|
||||
|
||||
| PRD | Title | Status |
|
||||
|-----|-------|--------|
|
||||
| _(none yet — new PRDs use `<issue#>-<slug>.md` naming)_ | | |
|
||||
| [3524-cjs-sdk-hard-seam.md](3524-cjs-sdk-hard-seam.md) | CJS↔SDK hard seam — phased migration (#3524) | Reference |
|
||||
|
||||
Reference in New Issue
Block a user