docs(issues): [#2238] plan native tracker fixture refactor

[josecelano]

Summary

• add the approved specification for refactoring the native tracker executable test fixture
• add the linked refactor plan as the sole technical task and implementation commit ledger
• define ownership invariants, AI-agent bounded-context goals, internal API freedom, module-layout constraints, maintenance task mapping, and verification requirements

Files

• docs/issues/open/2238-refactor-native-tracker-test-fixture/ISSUE.md
• docs/refactor-plans/open/2238-refactor-native-tracker-test-fixture.md

Validation

• TORRUST_GIT_HOOKS_LOG_DIR=.tmp ./contrib/dev-tools/git/hooks/pre-commit.sh
• all eight pre-commit steps passed, including cargo machete, cargo deny check bans, nightly formatting, linter all, Hadolint, and workspace documentation tests

Related to #2238

[Copilot]

🟡 Changes recommended

Unresolved verification, lifecycle, link, ownership, and plan-consistency issues remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Documentation-only PR adding the issue specification and refactor plan for the native tracker executable test fixture.

Changes:

• Defines scope, ownership invariants, module boundaries, and acceptance criteria.
• Adds the implementation task ledger and verification requirements.

File summaries

File	Summary	Final findings
docs/issues/open/2238-refactor-native-tracker-test-fixture/ISSUE.md	Issue specification and verification plan	[Major][F1] (nit, 1 vote): manual verification lacks concrete actions and child-process coverage. [Major][F2] (nit, 3 votes): GitHub issue still links to the stale drafts path. [Minor][NTR-004] (nit, 1 vote): failed-start ownership transfer chain is not documented.
docs/refactor-plans/open/2238-refactor-native-tracker-test-fixture.md	Refactor design and technical task ledger	[Minor][F3] (nit, 1 vote): command.rs documentation is not required. [Minor][F4] (nit, 1 vote): root documentation omits required exclusions. [Minor][F5] (nit, 1 vote): old path references are not updated for the proposed layout. [Major][2238-2] (moderate, 1 vote): verification omits configuration-suite consumers. [Major][NTR-001] (nit, 1 vote): plan lifecycle status conflicts with a spec-only PR. [Minor][NTR-003] (nit, 1 vote): task ordering conflicts with repository workflow. [Minor][NTR-005] (nit, 1 vote): multiple primary modules are listed.

Review details

Suppressed comments (9)

docs/issues/open/2238-refactor-native-tracker-test-fixture/ISSUE.md:227

• [Major][F1] These rows are structural code-reading exercises, not concrete manual verification processes: they provide no command or interaction to execute, and M1/M2 do not exercise the child-process behavior at all. The issue/template requires real human-oriented actions with actual observations recorded in manual-verification-evidence.md; for a fixture-only refactor, either define exact runtime/review steps and expected evidence here or explicitly justify why structural review is the only applicable manual check.

| ID | Scenario | Human-oriented command/steps | Expected Result | Status | Evidence |
| --- | --- | --- | --- | --- | --- |
| M1 | Trace a running tracker lifecycle | Starting from a signal test, navigate through its final fixture API into startup, readiness, shutdown, and drop cleanup; record the owner of each child, output reader, and workspace. | The consumer reads clearly, the complete normal lifecycle is understandable without entering failed-start implementation, and every resource has one clear owner. | TODO | `manual-verification-evidence.md` section V1 |
| M2 | Trace a failed-start lifecycle | Starting from an invalid-source test, navigate through its final fixture API into source preparation, spawn, diagnostics, permission restoration, wait, and drop fallback; record the owner and deadline at each stage. | The consumer reads clearly, the complete failure lifecycle is understandable without entering normal readiness orchestration, and cleanup responsibilities and deadlines remain explicit. | TODO | `manual-verification-evidence.md` section V2 |
| M3 | Walk the maintenance task map | For each row of the plan's task map, open only the named primary module and collaborators and confirm the described change could be made there; note any row that would require opening another module. | Every row holds; any exception is recorded and either fixed or justified. | TODO | `manual-verification-evidence.md` section V3 |

docs/issues/open/2238-refactor-native-tracker-test-fixture/ISSUE.md:122

• [Minor][NTR-004] This ownership row names NativeTrackerWorkspace or the final failed-start result, but the failed path actually transfers its TempDir through NativeTrackerStartAttempt and NativeTrackerFailedStart before it can be retained by NativeTrackerFailedStartResult. Since the specification requires every workspace owner to remain visible until cleanup, please document that transfer chain rather than leaving the pre-result owners implicit.

| Temporary workspace and configuration files | `NativeTrackerWorkspace` (`command.rs`) or the failed-start result | Paths remain available for diagnostics until the applicable child cleanup is complete. |

docs/refactor-plans/open/2238-refactor-native-tracker-test-fixture.md:233

• [Minor][F3] AC6 requires every fixture module to have a //! comment describing both its ownership and limits, but this extraction step never asks for documentation on command.rs (the output/health and failed-start steps do). Add that requirement here so the workspace/configuration module cannot be left undocumented while still satisfying the plan's checklist.

**Change**: Move `CONFIGURATION`, `NativeTrackerConfigurationSources`, `NativeTrackerWorkspace`,
`write_configuration`, `write_configuration_in_directory`, `tracker_command`,
`configure_tracker_command`, and `tracker_binary`. Re-export `NativeTrackerConfigurationSources`
from the root. Move the `tracker_command` and `write_configuration` tests. Keep the three item-level
dead-code allowances on the environment/override builders.

docs/refactor-plans/open/2238-refactor-native-tracker-test-fixture.md:272

• [Minor][F4] The stated module-documentation contract requires each module to say what it owns and what it must not do, but this root step only requests an ownership summary. Explicitly include the root exclusions from the responsibility map (failed-start, rendering, and probe implementation) so the final //! documentation satisfies AC6 rather than relying on an implicit interpretation.

**Change**: Order the root as: module doc, child `mod` declarations, `pub use` re-exports,
lifecycle constants, `NativeTracker` struct, `impl NativeTracker`, `impl Drop`. Extend the `//!` doc
with a short ownership summary matching the responsibility map. No behavior changes.

docs/refactor-plans/open/2238-refactor-native-tracker-test-fixture.md:196

• [Minor][F5] If the recommended mod.rs layout is selected, tests/common/native_tracker.rs is removed, but this item only updates the two Rust consumers. The issue spec and plan contain live references to the old path in frontmatter and prose, so the move can leave broken documentation and an incorrect related-artifact inventory; include those updates here while preserving historical progress-log paths.

**Change**: Select the layout that produces the clearest module ownership and consumer imports.
The recommended option remains a `tests/common/native_tracker/mod.rs` root, but narrower direct
imports are allowed if they better separate running and failed-start fixtures. Update both
consumers and run both binaries.

docs/refactor-plans/open/2238-refactor-native-tracker-test-fixture.md:153

• [Major][2238-2] This row covers running-tracker deadlines as well as drop cleanup, but the configuration suite also exercises NativeTracker::start_with_configuration_sources, wait_until_ready, and gracefully_shutdown in tests/configuration/cli_configuration/base_source_precedence.rs:21-31 and per_value_overrides.rs:18-28. A root lifecycle change can regress those paths while lifecycle-signals alone passes; use the combined command so the verification map covers both consumers.

| Change a running-tracker deadline or drop-path policy | root | none | `cargo test --test lifecycle-signals` |

docs/refactor-plans/open/2238-refactor-native-tracker-test-fixture.md:5

• [Major][NTR-001] This plan is already in docs/refactor-plans/open/ with status: open, but this PR is explicitly spec-only and the implementation has not started. The repository lifecycle requires plans to remain in drafts/ while awaiting review and move to open/ only when implementation starts (docs/refactor-plans/open/README.md:18-21). Please keep the plan in drafts/status draft until implementation begins, or update the lifecycle and all links consistently.

status: open
related-issue: 2238
spec-path: docs/refactor-plans/open/2238-refactor-native-tracker-test-fixture.md

docs/refactor-plans/open/2238-refactor-native-tracker-test-fixture.md:161

• [Minor][NTR-003] The plan explicitly orders items dependency-first, but the repository refactor-plan workflow requires the items and execution table to be sorted by highest impact first, then lowest effort within each impact band. Here the High-impact failed-start/API work (items 5 and 7) follows several Medium-impact items. Please either reorder the ledger and preserve dependencies in a separate note, or document and formally adopt this exception in the governing workflow.

Items are ordered dependency-first so every step is a compiling, behavior-preserving move: shared
leaf collaborators (`output`, `health`, `command`) move before the module that depends on them
(`failed_start`), and the root is trimmed last. Child modules can access the root's private items,
so any order compiles; leaf-first keeps each diff a pure move with `pub(super)` visibility.

docs/refactor-plans/open/2238-refactor-native-tracker-test-fixture.md:149

• [Minor][NTR-005] The bounded-context rule says each maintenance task has one primary module, but this row lists both failed_start.rs and the consumer test file under Primary module to open. Make failed_start.rs the single primary and list invalid_sources.rs as a collaborator so this map can actually verify AC7.

| Add an invalid CLI source case | `failed_start.rs` and `tests/configuration/cli_configuration/invalid_sources.rs` | `command.rs` only if a new rendering helper is needed | `cargo test --test cli-configuration` |

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.