Skip to content

Audit engine/lib for dead exports and missing wiring: every exported TS module must have a reachable production caller or be removed #311

Description

@arndvs

Problem

shft/engine/lib/ contains ~30 standalone TypeScript modules plus the workflow runners under shft/engine/workflows/. An import scan of the production (non-test) graph finds several exported modules with no reachable consumer: dispatch.ts, parse-cli-args.ts, parse-diff-lines.ts, retry-feedback.ts, and semaphore.ts each report zero production import sites. Yet each ships a corresponding *.test.ts that exercises it in isolation. A module kept alive only by its own test is ambiguous in the worst way:

  • It may be genuinely dead code that the CLI/scripts no longer call — removable, shrinking the runtime boundary.
  • It may be wired indirectly — invoked through .sandcastle/engine/ vendored copies, script entrypoints, or config-generated call sites that escape the static import graph — in which case the "live" path is unguarded by the same tests and can rot silently.
  • It may represent a feature whose helper shipped but whose caller never landed — meaning the intended behavior silently never runs in production.

Because tests pin these modules in place, deletion is hard today without breaking a green suite — the classic "delete is hard because responsibilities are tangled" hazard. The architectural gap is that there is no single accounting of which exported engine symbols are actually reachable when Sandcastle runs, and no contract protecting that boundary from drift.

Goal

Make "every exported engine symbol has a reachable production caller (or is intentionally public API for consumers under .sandcastle/engine)" a verifiable invariant, and either rewire or remove the modules that fail it.

Proposed scope

  1. Establish module-truth for each suspect module. For dispatch, parse-cli-args, parse-diff-lines, retry-feedback, semaphore, and the lower-confidence set (pipeline-states, lint-pipeline-labels, render-pipeline-artifacts): trace every reach path including (a) shft/engine/workflows/*, (b) the scripts/pipeline-artifacts.ts entrypoint, (c) .sandcastle/engine/ vendored copies, and (d) any shft/templates generated call sites. Classify each as LIVE / DEAD / SHIPPED-AS-PUBLIC.

  2. Remove genuinely dead modules (no consumer anywhere) and delete their now-orphaned tests, confirming npx tsc --noEmit and npx vitest run stay green and ctrl update-sandcastle --dry-run reports no drift.

  3. Rewire or flag SHIPPED-AS-PUBLIC modules. If a module is intentionally public API for .sandcastle/engine consumers, name that contract explicitly (export set, doc) so it is not mistaken for dead code in future audits.

  4. Close wiring gaps. Where a suspect module is LIVE only through generated/vendored paths, add a smoke or integration assertion at the engine boundary that exercises it through the real call path (not its isolated unit test), so the live path is covered by the same protection as its unit test.

  5. Land a boundary check. Add a lightweight script (extend the existing pipeline-artifacts pattern) that greps the non-test graph for lib/* import sites and warns on exported modules with zero production callers, so the invariant is re-verifiable on every run rather than a one-time exercise.

Acceptance criteria

  • Every exported symbol in shft/engine/lib/ is classified LIVE / DEAD / SHIPPED-AS-PUBLIC in a committed manifest (or the classification is derivable from a script).
  • DEAD modules and their orphaned tests are removed; tsc --noEmit, vitest run, and update-sandcastle --dry-run all pass.
  • Every LIVE module reachable only via vendored/generated paths has a boundary-level assertion covering its real call path.
  • The boundary-check script runs in CI and fails on a newly-exported module with no production caller unless it is declared SHIPPED-AS-PUBLIC.

Out of scope / notes

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    source:architecture-reviewPRDs proposed by the automated architecture-review workflow

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions