Skip to content

structural debt: sanction the module splits, leaf crates, and port redesigns — or close them won't-do #620

Description

@macanderson

What decision this asks for

None of these 22 items are defects. Each one is a move — split a god-module, extract a leaf crate, or reshape a port — and each was declined by the PR that found it for the same three reasons: it is a drive-by refactor with no witness, it collides textually with every other edit in the same file, and the boundary is an owner's architecture call rather than a mechanical one. So the question is not where the boundary should go. It is whether structural work is sanctioned at all, in what order, and with a merge freeze on the target file while it lands — because without that ruling these will keep resurfacing in every audit and keep getting declined for the same reason.

Concretely: rule each item in or out. For the ones ruled in, sequence them (a split and a port change on the same file cannot both be in flight) and declare the file frozen for the duration. For the ones ruled out, close them won't-do so a future audit stops re-reporting them.

Verified against main (b5a99b4) on 2026-07-25: all 22 are still unaddressed. stella-config, stella-store-schema, stella-subprocess and stella-cli/src/lib.rs do not exist; SessionLedgers, trait IssueTracker, trait Forge and guarded_panel_mut have no occurrences anywhere in the tree. Several line counts have drifted up since the audits were written — deck_ui.rs is now 6,880 (audited at 6,609), command_deck.rs 4,475 (4,310), pipeline.rs 2,351 (2,342), stella-graph/src/store.rs 1,159 (1,156). The debt is compounding, not static.

Items

Module splits (pure code motion, no defect behind them)

Leaf-crate extractions (new workspace members)

  • Extract a stella-store-schema leaf crate and read PRAGMA user_version. stella-observatory/src/db.rs:1. Deferred because it needs "a new workspace crate, a cross-crate API contract, and a new migration-guard test". (audit Batched audit cleanup: area:core (1/2) — 31 items #559, deferred by chore(audit): batched area:core cleanup (7 of 31 items) #590)
  • Add stella-cli/src/lib.rs re-exporting the existing modules, reduce main.rs to a fn main() { stella_cli::run() } shim, then extract the leaf half (config.rs, settings/, engine_config.rs, model_catalog.rs, env_files.rs — ~7k LOC) into a new stella-config workspace crate, so the reusable half of the 49k-LOC binary-only crate becomes linkable by other hosts (stella-serve, observatory) and integration-testable. stella-cli/Cargo.toml, stella-cli/src/main.rs. Deferred as "architectural restructuring of a 49k-LOC crate plus a new workspace member; it would collide with nearly every other item here". Note the premise has partly drifted — stella-cli/tests/inspect_cli.rs exists today, so "zero integration tests" is no longer true. (audit Batched audit cleanup: area:cli — 30 items #557, deferred by chore(stella-cli): apply the area:cli audit batch (14 of 30 items) #597)
  • Extract subprocess_env out of stella-tools into a stella-subprocess leaf crate, so stella-tui stops pulling all of stella-tools for the single call at stella-tui/src/deck_shell.rs:202. stella-tui/Cargo.toml:17. Deferred because "the fix creates a new workspace member with publish/license metadata, dependency rewiring in three crates, and a cargo-deny licensing surface. Its only payoff (build time) isn't measurable from a cold target dir here." (audit Batched audit cleanup: area:tui — 26 items #571, deferred by chore(stella-tui): clear the area:tui audit batch (9 items) #602)

Port redesigns (owner architecture calls, cross-crate blast radius)

  • Replace the closed IssueBackend enum with a trait IssueTracker port, and add a semantic ForgeBackend for stella-fleet, instead of 24 match arms of dispatch. stella-tools/src/issues.rs:27, issue_ops.rs, registry.rs, stella-fleet/src/monitor.rs. Deferred as a "cross-crate architectural change: 24 match arms across the 1,376-line issue_ops.rs plus issues.rs, registry.rs, stella-fleet/src/monitor.rs and the Command Deck's issue panel. That is a port-design decision (AGENTS.md's central invariant) and its blast radius cannot be verified inside a batched PR." (audit Batched audit cleanup: area:tools — 26 items #570, deferred by chore(stella-tools): bound tool output, harden sub-resource auth, prune the process table #603)
  • Promote the gh port from the CLI level (GhCli::run(&[&str]) -> stdout) to a domain-level trait Forge { async fn pr_status(...); async fn ci_runs(...); } with SystemGhCli as one implementation, making parse_run_list gh-specific rather than crate-wide so gh's vocabulary (--json status,conclusion,name, gh's exact status/conclusion strings) stops leaking into the crate. Only the --limit-into-WatchConfig fallback was taken. stella-fleet/src/monitor.rs:519, stella-cli/src/fleet_cmd.rs. Deferred because "the Fix text conditions it on 'If a second forge is ever in scope' — an architecture decision with no second forge in the repository today. It would restructure the crate's public port surface and every caller in stella-cli." (audit Batched audit cleanup: area:fleet — 14 items #562, deferred by chore(stella-fleet): apply the area:fleet audit batch #593)
  • Unify the three parallel, non-interoperating VCS ports (RepoBackend, stella_fleet::git::GitCli, stella-pipeline's ports) and reroute the ~20 direct Command::new("git") sites that bypass all of them. stella-tools/src/repo.rs:123, stella-fleet/src/git.rs. Deferred as "explicitly a risky cross-crate rename plus a port consolidation spanning four crates". (audit Batched audit cleanup: area:tools — 26 items #570, deferred by chore(stella-tools): bound tool output, harden sub-resource auth, prune the process table #603)
  • Narrow the storage ports (ExecutionSink, TelemetrySink, FileLockStore) so Store is fakeable, instead of consumers depending on the 47-method concrete struct. stella-store/src/lib.rs:549. Deferred as "a cross-crate redesign touching six crates that each open rusqlite directly, plus new public traits on a 47-method concrete struct. A design decision for the owner, and nothing about it is verifiable by running the suite." (audit Batched audit cleanup: area:store — 23 items #569, deferred by chore(stella-store): apply ten batched audit fixes from the area:store sweep #599)
  • Extract the six unrelated session ledgers hanging off ToolRegistry (touched, span_reads, citations, …) into a SessionLedgers struct. stella-tools/src/registry.rs:49. Deferred because it "changes ToolRegistry's construction API, which stella-cli depends on. The shape of the extracted aggregate is an owner architecture call, and the stella-serve benefit cannot be demonstrated here." (audit Batched audit cleanup: area:tools — 26 items #570, deferred by chore(stella-tools): bound tool output, harden sub-resource auth, prune the process table #603)
  • Emit BlockKind::Attachment drafts and classify User blocks by provenance. stella-core/src/receipts.rs:141, stella-core/src/driver.rs. Deferred because "provenance needs a new seam threaded from the driver (a message-index → BlockKind index maintained across the turn), because the summary/steer markers are private consts and a plain steer carries no marker. That is a design change, not a local edit." (audit Batched audit cleanup: area:core (1/2) — 31 items #559, deferred by chore(audit): batched area:core cleanup (7 of 31 items) #590)
  • Route bash / run_custom through exec::drive rather than each tool carrying its own copy of the arm/disarm guard logic — the duplication between exec.rs, bash.rs and custom.rs stays. stella-tools/src/exec.rs, bash.rs, custom.rs. Deferred because "exec.rs's module doc states explicitly that the bash tool deliberately does not come through drive, so crate::sandbox can replace the program; run_custom needs piped stdin and per-tool env that drive does not model. Collapsing that boundary is a design change, not this issue's scope." (audit stella-tools: the bash tool orphans its detached process group on cancellation #550, deferred by fix(stella-tools): kill the bash and custom-tool process groups on cancellation #582)
  • Add a guarded_panel_mut panic boundary for the deck. stella-tui/src/deck_render.rs:41. Deferred, quoting the audit: "I did not attempt it: it changes a pub(crate) signature, needs a new soundness argument, and would touch every deck golden test." Plus: "writing a new AssertUnwindSafe soundness argument is owner judgment, and release panic = \"abort\" makes the guarantee partial by construction." (audit Batched audit cleanup: area:tui — 26 items #571, deferred by chore(stella-tui): clear the area:tui audit batch (9 items) #602)
  • Gate the replay fixture loader out of release builds. use std::fs; sits in a non-cfg(test) pub mod (declared at stella-pipeline/src/replay.rs:35) inside the orchestration crate — the only filesystem I/O across stella-core / stella-protocol / stella-pipeline — so the release binary carries fixture-loading code and the GoldenError surface, and the crate's I/O-free property becomes "almost" rather than provable. Fix would split the pure parts (validate_stream, structural_diff, parse_jsonl, RecordingSource/manifest types) from the loader and gate the loader behind #[cfg(any(test, feature = "fixtures"))] or move it to tests/. stella-pipeline/src/replay/golden.rs:38. Deferred because "the loader is consumed by the crate's own integration tests as external users of the public API (tests/replay_fixtures.rs, tests/reference_conformance.rs), so gating it needs a new Cargo feature wired through dev-dependencies and removes public items under default features — the same public-API change other items here declare out of scope. It also touches the Makefile's golden-refresh path." (audit Batched audit cleanup: area:pipeline — 15 items #567, deferred by test(pipeline): name the work-item test modules for what they pin #594)

Why these are together

These share no subsystem. What they share is a shape: the only obstacle to each is moving code, each was disclaimed as a drive-by refactor with no defect behind it, and each collides textually with every other edit in the same file — which is precisely why every audit that found one also declined it. Bundling them makes the real question visible exactly once, as a scheduling and sanction decision rather than twenty-two separate re-litigations of "should we refactor this file".

Deferred during the #546 audit remediation. Put Closes #<this issue> in the PR description and as a commit trailer when it lands.

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Polish — worth doing, not urgent

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions