Skip to content

First-ten audit maintenance: CI concurrency, Greptile scopes and context, wait exit tests - #23

Open
krishhgg wants to merge 10 commits into
masterfrom
t3term/first-ten-ci-greptile-maintenance
Open

krishhgg wants to merge 10 commits into
masterfrom
t3term/first-ten-ci-greptile-maintenance

Conversation

@krishhgg

@krishhgg krishhgg commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner

Each master commit gets its own CI run, Greptile checks more of the wire-reading and CLI code, and three integration tests cover attention, failed-run and timeout outcomes. This completes the warranted CI/config changes from Astra and Fable's first-ten audit.

Sol passed formatting, strict Clippy and all 296 tests at d21868c: 274 library, 2 main, 12 CLI and 8 fake-RPC tests. The final b3d3796 changes only Greptile prose/scope and the audit record; its source, tests and workflow are identical. The diff check and Greptile JSON/context validation pass. Final CI passed on b3d3796. Greptile reviewed that same commit, gave it 5/5 and found no actionable issue.

Decision log

What I noticed

Astra and Claude Fable 5.1, both at xhigh, independently audited sprint PRs #8 through #17. They found cancelled master CI runs, gaps in Greptile's protocol/cost coverage and missing CLI wait outcomes. The context file still called an earlier proposal the agreed plan.

How I confirmed it

Sol checked all ten cancelled master runs from #8 through #21; none had a job. The CLI tests run the actual binary with temporary homes and fake HTTP, WebSocket and auth endpoints. Plain and JSON cases verify exits 7, 1 and 6, partial replies, exactly one JSON result, and the timeout's sent message ID. Sol also verified master's ruleset now requires macOS and Greptile Review.

Options

Group PR runs by number and master runs by commit SHA. Keeping one master group still replaces pending runs even when cancellation is off. Expand the existing protocol and idle-cost rules, and use README for current commands, the parity documents for the pinned baseline and source references, and sprint.md for progress. Reuse the existing fake-server harness. A queue redesign or new testing framework was unnecessary.

What review changed

Sol reconciled both audits. #20 had already fixed repeated handoff descriptions, so this PR leaves that code alone. Sol added models.rs to protocol coverage and corrected reconnect guidance: shell/thread streams resume with afterSequence; config subscriptions use an empty request and a fresh snapshot. Claude kept #22's changes when bringing master into this branch. The audit record tracks 15 merged sprint PRs and the next audit at 20. Greptile's final review found no actionable issue.

Known limits

This changes no product code or TUI layout. The raw-control-character gap in plain CLI output needs a separate maintenance PR. Attention coverage exercises a command approval, not a pending question. The timeout test takes about two seconds across its two runs. Failed-run JSON retains the existing ok: true and outcome: "failed" contract. Maintenance PRs do not count toward the next audit.

RetriggerConfidence Score: 5/5

This PR appears safe to merge; no actionable issue was found.

Summary

This PR updates CI grouping, review guidance, and CLI wait tests without changing product code.

  • Each master commit gets its own CI run group.
  • Greptile checks more of the code that reads server data and CLI streams.
  • CLI integration tests cover approval, failure, and timeout outcomes.

Acknowledgments supplied by krishhgg:

  • Raw control characters in existing plain CLI output are deferred to a separate maintenance PR.
  • Attention coverage intentionally tests command approval, not a pending question.
  • Failed-run JSON intentionally keeps ok: true with outcome: "failed".
  • The timeout test intentionally takes about two seconds across its two runs.

Reviews (1) · Last reviewed commit: "greptile: cover models.rs and state each..." · Reviewed by Greptile

krishhgg and others added 9 commits October 9, 2026 22:16
GitHub cancels a pending run when a newer run joins its concurrency group,
even with cancel-in-progress off. Master pushes shared one group through
github.ref, so a quick series of merges cancelled most of their checks.
Pull requests still group by number and cancel their running check.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
main.rs, transcript.rs and src/tui/ read nightly fields, so protocol-fidelity
now covers them. idle-cost now covers main.rs and transcript.rs and names the
per-event work a CLI stream can repeat. The instructions list the modules
and tests that exist now.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
research/next-steps.md was listed as the agreed plan, but it is the earlier
proposal and its CLI syntax is stale. The context now names README.md as the
command contract, docs/parity.md as the plan pinned to the nightly, with its
statuses as that baseline, docs/parity-inventory.md for the desktop's source
references and docs/sprint.md for the process and the merged PRs.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
wait exits 7 with the outcome needs-attention when T3 streams a pending
approval, and 1 with the outcome failed when the run fails. send --wait
exits 6 with THREAD_WAIT_TIMEOUT when the turn outlasts --timeout, and
the error names the message it sent. Each case checks the plain output
and that --json prints one object.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Each one is the mergeCommit GitHub reports for #15 to #21.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The record lists each finding from Astra and Claude Fable 5.1 with Sol's
outcome: what #23 fixes, what #20 had fixed, the ruleset change Sol made,
the CLI control-character gap left for its own PR, and the evidence for
the parts that needed no change.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
#22 merged at cb51f8c while this branch was open, and both added to the
README's tests paragraph and the sprint table. The merge keeps #22's text
and row, adds this branch's sentences after it and fills in row 15's merge
commit, so the record counts 15 merged sprint PRs.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@krishhgg krishhgg changed the title First-ten audit maintenance: CI concurrency, Greptile scopes, wait exit tests First-ten audit maintenance: CI concurrency, Greptile scopes and context, wait exit tests Oct 10, 2026
protocol-fidelity now includes src/models.rs, which reads the providers,
models and option descriptors in the server config. The rule and the
instructions no longer say every stream resumes with afterSequence. The
shell and thread subscriptions do, and subscribeServerConfig resubscribes
with an empty payload and starts from a fresh snapshot, as client.rs does
since #22. The audit table in docs/sprint.md records both changes.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@krishhgg
krishhgg marked this pull request as ready for review October 10, 2026 05:37
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.

1 participant