fix(E1-12): make shared graph-test teardown deterministic

[davidru85]

Summary

• Make shared graph-test teardown deterministic by cancelling test-owned state-holder scopes and collectors before the graph releases its native SQLite handle.
• Add a reusable AppGraphTestHarness, migrate every Kotlin caller-owned graph holder scope in :shared tests, and remove direct backgroundScope.launch collectors.
• Audit every shared test class that mounts an AppGraph and update project continuity records.
• Resolve the second review round: prevent an orphaned child Job when harness construction fails, make all harness-test teardown exception-safe, remove obsolete imports, and refresh the continuity record with final report-derived counts.

Story

E1-12 - FuelEntryStateHolderTest Kotlin/Native SIGSEGV on Graph Close - S

Addresses #42.

Ready Check

• Backlog story is explicit
• Acceptance criteria reviewed
• Dependencies checked
• Required decisions are not Proposed or Pending
• Normative sections reviewed
• Expected verification identified
• Human review gates identified before work

In-Progress Checkpoint

• Date: 2026-09-02
• Branch and base: story/E1-12-shared-test-graph-close-race, based on main at 64c91d6
• Current phase and latest commit: all phases complete; final HEAD e4c641e, last code-bearing commit 4d86f82 (RED ed698e0, GREEN 55f075c, REFACTOR a7fb619, continuity checkpoints b23b4d2/a4009df, review fixes 27030d6/fa69083/eb8755f, second review round RED 5b9cb46, GREEN 92bc607, REFACTOR 4d86f82, documentation 9258821, CI-evidence checkpoints f62516d, 6776810 and e4c641e)
• Push and pull-request status: every commit is pushed and the ten required checks passed on the final HEAD e4c641e in run 33678743365 with no reruns; this pull request is open and MUST NOT be merged by the agent
• Completed since the previous checkpoint: prevented an orphaned child Job when harness construction fails (scheduler validated before scopeJob creation, RED-confirmed); made every harness-test teardown exception-safe with nested try/finally; removed the obsolete DatabaseFactory/DatabaseHandle imports from AppGraphCloseTest.kt; refreshed the handoff continuity record, Files Changed list and final test counts; appended a correction entry to docs/PROJECT_LOG.md
• Verification evidence and known failures: final post-review counts from generated reports: 32 tests, 0 failures, 0 skipped on each of :shared:testAndroidHostTest and :shared:iosSimulatorArm64Test; complete non-instrumented command passed with 627 actionable tasks; contractCheck reports no unresolved decisions and no PENDING assertions; git diff --check clean; source audit finds no direct backgroundScope.launch state-holder collector under shared/src/commonTest
• Open decisions or blockers: none; only the mandatory human review remains
• Exact next step: owner review and merge of this pull request

Scope Completed

• Added a child-scope-owning test harness whose close() cancels and joins its scope before closing the graph.
• Centralized eager state-holder collector launches in the harness (UnconfinedTestDispatcher).
• Migrated FuelEntryStateHolderTest, AppGraphContractTest, VehicleFormStateHolderTest, and VehicleListStateHolderTest.
• Audited all shared test classes that mount an AppGraph.
• Second review round: scheduler validation precedes scopeJob creation, so a failed construction leaves no orphaned child Job; all harness-test teardown is exception-safe.

Acceptance Evidence

• AppGraphTestHarnessTest.closeCancelsCollectorsBeforeClosingTheGraph deterministically observes collectors-cancelled before graph-closed on Android host and Kotlin/Native.
• constructorThrowsWhenParentScopeHasNoTestCoroutineScheduler retains the parent Job explicitly and asserts that a failed construction leaves no children attached; the assertion was RED against the previous implementation ([SupervisorJobImpl{Active}] observed) and its teardown cancels the parent Job even if an assertion fails.
• collectorsRunEagerlyOnUnconfinedTestDispatcher proves emissions are observed eagerly.
• No backgroundScope.launch and no state-holder factory call receiving backgroundScope remains under shared/src/commonTest.
• Final report-derived counts: 32 tests, 0 failures, 0 skipped on each shared target.
• The complete audit is recorded in docs/handoff-E1-12.md.

Out of Scope / Not Done

• Production hardening of AppGraph.close(), state-holder production lifecycle changes and DatabaseFactory changes remain explicitly deferred. Pursuing that question requires a separate story because it changes D-89 and touches gated core/database/**.

Files Changed

• AGENTS.md
• README.md
• docs/BACKLOG.md
• docs/PROJECT_LOG.md
• docs/handoff-E1-12.md
• shared/src/commonTest/kotlin/com/ruizurraca/carapp/AppGraphCloseTest.kt
• shared/src/commonTest/kotlin/com/ruizurraca/carapp/AppGraphContractTest.kt
• shared/src/commonTest/kotlin/com/ruizurraca/carapp/AppGraphTestHarness.kt
• shared/src/commonTest/kotlin/com/ruizurraca/carapp/AppGraphTestHarnessTest.kt
• shared/src/commonTest/kotlin/com/ruizurraca/carapp/FuelEntryStateHolderTest.kt
• shared/src/commonTest/kotlin/com/ruizurraca/carapp/RecordingDatabaseFactory.kt
• shared/src/commonTest/kotlin/com/ruizurraca/carapp/VehicleFormStateHolderTest.kt
• shared/src/commonTest/kotlin/com/ruizurraca/carapp/VehicleListStateHolderTest.kt

Decisions Made

• No technical decision was introduced; the implementation follows the E1-12 acceptance criteria and preserves D-89 unchanged.
• The owner's NETWORK phase label was interpreted as RED, because this test-only story has no network behavior and the requested sequence otherwise matches the repository TDD workflow.
• The owner explicitly requested one push after the three local phase commits, superseding the default push-after-each-phase cadence for this story (first round).

Verification Run

• Relevant tests pass
• Lint passes (ktlint, detekt)
• Coverage thresholds hold
• Architecture checks pass
• Contract check passes
• Relevant builds pass (Android, iOS simulator, Shared framework from :composition:ios)
• Documentation updated if behaviour, decisions or models changed

Commands or checks run (second review round, final):

./gradlew :shared:testAndroidHostTest --tests com.ruizurraca.carapp.AppGraphTestHarnessTest.constructorThrowsWhenParentScopeHasNoTestCoroutineScheduler --rerun-tasks
  Expected RED: BUILD FAILED; expected no children on the retained parent Job but observed
  [SupervisorJobImpl{Active}] — the failed constructor orphaned scopeJob.

./gradlew :shared:testAndroidHostTest --tests com.ruizurraca.carapp.AppGraphTestHarnessTest --rerun-tasks
  BUILD SUCCESSFUL; 3/3 harness tests passed after the scheduler-before-scopeJob fix.

./gradlew :shared:testAndroidHostTest :shared:iosSimulatorArm64Test :shared:ktlintCheck :shared:detekt --rerun-tasks
  BUILD SUCCESSFUL in 16s; all 143 actionable tasks executed; 32 tests, 0 failures, 0 skipped
  on each target (generated-report counts).

./gradlew ktlintCheck detekt architectureCheck contractCheck :build-logic:convention:test koverVerify :androidApp:assembleDebug :androidApp:testDebugUnitTest testAndroidHostTest iosSimulatorArm64Test -x :integration:firebase-auth:iosSimulatorArm64Test -x :integration:firebase-firestore:iosSimulatorArm64Test -x :wiring:firebase:iosSimulatorArm64Test -x :composition:ios:iosSimulatorArm64Test
  BUILD SUCCESSFUL; 627 actionable tasks.

./gradlew contractCheck
  No unresolved decisions, no PENDING assertions.

git diff --check
  Clean.

Source audit: no backgroundScope.launch state-holder collector and no state-holder factory call
receiving backgroundScope under shared/src/commonTest; backgroundScope appears only as the
harness parentScope argument.

Historical first-round evidence (retained for the TDD record; superseded by the final counts above):

First-round RED: closeCancelsCollectorsBeforeClosingTheGraph expected [collectors-cancelled,
graph-closed] but observed [graph-closed].
First-round GREEN: 30 tests passed on each shared target.
First-round REFACTOR stability: 10/10 consecutive forced iosSimulatorArm64Test runs on local
Apple silicon; historical CI run 33625103198 passed all 10 jobs (3/3 successful macOS
shared-tests executions).

Review Findings Resolution

First round:

• Collector dispatcher regression: collect() now launches through UnconfinedTestDispatcher resolved from the parent scope's TestCoroutineScheduler, preserving scopeJob parentage; collectorsRunEagerlyOnUnconfinedTestDispatcher proves eager execution.
• Deduplicated the recording database test double into a shared RecordingDatabaseFactory.kt; hardened first-round teardown; added the constructor and eager-dispatcher unit tests.

Second round (current):

• Finding 1 (orphaned child Job): extended constructorThrowsWhenParentScopeHasNoTestCoroutineScheduler to retain the parent Job and assert no children remain; RED confirmed against the old implementation; fixed by validating the scheduler before creating scopeJob. Valid harness behavior unchanged: collectors remain children of scopeJob, run eagerly through UnconfinedTestDispatcher, and are cancelled and joined before graph.close().
• Finding 2 (exception-safe teardown): nested try/finally blocks in every harness test so owningFactory.close() still runs when harness.close() or graph.close() throws; obsolete DatabaseFactory/DatabaseHandle imports removed from AppGraphCloseTest.kt.
• Finding 3 (continuity record): handoff checkpoint, Files Changed (now including AppGraphCloseTest.kt and RecordingDatabaseFactory.kt, matching git diff --name-status main...HEAD), report-derived final counts and historical/final evidence separation refreshed; append-only correction entry added to docs/PROJECT_LOG.md.
• Finding 4 (PR description): this description was refreshed after the final push to agree with the handoff.

Contract Impact

• No contract changes

Decision Board Impact

• No decision changes

Shared-Write Modules Touched

• None

Project Log Entry

• Entry appended to docs/PROJECT_LOG.md (story entry plus append-only correction entry)

CI Evidence

• Final HEAD e4c641e: run https://github.com/davidru85/carApp/actions/runs/33678743365 passed all ten required jobs: ktlint (45s), detekt (38s), architecture-check (37s), contract-check (1m41s), android-assemble (44s), android-instrumented-tests (2m34s), shared-tests (3m22s), ios-simulator-build (12m12s), objc-header-golden-check (2m29s), provider-decoupling (2m25s). No reruns needed on this run.
• Code-identical documentation-only HEADs also passed all ten jobs: 9258821 in run 33667021311 and f62516d in run 33669085132 (attempt 4, after three macOS runner flakes: two AndroidxDriverConnectionPool.close() crashes in ViewModelLifecycleTests — the E1-11-documented signature — and one XCUITest swipe-gesture flake; all on commits that touch no code the iOS host tests link).
• The intermittent AppGraph.close() connection-pool signature remains the explicitly deferred production-hardening follow-up.

Human Review Gate

Gates are defined canonically in AGENTS.md.

• Not applicable
• Gated path (AGENTS.md repository-state update)
• E1-12 explicit backlog human-review requirement

The agent does not merge this pull request.

Risks or Follow-ups

• The production safety question for graph close remains outside this story under D-89.
• The issue [Bug]: FuelEntryStateHolderTest segfaults on iosSimulatorArm64 when the graph closes while collectors are live #42 record was already closed before implementation; this PR supplies the missing implementation and verification evidence.