Repository navigation
Description
CodeRabbit minor findings on PR #2840 — deferred follow-ups
PR #2840 (fix for #2573) was reviewed by CodeRabbit at head 831ac5dc (2026-10-30T23:17Z), which raised 3 Minor findings. One was already fixed by the next head; two are valid-but-minor and are tracked here instead of blocking the merge (Codex reviewed the final head 8db2e07b clean; the PR's own acceptance in #2573 covers the 4 Codex findings from #2541, all fixed).
1. Workflow log formatter retains every parsed event in memory — .github/workflows/mcpx.yml:187
The readable-output formatter appends every parsed go test -json event to lines before printing in two passes. Retained memory grows with the full stream. Suggested fix (from the review thread): re-read the file per pass with a generator instead of retaining events. Thread: #2840 (comment)
2. Hook failure text can be polluted by stdout — pkgs/mcpx/internal/hooks/runner.go
execute() points both cmd.Stdout and cmd.Stderr at one shared cappedWriter, and the failure text is firstLine(out.String()). A hook that fills the cap with stdout (no newline) before writing its stderr diagnostic records truncated stdout as the "error". Suggested fix: give stderr its own capped writer, build the failure text from stderr's first line, and fall back to stdout's first line when stderr is empty. Add a regression test (hook writes >cap stdout, then a one-line stderr diagnostic, exits non-zero; Stats().LastError names the diagnostic). Thread: #2840 (comment)
Already fixed (no action)
The third finding (wildcard hook's Dropped counting hook.* events, thread #2840 (comment)) was fixed in head 8db2e07b by Hook.SubFilter() (pkgs/mcpx/internal/hooks/hooks.go), which expands a wildcard to the hookable kinds so hook.* events are excluded before the bus buffers them.
Acceptance
- Formatter streams per pass (no full event retention) with identical log output
- stderr has its own capped writer; failure text prefers stderr, falls back to stdout; regression test above passes
-
go test ./internal/hooks/...green in CI
Acceptance criteria
- The mcpx workflow log formatter streams parsed
go test -jsonevents without retaining the full event list in memory (or the retention is bounded and documented). - Each deferred CodeRabbit minor finding from PR fix(mcpx): disposition the 4 undispositioned Codex findings from PR #2541 (#2573) #2840 listed in this issue is either fixed or explicitly dispositioned as wontfix with a reason.
- mcpx CI stays green on the change; no behavior change to the formatted output content.
Activity
dezren39 commentedon Oct 31, 2026
Intake review
Related: PR #2840 (merged work for #2573) — CodeRabbit minor findings deferred here by design. Risks: the log formatter change touches CI readability; keep output format identical. Suggested approach: stream-print events in one pass; disposition each listed finding in the PR description.
— Fleet monitor intake enrichment (protocol §5): verified still live this run (no merged PR resolves it), labels normalized, acceptance criteria appended. Marked
meta/enriched.dezren39 commentedon Nov 3, 2026
agent-note:v1 — task 7985 picked up by lane code-k (repo-worker-code). Execution lane assigned: opencode — handed off to the opencode claimant, which will launch a bound session to triage issue #2880 at current main.
dezren39 commentedon Nov 3, 2026
🤖 Agent note · handed off
opencode; re-handing off to the laptop claimant (earlier handoff at 14:36Z expired unlaunched).13 remaining items
developing-today commentedon Nov 3, 2026
agent-note:v1
state: triaged (coverage pass)
Coverage pass 2026-11-03: reviewed in the fleet-wide per-issue pass (drift / overlap / already-handled / stale / blockers / ordering). Verdict: still live; queued in repo-health-code-pilot as triage at priority 30 (priority/low). If this is already handled at current main, the triage closes it with evidence; if it needs splitting, the triage files the split.
Found via: coverage pass 2026-11-03
dezren39 commentedon Nov 3, 2026
🤖 Agent note · parked (CI still pending)
bf419306-c4ef-470f-a553-beabf93239de(OpenChamber scheduled lane), task 7985 (attempt 5).946bbf059, not draft. Mergeable state is UNKNOWN andbasic: production line limit (2500)is still pending (Actions backlog — no run has started); CodeRabbit still NEUTRAL/skipping. Attempt-4's local verification (go tests ok, vet clean, formatter byte-parity) stands unrebutted, but the merge criteria (checks green) are not met — parking rather than merging on local evidence alone.mcpxchecks fail, fix forward on the same branch.dezren39 commentedon Nov 3, 2026
dezren39 commentedon Nov 3, 2026
🤖 Agent note · picked up
repo-worker-code), OpenCode laneses_e5e5d8677cfffe2r4ZAMWCJVaNmain, tests, and PR checks; follow ownership restrictions before any merge.fix(mcpx): separate hook stderr cap, stream the CI log formatter (#2880…
dezren39 commentedon Nov 3, 2026
🤖 Agent note · done
repo-worker-code), OpenCode laneses_e5e5d8677cfffe2r4ZAMWCJVaN821c021055b2ba7d716dd023e3c3a86cfafeb6c9; this closes mcpx: CodeRabbit minor follow-ups from PR #2840 — stream the CI log formatter, separate stderr cap for hook failure text #2880.go test ./internal/hooks/...passed;go vet ./internal/hooks/...passed; production-line-limit CI check passed; formatter fixture parity was recorded in the prior verification and PR summary.