Skip to content

De-flake the journal pending-gap and version-probe drain tests - #994

Merged
realtonyyoung merged 1 commit into
mainfrom
ai-2843-flaky-daemon-unit-tests
Sep 18, 2026
Merged

realtonyyoung merged 1 commit into
mainfrom
ai-2843-flaky-daemon-unit-tests

Conversation

@realtonyyoung

Copy link
Copy Markdown
Collaborator

What & why

De-flakes two timing-sensitive tests in Capacitor.Cli.Daemon.Tests.Unit that failed once each on an unrelated PR (run 35034079520) and passed on rerun. The rerun was triage, not the fix.

Closes #970. Linear: AI-2843.

TranscriptJournalTests.Record_never_blocks_while_the_sink_hangs — asserted PendingGap > 0 after 100 records into a 4-deep queue. The sink only hung once the file had a line, and the header lands via Open, not the sink — so on a fast runner the writer could drain enough that the final TryWrite succeeded and PendingGap fell back to 0. The sink now hangs unconditionally, parking the writer from its first append so the overflow is structural. The wall-clock bound also moves 500ms → 5s (100 TryWrites on a loaded runner are not 500ms).

DaemonRunnerVersionProbeTests.A_version_whose_output_exceeds_the_pipe_buffer_is_drained_not_deadlocked — the stub flooded 500 KB to stderr before echoing the version on stdout, on a 3s launch-probe budget. Under load the yes | head pipeline overran the budget, the child was killed before the stdout echo, and the parse fell back to the stderr flood (0123456789ABCDEFGHIJ). Flood reduced to 200 KB — still 3× the OS pipe buffer, comfortably inside the budget.

Where to look

  • test/Capacitor.Cli.Daemon.Tests.Unit/Services/TranscriptJournalTests.cs
  • test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs

No production code changes — the probe and journal behave correctly; only the tests raced.

Verification

  • Both targets run 15× in isolation: 15/15 pass each.
  • Full Capacitor.Cli.Daemon.Tests.Unit suite: 3253 passed, 38 skipped, 1 failure (Installed_codex_schema_matches_the_vendored_pin, a known local environment pin, unrelated).
  • dotnet build clean, 0 warnings.

@linear-code

linear-code Bot commented Sep 17, 2026

Copy link
Copy Markdown

AI-2843

@qodo-code-review

qodo-code-review Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Committed report exposes a developer path ✗ Dismissed 🐞 Bug ⚙ Maintainability
Description
The added fix report records the developer's local checkout path and detailed push/session history
instead of repository-maintained project information. Anyone reading the repository can see
/Users/tony/Documents/kcap-cli and unrelated PR workflow details, while future maintainers must
distinguish this artifact from actual project documentation.
Code

.sdd-244-fix-report.md[R1-4]

+# PR #244 fix report — AI-684 ACP daemon foundation
+
+Branch: `ai-684-acp-protocol-foundation`
+Repo: `kurrent-io/kcap-cli` (worked in standalone clone `/Users/tony/Documents/kcap-cli`)
Relevance

●●● Strong

Absolute checkout paths and incidental workflow reports are inappropriate committed artifacts;
similar path-leak feedback was accepted.

PR-#643

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new file identifies a prior PR and branch, then includes the absolute local checkout path
/Users/tony/Documents/kcap-cli; the file is unrelated to the two test-only changes described by
this PR.

.sdd-244-fix-report.md[1-4]
.sdd-244-fix-report.md[284-304]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR adds `.sdd-244-fix-report.md`, an unrelated historical report containing a developer-local filesystem path and detailed prior-PR/session information. This is not needed to de-flake the two tests and leaks environment-specific details into the repository.

## Fix Focus Areas
- .sdd-244-fix-report.md[1-4]

## Recommended Fix
Delete `.sdd-244-fix-report.md` from the PR. If any project-relevant rationale must be preserved, move only a concise, repository-oriented explanation into the affected test comments or the PR description, removing local paths, push logs, and unrelated historical details.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Unrelated agent guidance reaches the repository ✗ Dismissed 🐞 Bug ⚙ Maintainability
Description
The new AGENTS.md adds repository-wide instructions and project claims unrelated to the
pending-gap and version-probe test fixes. Because agent tooling may consume this file automatically,
future automated changes can be guided by stale or out-of-scope protocol and workflow assertions
that this PR neither validates nor requires.
Code

AGENTS.md[R7-17]

+The `kcap` CLI records Codex sessions by forwarding hook payloads and transcript data to a Kurrent Capacitor server. It also hosts an agent daemon for remote Codex CLI management and provides PR review context via MCP tools.
+
+Review flows use a vendor-neutral catalog-start v2 protocol: reserved `spec-review`/`code-review`
+aliases select an explicit or server-default reviewer independently of the driver. Codex setup
+registers `kcap-flows` without auto-approval and tracks only newly-created global TOML entries in
+`mcp-ownership-v1.json`, so uninstall preserves manual/customized MCP configuration. Daemons retain
+the string unattended-vendor list for compatibility and additionally advertise structured
+per-vendor CLI/launcher-policy capabilities. Cursor serves borrowed review context from a
+daemon-owned snapshot (dirty tracked and non-ignored untracked files, refreshed between rounds),
+because its zero-interaction modes may write; Codex borrowed-worktree review remains uncertified
+and therefore fails closed to an owned worktree.
Relevance

●● Moderate

The guidance is broad and unrelated, but historical practice is mixed on accepting verbose
repository documentation.

PR-#285
PR-#390

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added guidance describes ACP/catalog-start protocols, Cursor review snapshots, MCP ownership,
issue workflow, and AOT publishing, none of which is related to either modified test. It also
declares itself repository-wide through its root-level location and contains imperative agent
instructions.

AGENTS.md[1-17]
AGENTS.md[54-73]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR adds a broad `AGENTS.md` containing protocol, vendor, worktree, and review-flow claims unrelated to the two test changes. Repository-wide agent instructions should be introduced in a dedicated documentation change with validated guidance, not bundled into a timing-test de-flake PR.

## Fix Focus Areas
- AGENTS.md[1-17]
- AGENTS.md[54-73]

## Recommended Fix
Delete `AGENTS.md` from this PR, or split it into a separately reviewed documentation change after verifying every instruction against the current repository. Keep this PR limited to the test changes and their directly relevant comments.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
Review mode: 🚀 Fast: The behavioral changes are narrowly localized to two timing-sensitive unit tests, with no production, API, security, or data-path changes; the remaining additions are documentation and repository guidance.

Grey Divider

Tip of the day
💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread .sdd-244-fix-report.md Outdated
Comment thread AGENTS.md Outdated
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

De-flake daemon timing tests and add repository guidance

🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Make journal queue overflow deterministic by blocking every asynchronous sink append.
• Reduce version-probe flood volume while retaining pipe-buffer drain coverage.
• Add repository guidance, a prior-PR fix report, and an empty root artifact.
Diagram

graph TD
  VPTest["Version Probe Test"] -->|"creates flood"| Stub["POSIX Flood Stub"] -->|"writes both streams"| Probe["Version Probe"] -->|"parses output"| Version["Parsed Version"]
  JournalTest["Journal Test"] -->|"records events"| Journal["Transcript Journal"] -->|"appends asynchronously"| Sink["Blocked Sink"] -->|"forces overflow"| Gap["Pending Gap"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Split unrelated repository artifacts
  • ➕ Keeps the PR focused on the two flaky daemon tests
  • ➕ Avoids publishing a stale report for another branch and PR
  • ➕ Prevents an unexplained empty root file from entering the repository
  • ➖ Requires removing these files or submitting repository guidance separately

Recommendation: Keep the deterministic sink and reduced flood-size test changes; they directly address the observed races while preserving coverage. Remove .sdd-244-fix-report.md and long, and move AGENTS.md to a separate documentation PR unless these additions are explicitly intended for this work.

Files changed (5) +398 / -4

Tests (2) +16 / -4
DaemonRunnerVersionProbeTests.csReduce oversized version-probe fixture output +6/-2

Reduce oversized version-probe fixture output

• Reduces the POSIX stub's stderr flood from 500 KB to 200 KB so it reliably completes within the three-second probe budget. The output remains substantially larger than the pipe buffer, preserving concurrent-drain coverage.

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs

TranscriptJournalTests.csMake pending-gap overflow structurally deterministic +10/-2

Make pending-gap overflow structurally deterministic

• Blocks every sink append from the first invocation so the bounded journal queue cannot drain before the overflow assertion. It also raises the non-blocking wall-clock allowance from 500 milliseconds to five seconds for loaded runners.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/TranscriptJournalTests.cs

Documentation (3) +382 / -0
.sdd-244-fix-report.mdAdd historical PR #244 fix report +309/-0

Add historical PR #244 fix report

• Adds a detailed report about conflict resolution, ACP fixes, verification results, commits, and push status for a different branch and pull request. It is unrelated to the daemon test de-flaking described by this PR.

.sdd-244-fix-report.md

AGENTS.mdAdd repository development guidance +73/-0

Add repository development guidance

• Documents project structure, build and test commands, PR conventions, NativeAOT precautions, and common implementation mistakes for coding agents and contributors.

AGENTS.md

longAdd empty root artifact +0/-0

Add empty root artifact

• Introduces a zero-byte file named 'long' without content or a documented repository purpose.

long

@realtonyyoung
realtonyyoung force-pushed the ai-2843-flaky-daemon-unit-tests branch from 6b8e42e to c7ec5c6 Compare September 17, 2026 21:35
@realtonyyoung
realtonyyoung merged commit 30b414d into main Sep 18, 2026
8 checks passed
@realtonyyoung
realtonyyoung deleted the ai-2843-flaky-daemon-unit-tests branch September 18, 2026 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky daemon unit tests: TranscriptJournal pending gap and version-probe pipe drain

1 participant