-
Notifications
You must be signed in to change notification settings - Fork 1
Closed
Labels
Description
Plan: Workspace obsolete test cleanup and de-risking
Overview
This plan defines a full-workspace cleanup pass for obsolete tests and stale test helpers across all crates under crates/*.
The goal is to remove tests that no longer protect behavior (for example dead-code-shield tests, deprecated-path leftovers, and unreferenced helper functions), while preserving contract fidelity for CLI output, warnings, color behavior, JSON shape, and exit semantics.
Execution is organized as sprint-level integration gates: inventory first, safe helper cleanup next, high-risk behavior cleanup after that, then prevention/guardrail hardening.
No cross-sprint execution parallelism is planned; parallelization is only used within each sprint.
Scope
- In scope:
- All Rust test surfaces in this workspace (
crates/*/tests,#[cfg(test)]modules, test fixtures tightly coupled to removed behavior). - Test helper modules such as
tests/common.rs,tests/support/mod.rs,tests/real_common.rs, and similar scaffolding. - Obsolete
#[allow(dead_code)]usage in tests when the underlying helper is now unused. - Test cleanup reporting artifacts under
$AGENT_HOME/out/.
- All Rust test surfaces in this workspace (
- Out of scope:
- Product feature changes unrelated to test cleanup.
- Intentional parity/contract tests that still guard user-visible behavior.
- Major architecture migrations outside test surfaces.
Assumptions
- Contract fidelity remains the top priority: no unapproved drift in human output text, warning style, color behavior, JSON contracts, or exit codes.
- A test is considered obsolete only when it no longer guards reachable behavior or contractual compatibility.
- Each sprint is a hard integration gate and must pass its validation commands before moving to the next sprint.
- Temporary reports and inventories are written to
$AGENT_HOME/out/workspace-test-cleanup/(not as canonical repo docs). - Final delivery must pass required repo checks and workspace coverage gate from
DEVELOPMENT.md.
Success criteria
- Every crate has an explicit stale-test classification record with rationale (
remove,keep,rewrite,defer). - Unused test helper functions/modules are removed or consolidated, and related dead-code allowances are minimized.
- Deprecated/removed-path tests are cleaned without reducing contract coverage.
- Required checks and workspace coverage remain green after cleanup.
- A repeatable stale-test audit workflow exists to prevent re-accumulation.
Task Decomposition
| Task | Summary | Owner | Branch | Worktree | Execution Mode | PR | Status | Notes |
|---|---|---|---|---|---|---|---|---|
| S1T1 | Build workspace stale-test scanner and normalized output schema | subagent-s1-t1 | issue/s1-t1-build-workspace-stale-test-scanner-and-normalize | issue-s1-t1 | per-sprint | #262 | done | sprint=S1; plan-task:Task 1.1; validate=mkdir -p "$AGENT_HOME/out/workspace-test-cleanup"; pr-grouping=per-sprint; pr-group=s1; shared-pr-anchor=S1T1 |
| S1T2 | Build unused-helper call graph for test scaffolding modules | subagent-s1-t1 | issue/s1-t1-build-workspace-stale-test-scanner-and-normalize | issue-s1-t1 | per-sprint | #262 | done | sprint=S1; plan-task:Task 1.2; deps=Task 1.1; validate=test -s "$AGENT_HOME/out/workspace-test-cleanup/helper-callgraph.tsv"; pr-grouping=per-sprint; pr-group=s1; shared-pr-anchor=S1T1 |
| S1T3 | Define stale-test decision rubric and contract-protection allowlist | subagent-s1-t1 | issue/s1-t1-build-workspace-stale-test-scanner-and-normalize | issue-s1-t1 | per-sprint | #262 | done | sprint=S1; plan-task:Task 1.3; deps=Task 1.1; validate=test -s "$AGENT_HOME/out/workspace-test-cleanup/decision-rubric.md"; pr-grouping=per-sprint; pr-group=s1; shared-pr-anchor=S1T1 |
| S1T4 | Produce sprint execution manifest and crate-tier lanes | subagent-s1-t1 | issue/s1-t1-build-workspace-stale-test-scanner-and-normalize | issue-s1-t1 | per-sprint | #262 | done | sprint=S1; plan-task:Task 1.4; deps=Task 1.2,Task 1.3; validate=test -s "$AGENT_HOME/out/workspace-test-cleanup/execution-manifest.md"; pr-grouping=per-sprint; pr-group=s1; shared-pr-anchor=S1T1 |
| S2T1 | Remove orphaned helper functions in low-risk crates | subagent-s2-t1 | issue/s2-t1-remove-orphaned-helper-functions-in-low-risk-cra | issue-s2-t1 | per-sprint | #263 | done | sprint=S2; plan-task:Task 2.1; deps=Task 1.4; validate=cargo test -p nils-git-scope -p nils-image-processing -p nils-api-testing-core -p nils-git-summary -p nils-plan-tooling; pr-grouping=per-sprint; pr-group=s2; shared-pr-anchor=S2T1 |
| S2T2 | Reduce obsolete #[allow(dead_code)] and crate-level dead-code shields in test helpers |
subagent-s2-t1 | issue/s2-t1-remove-orphaned-helper-functions-in-low-risk-cra | issue-s2-t1 | per-sprint | #263 | done | sprint=S2; plan-task:Task 2.2; deps=Task 1.4; validate=cargo test -p nils-fzf-cli -p nils-semantic-commit -p nils-git-lock -p nils-memo-cli -p nils-agent-docs; pr-grouping=per-sprint; pr-group=s2; shared-pr-anchor=S2T1 |
| S2T3 | Consolidate nils-test-support helper primitives (atomic lane A) |
subagent-s2-t1 | issue/s2-t1-remove-orphaned-helper-functions-in-low-risk-cra | issue-s2-t1 | per-sprint | #263 | done | sprint=S2; plan-task:Task 2.3; deps=Task 2.1,Task 2.2; validate=cargo test -p nils-test-support; pr-grouping=per-sprint; pr-group=s2; shared-pr-anchor=S2T1 |
| S2T4 | Adopt consolidated helpers in medium-risk crates (atomic lane B) | subagent-s2-t1 | issue/s2-t1-remove-orphaned-helper-functions-in-low-risk-cra | issue-s2-t1 | per-sprint | #263 | done | sprint=S2; plan-task:Task 2.4; deps=Task 2.3; validate=cargo test -p nils-git-cli -p nils-git-lock -p nils-semantic-commit -p nils-plan-issue-cli -p nils-macos-agent; pr-grouping=per-sprint; pr-group=s2; shared-pr-anchor=S2T1 |
| S2T5 | Tier-2 regression sweep and fallout fixes | subagent-s2-t1 | issue/s2-t1-remove-orphaned-helper-functions-in-low-risk-cra | issue-s2-t1 | per-sprint | #263 | done | sprint=S2; plan-task:Task 2.5; deps=Task 2.4; validate=cargo test -p nils-git-cli -p nils-plan-issue-cli -p nils-macos-agent -p nils-screen-record -p nils-git-summary -p nils-fzf-cli; pr-grouping=per-sprint; pr-group=s2; shared-pr-anchor=S2T1 |
| S3T1 | Identify deprecated/removed-path tests and fixture dependencies | subagent-s3-t1 | issue/s3-t1-identify-deprecated-removed-path-tests-and-fixtu | issue-s3-t1 | per-sprint | #264 | done | sprint=S3; plan-task:Task 3.1; deps=Task 2.5; validate=test -s "$AGENT_HOME/out/workspace-test-cleanup/deprecated-path-candidates.tsv"; pr-grouping=per-sprint; pr-group=s3; shared-pr-anchor=S3T1 |
| S3T2 | Remove obsolete deprecated-path tests and stale fixtures | subagent-s3-t1 | issue/s3-t1-identify-deprecated-removed-path-tests-and-fixtu | issue-s3-t1 | per-sprint | #264 | done | sprint=S3; plan-task:Task 3.2; deps=Task 3.1; validate=cargo test --workspace; pr-grouping=per-sprint; pr-group=s3; shared-pr-anchor=S3T1 |
| S3T3 | Rebaseline parity and contract suites for current command surfaces | subagent-s3-t1 | issue/s3-t1-identify-deprecated-removed-path-tests-and-fixtu | issue-s3-t1 | per-sprint | #264 | done | sprint=S3; plan-task:Task 3.3; deps=Task 3.2; validate=cargo test -p nils-codex-cli -p nils-gemini-cli -p nils-plan-issue-cli -p nils-api-rest -p nils-api-gql; pr-grouping=per-sprint; pr-group=s3; shared-pr-anchor=S3T1 |
| S3T4 | Full workspace regression gate after high-risk cleanup | subagent-s3-t1 | issue/s3-t1-identify-deprecated-removed-path-tests-and-fixtu | issue-s3-t1 | per-sprint | #264 | done | sprint=S3; plan-task:Task 3.4; deps=Task 3.3; validate=./.agents/skills/nils-cli-verify-required-checks/scripts/nils-cli-verify-required-checks.sh; pr-grouping=per-sprint; pr-group=s3; shared-pr-anchor=S3T1 |
| S4T1 | Add strict CI stale-test audit command | subagent-s4-t1 | issue/s4-t1-add-strict-ci-stale-test-audit-command | issue-s4-t1 | per-sprint | #265 | done | sprint=S4; plan-task:Task 4.1; deps=Task 3.4; validate=mkdir -p "$AGENT_HOME/out/workspace-test-cleanup"; pr-grouping=per-sprint; pr-group=s4; shared-pr-anchor=S4T1 |
| S4T2 | Document stale-test lifecycle and review checklist | subagent-s4-t1 | issue/s4-t1-add-strict-ci-stale-test-audit-command | issue-s4-t1 | per-sprint | #265 | done | sprint=S4; plan-task:Task 4.2; deps=Task 3.4; validate=rg -n 'stale test/remove/keep/rewrite/defer/test-stale-audit' docs/runbooks/test-cleanup-governance.md DEVELOPMENT.md AGENTS.md; pr-grouping=per-sprint; pr-group=s4; shared-pr-anchor=S4T1 |
| S4T3 | Wire stale-test audit into required check flow and run final workspace gate | subagent-s4-t1 | issue/s4-t1-add-strict-ci-stale-test-audit-command | issue-s4-t1 | per-sprint | #265 | done | sprint=S4; plan-task:Task 4.3; deps=Task 4.1,Task 4.2; validate=./.agents/skills/nils-cli-verify-required-checks/scripts/nils-cli-verify-required-checks.sh; pr-grouping=per-sprint; pr-group=s4; shared-pr-anchor=S4T1 |
Consistency Rules
Statusmust be one of:planned,in-progress,blocked,done.Status=in-progressordonerequires non-TBDexecution metadata (Owner,Branch,Worktree,Execution Mode,PR).Ownermust be a subagent identifier (containssubagent) once the task is assigned;main-agentownership is invalid for implementation tasks.Execution Modeshould be one of:per-sprint,pr-isolated,pr-shared(orTBDbefore assignment).BranchandWorktreeuniqueness is enforced only for rows usingExecution Mode = pr-isolated.
Risks / Uncertainties
- Sprint approvals may be recorded before final close; issue stays open until final plan acceptance.
- Close gate fails if task statuses or PR merge states in the issue body are incomplete.
Evidence
- Plan source:
docs/plans/workspace-obsolete-tests-cleanup-plan.md - Sprint approvals: issue comments (one comment per accepted sprint)
- Final approval: issue/pull comment URL passed to
close-plan
Reactions are currently unavailable