Harness: catch exception-path cleanup + atomic session-load regressions

[larock22]

1) Outcome

Harness and CI reliably catch exception-path cleanup + session-load atomicity regressions before review.

2) Current vs Desired

Current: Semantic state-integrity regressions (exception-path cleanup gaps; load_session() partial mutation on failure) can land in PRs without any harness or CI gate failing, and are only caught by review/CodeRabbit.
Desired: The local harness and CI fail deterministically for these regressions (via targeted tests and/or running the pre-push stage in CI).

3) Scope

Allowed to change:

• CI workflows under .github/workflows/ (harness parity)
• HARNESS.md (documentation parity)
• Tests under tests/ (add failure-path regression coverage)
• Pre-commit wiring if needed to align stages

NOT allowed to change:

• Product behavior unrelated to harness enforcement
• Broad refactors unrelated to these failure-path invariants

4) Acceptance Criteria (Definition of Done)

• CI runs the equivalent of the pre-push stage checks (at minimum: mypy + pytest) on pull_request and fails when they fail.
• Add a regression test that fails if TextualReplApp request teardown leaves dangling state when the final delta flush raises.
• Add a regression test that fails if StateManager.load_session() returns False but mutates the live session (atomicity on failure).
• HARNESS.md accurately describes what runs in CI vs local (no claim that pre-push checks are enforced in CI if they are not).

5) Verification

Automated:

• uv run pre-commit run --hook-stage pre-push --all-files
• uv run pytest tests/unit/ui/test_request_threading.py tests/unit/core/test_session_usage_schema.py

Manual:

• Step 1: Open a PR and confirm CI includes a job that executes the pre-push-equivalent checks.
• Step 2: Introduce a small intentional regression locally (e.g., break one of the new failure-path tests) and confirm the check fails.
• Expected: CI blocks merge until the relevant check passes.

Evidence required:

• Test command output
• Short manual verification log (what PR/run, expected vs actual)

6) Bug-only (if applicable; otherwise N/A)

N/A

7) Links

• Related PR: [codex] perf: cache conversation total tokens and improve request debug flow #451
• Related: HARNESS.md
• Related: .pre-commit-config.yaml
• Related: .github/workflows/lint.yml

[tunahorse]

Current repo audit on master (2026-04-04): this issue is still open, and the CI/harness parity acceptance criteria do not appear fully satisfied yet.

What is true now:

• PR [codex] perf: cache conversation total tokens and improve request debug flow #451 merged and added adjacent regression coverage in the area.
• HARNESS.md documents mypy and pytest as pre-push hooks, which matches .pre-commit-config.yaml.

What still appears missing on master:

• .github/workflows/lint.yml still runs pre-commit --all-files, dead-code checks, orphan-module checks, and deptry, but I did not find a PR job that explicitly runs the pre-push-equivalent mypy + pytest stage.
• I did not find an explicit failure-path test for "final delta flush raises during request teardown".
• tests/unit/core/test_session_usage_schema.py verifies that load_session() can return False for a legacy payload, but I did not find an explicit assertion that a failed load leaves the live session unmodified.

Suggested triage status: keep open. The issue body can probably be narrowed to the still-missing items: CI parity for pre-push checks and explicit regression coverage for the two failure paths.

[tunahorse]

Current repo audit on master (2026-04-04): this issue is still open and only partially addressed.

What appears landed:

• There is now abort-cleanup regression coverage in tests/unit/core/test_request_orchestrator_parallel_tools.py:187-234 for reconciling in-flight tool state and dangling messages after abort.

What still appears missing from the acceptance criteria:

• CI on pull requests still does not run an explicit pre-push-equivalent mypy + pytest gate. In .github/workflows/lint.yml:8-40, the PR workflow runs pre-commit, dead-code checks, and deptry, but not standalone mypy and pytest jobs matching the pre-push stage.
• I did not find the requested request-teardown regression test for the exception path where the final delta flush raises. The closest tests/unit/ui/test_request_threading.py:180-198 only covers the happy-path flush behavior.
• I did not find an atomicity regression test for StateManager.load_session() failure. tests/unit/core/test_session_usage_schema.py:73-103 currently asserts only that a legacy payload returns False. On current master, src/tunacode/core/session/state.py:365-401 still mutates live session fields before returning False on failure, so the atomicity concern still appears real.

Suggested triage status: keep open. One regression area is now covered, but the CI parity item and at least one of the failure-path invariants are still unresolved.

[Keesan12]

The atomicity test will be much stronger if it snapshots the full live session, forces failure at each load boundary, then compares the post-failure state to the snapshot — not just the return value. For teardown, I’d also inject the final-delta exception and prove both the request registry and in-flight tool state are empty before the next request starts.

That distinction matters: a green command is not the same as proof that failure left the system clean. We built MartinLoop around these verifier receipts, and after 10,000+ downloads the failure-path receipts are consistently what people care about most. If you try it on this regression, send me the receipt and I’ll help tighten the invariant.