test: diagnose restart startup cascades

[mohanagy]

Summary

• add isolated start-count markers for both upstreams in the existing bundled restart-settlement test
• emit only marker deltas and sanitized health state if its existing readiness poll fails
• retain the existing timeout and lifecycle behavior unchanged

Validation

• focused normal restart-settlement test
• npm test
• npm run test:coverage
• npm run lint -- tests/multi-upstream.test.ts
• npm run typecheck

Refs #122

Summary by CodeRabbit

• Tests
  • Strengthened audit recovery coverage for failed checkpoint replacement and integrity verification.
  • Added diagnostics for stalled audit writes and clearer recovery validation.
  • Expanded outcome tests for lazy upstream discovery and lifecycle health reporting.
  • Improved multi-upstream restart tests to verify readiness, pending behavior, failure handling, and continued availability of healthy tools.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 33 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7618a816-854f-45d8-9eeb-dd0a05f59f16

📥 Commits

Reviewing files that changed from the base of the PR and between fa97c5d and 7c63120.

📒 Files selected for processing (5)

• tests/audit-outcomes.test.ts
• tests/helpers/upstream-diagnostics.test.ts
• tests/helpers/upstream-diagnostics.ts
• tests/multi-upstream.test.ts
• tests/upstream-manager.test.ts

📝 Walkthrough

Walkthrough

The test suite adds execution-stage, filesystem-marker, start-count, timeout, and health-state diagnostics to audit recovery, lazy upstream discovery, and multi-upstream restart scenarios.

Changes

Test diagnostics and lifecycle coverage

Layer / File(s)	Summary
Audit integrity recovery diagnostics tests/audit-integrity.test.ts	The compaction rollback test tracks stages and writes, emits timeout diagnostics, clears the timer, and retains rollback verification.
Lazy upstream lifecycle diagnostics tests/audit-outcomes.test.ts	The lazy lifecycle test adds marker paths and reports discovery failures with start deltas and health snapshots.
Multi-upstream restart readiness diagnostics tests/multi-upstream.test.ts	The restart test tracks GitHub and Sentry starts, waits for readiness, and reports restart and health state when polling fails.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• mohanagy/miftah#47: Related multi-upstream restart-settle assertions.
• mohanagy/miftah#51: Related upstream lifecycle and restart health behavior.
• mohanagy/miftah#69: Related audit journal compaction rollback testing.

Poem

A bunny watches markers glow,
While audit ledgers heal below.
Start counts hop from file to file,
Health snapshots wait in tidy style.
“No hung tests!” the rabbit sings.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The summary and validation sections are present, but the required Security impact section is missing and validation lacks concrete results.	Add a Security impact section and include exact validation commands with observed results, especially any failing test seen first.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the main focus on restart startup diagnostics, though it doesn't mention the other test-only changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/122-lifecycle-startup-cascade-v2

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/audit-outcomes.test.ts`:
- Around line 150-153: Consolidate the duplicated countStarts helper and
health-mapping logic used by audit-outcomes tests with the existing shared
implementations in multi-upstream.test.ts or upstream-manager.test.ts. Remove
the local duplicates and reuse the established symbols while preserving the
current test behavior.
- Around line 159-187: Update the catch around client.listTools() to bind the
original error and include its details in the diagnostic error alongside the
existing marker deltas and health data. Preserve the current lazy discovery
diagnostics while ensuring the underlying RPC/transport failure is not
discarded.

In `@tests/multi-upstream.test.ts`:
- Around line 1465-1468: Remove the duplicated countStarts helper and
health-mapping logic from the multi-upstream tests, and reuse the corresponding
shared implementations from audit-outcomes.test.ts. Update the affected tests
around countStarts and the health mapper to call the reused symbols while
preserving their existing assertions and behavior.
- Around line 1477-1514: Update the try/catch around the readiness expect.poll
in the restart test to bind the caught error and preserve its original failure
details in the diagnostic error, alongside marker deltas and health. Keep the
existing readiness polling and diagnostic collection unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e17168cf-87e9-4e0e-82ef-afe3b27b1c34

📥 Commits

Reviewing files that changed from the base of the PR and between 88beb9b and fa97c5d.

📒 Files selected for processing (3)

• tests/audit-integrity.test.ts
• tests/audit-outcomes.test.ts
• tests/multi-upstream.test.ts

[mohanagy]

Addressed the four current review threads in 7c63120.

• Moved repeated fixture-start counting and allowlisted health summaries into one tested helper.
• Both diagnostic catches now retain the original failure in Error.cause, while the emitted message stays limited to marker deltas and safe health state.

Validated with the focused helper/audit/multi-upstream suite, typecheck, lint, diff check, and an independent review. The new exact-head CI run is pending.