Act on a transition instead of logging it#54
Merged
Conversation
crq resolve and crq decline both act on a review thread. A review-body finding, a review-skipped notice and an outside-diff remark have none, so neither command can touch them — and drain-first then blocks every future round on a finding that can never drain. The observed end state was a PR reporting that no review was ever requested for its current head, four rounds running: the round could not start because the finding was undrained, and the finding could not drain because nothing could act on it. crq dismiss <repo> <pr> <finding-id>... --reason "<why>" records that the agent judged it. Dismissed findings are withheld from the action, and crq next reports dismissed: N so nothing looks silently dropped. Three choices worth stating. The reason is stored, not just demanded — a dismissal that discards its justification is not auditable. Finding IDs are content-derived rather than GitHub node IDs, so the repo and PR are required to identify one. And the record is scoped to the round for the current head: the same content yields the same ID, so a dismissal that outlived its head would silently swallow the finding when the next reviewer reports it again. Pushing supersedes the round and clears it, which is the rule body findings already follow. Dismissing enqueues the PR when crq is not yet tracking the current head, because that is the deadlock's own signature — no round for the head at all — and a dismissal with nowhere to live would change nothing.
Three review findings, all correct. Filtering happened in nextFromState, so a dismissal did nothing for crq feedback or crq loop: convergence and the exit code are computed from the full list, leaving the finding permanently actionable outside crq next. The filter now lives in Feedback, before Converged, so there is one of it. Nothing stopped dismissing a finding that HAS a thread. That would let a round converge with the thread still open, skipping the resolve/decline flow that puts the decision on the PR where the bot can answer it. A threaded finding is now refused by name, and so is an ID that is not a finding at this head at all — a stale copy-paste would otherwise record a dismissal that silences whatever later matches it. And the write is one CAS update again. Enqueueing first meant that a failure between the enqueue and the record left a fire-eligible round behind with nothing dismissed on it, which the autoreview daemon could claim and spend a review on. Dismiss now reads the findings, validates them, then creates or supersedes the round and records the decision in a single write.
Six review findings, all correct, and three of them P1. A round tracking a different head is now refused rather than superseded. If the head moved after Dismiss read the findings, superseding archives the live round and points the queue back at a commit nobody is looking at — and CAS retries cannot protect against a mutation that deliberately overwrites newer state. A round may only be CREATED when this call drains the head. Dismissing one of several open findings used to queue a fire-eligible round that DecideFire cannot hold back, because it never sees findings — so a pump could spend the primary's quota on code the caller is still expected to fix. A finding carried from an older commit is refused. IDs hash the text, not the commit, so recording one against the current head would silently filter the identical finding when the current reviewer reports it. And "no thread ID" turns out not to be the question. When the GraphQL thread query fails, Feedback falls back to REST, which does not return thread IDs at all, so an inline comment with an open thread arrives looking threadless. Only sources that intrinsically cannot have one — review bodies, prompt blocks, skip notices, issue comments — are dismissible now. The CAS write moves to service.go as recordDismissal, which is where AGENTS.md says every write belongs, and it honours DryRun.
Everything crq does with git assumes it was RUN inside the checkout it cares about: the local-work probe shells out with no directory, target inference reads the current branch. That is right for a command an agent types from its working copy, and useless for the daemon, which has no checkout of any repository it reviews — the reason dispatch has nothing to stand on. Workspace separates the two: a bare mirror per repository, fetched rather than re-cloned, and a throwaway detached worktree per head. Detached on purpose — a place to inspect and build, not a branch to commit to by accident. A worktree left behind by a killed process is replaced rather than reused, since the head it holds is stale anyway. Credentials stay out of it. The remote is the ordinary https URL, so the host's existing git credential helper supplies the token and crq never holds a secret it would then have to avoid logging. One runner now executes every git command, taking the directory to run in, and it folds stderr into the error — "exit status 128" alone has never told anybody what went wrong. localWork takes that directory too, through Config.WorkDir, so a caller working in a worktree it made can say so without the Service being copied. The tests build a real repository and clone it, because the thing under test is whether the git invocations are right.
The loop had a hole at the end: crq could say a PR needed fixing, and nothing turned that into work. A supervisor script polled, wrote the answer to a file, and logged the change — and then depended on somebody reading the log. Tonight that somebody did not, and eight findings sat for an hour. crq watch drives every open PR through the same next oracle an agent uses and emits one JSON line per PR per pass. With --dispatch, a PR whose action is fix gets a session started for it, in a worktree crq checked out at the head the findings are about, with the findings on disk and the target in the environment. This keeps the queue's non-goal intact. crq still does not decide which findings are real: it starts the session and says which PR to look at, and the session judges. That is why dispatch is a separate command over the same oracle rather than something the pump does, and why it is off unless asked for — watching is an observation, dispatching writes code. Two bounds make it safe to leave running. Every dispatch is claimed under compare-and-swap with a heartbeat, so two watchers cannot both work one PR and a session that dies frees its round after the TTL rather than holding it forever. And attempts are counted per head, so a fix that keeps not working stops instead of spending a review round each time; a new head clears the count, because reaching one means the attempt achieved something. The command runs directly, not through a shell, so nothing expands that the operator did not write.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesThe PR adds Watch and dispatch pipeline
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WatchCLI
participant Service.Watch
participant GitHubAPI
participant StateStore
participant Worktree
participant FixCommand
WatchCLI->>Service.Watch: WatchOptions
Service.Watch->>GitHubAPI: list open PRs
Service.Watch->>StateStore: claim dispatch round
Service.Watch->>Worktree: checkout PR worktree
Service.Watch->>FixCommand: start fix session
Service.Watch->>StateStore: heartbeat dispatch claim
FixCommand-->>Service.Watch: session exits
Service.Watch->>StateStore: release dispatch claim
Service.Watch-->>WatchCLI: WatchEvent JSON line
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Action performedReview finished.
|
Six review findings on the workspace, two of them P1. The mirror tree was created with the umask's 0755, so on a shared host git wrote a private repository's objects and source world-readable under ~/.cache. The root is 0700 now, enforced rather than assumed. Clones were unauthenticated in the documented token-only setup: git does not read GITHUB_TOKEN or GH_TOKEN by itself, so a daemon with no credential helper would have failed at its first private checkout — dispatch's first real act. A credential helper is injected when crq has a token, and the token travels in the environment, never in argv, so a process listing, a log line and this package's own error strings carry the helper snippet and not the secret. Three ways a checkout could destroy another. Two workers first cloning one repository both passed the missing-HEAD check and cloned into the same directory; the clone now lands in a staging directory and is moved into place, and losing that race is fine because the winner's mirror is just as good. "a-b/c" and "a/b-c" joined with a dash are the same path, so one repository's cleanup deleted the other's live worktree; owner and name stay separate components. And a deferred Remove on a stale handle deleted the checkout that replaced it — each checkout now owns a generation directory and removes only its own. CRQ_WORKSPACE is read through Config, so a value in ~/.config/crq/env is actually used instead of being silently ignored by the daemon.
Owner
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a21d30799
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Three more findings, one P1. The eligibility guard asked whether a round object existed, when what matters is whether a FIRE-ELIGIBLE one would. A queued round is exactly as dangerous as a newly created one: Pump can hand it to DecideFire, which sees no findings and cannot enforce drain-first, so dismissing one of several open findings could still buy a review of code the caller is still fixing. Repeating a dismissal failed on its own earlier success: Feedback filters the dismissed ID out, so validating against the current findings alone rejected it as unknown. The command is documented as idempotent and an interrupted agent repeating itself is the ordinary case, so an ID this round already dismissed is accepted and reported as such. And the filter now checks the SOURCE, not just the ID. Finding IDs hash the text, not where it came from, so a dismissed body finding later delivered as an inline comment through the REST fallback hashes the same — and an ID-only filter would hide a review thread that is open.
Four findings, one P1. Reading only GITHUB_TOKEN and GH_TOKEN meant the documented `gh auth login` setup produced unauthenticated clones: API calls worked, and every private checkout failed at dispatch's first real act. git now gets the same token the API client resolves, `gh auth token` included. A relative workspace root put the worktree somewhere other than where the returned path said. `git worktree add` runs inside the mirror, so the relative directory landed under the mirror while Checkout.Dir pointed at a path that did not exist. Roots are absolute now. Clearing a PR's directory before making a new generation force-removed a checkout another worker might be building in. Old generations are pruned by age instead, which collects what a killed process left without touching a live session. And two workers fetching one mirror race on git's ref locks, so the loser reported "cannot lock ref" although the winner had just made the mirror current. That retries briefly and then accepts the mirror as it stands, rather than failing a dispatch over a fetch somebody else finished.
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Three findings, one P1 in the fix itself. After a push the stored round is still on the PREVIOUS head, because `crq next` returns on a current-head finding before it enqueues. Treating that as a concurrent move rejected every dismissal and left the new head in the exact drain-first deadlock this command exists to end. The two cases are told apart by when the round was enqueued: after the findings were read means somebody moved the PR forward and the decision is stale; otherwise it is the ordinary post-push round, and it is superseded. A finding another agent dismissed concurrently now counts as handled when deciding whether other work is still open, instead of making this call refuse over a finding that is already accounted for. And the docs no longer send an agent to a command that will refuse it: a `review_comment` finding lost its thread ID to the REST fallback, so it is neither resolvable nor dismissible until crq can read review threads again.
Ten findings, six P1. `crq watch --dispatch -- <cmd>` never worked: FlagSet.Parse consumes the terminator, so looking for it afterwards found nothing and the fix command was read as a list of repositories. argv is split before parsing. Five ways dispatch could do harm. It ignored DryRun, so the mode that promises crq writes nothing claimed shared state and ran a code-writing command. It wrote the findings at the worktree root, where a session following the documented `git add -A` push would commit crq's review payload into the PR — they go outside the checkout now. It deleted the worktree after a successful session, discarding fixes that were made but not pushed; a worktree with uncommitted work is kept. It ignored the fleet skip marker before calling Next, which enqueues and can fire — the marker exists to protect the shared quota, so it is honoured first. And a lost heartbeat left the session running while another watcher started a second one on the same worktree; losing the claim now stops the session. Attempts were counted at the claim, so a failed clone ate the per-head budget and permanently skipped the PR after three tries. The attempt is given back when nothing ran. Two smaller: a throttle now sleeps for the reset the API named instead of the ordinary interval, which was hammering an exhausted quota, and a one-shot pass reports the PRs it could not read instead of exiting 0 — cron reporting a clean scan of a broken one is worse than a failure. An emit failure stops the watcher, because a closed pipe means nothing is observing something that fires reviews and starts sessions.
Owner
Author
|
@codex review |
Four findings, one P1 that would have destroyed a fix session's work. The mirror was a --mirror clone, whose refspec is +refs/*:refs/*, so the next `fetch --prune` reached into refs/heads and deleted any branch a session had created in its worktree — the documented way for a session to make changes. It is a bare clone now, fetching into refs/remotes/origin/*, which leaves refs/heads to the sessions. Pruning read the checkout directory's own timestamp, which editing files inside does not update: a session busy for twelve hours read as abandoned and had its worktree force-removed. It measures the newest file under the checkout instead. Persistent ref-lock contention on a shared mirror returned an error even though the mirror was current, failing a dispatch over another worker's success — the exact case concurrent dispatch has to survive. And the worktree add now goes through the credential-carrying runner, so a checkout filter that fetches from a private repository is authenticated like every other git call.
Three findings, one P1 in the guard I added last round. FireEligible answers "right now", which is the wrong question when deciding whether a partial dismissal would leave a round able to buy a review. A round cooling in awaiting_retry is not eligible this second and becomes eligible the moment RetryAt passes, so letting the dismissal through on that technicality just deferred the hazard to the cooldown expiring. The guard asks whether the round will ever fire again. State alone could not tell "no round for this head" from "the head moved and nothing has enqueued it yet", so a push landing between Feedback and the write could record the dismissal against the wrong commit. The head is re-read once before recording. And the README now carries the same caveat as the skill: a review_comment finding lost its thread ID to the REST fallback and still has an open thread, so it is neither resolvable nor dismissible until crq can read threads again.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d054450030
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Both branches carry this file so the watcher and fleet auto-review cannot drift apart in how they read the marker, and the two copies had drifted anyway: the hold branch grew a line-based scanner that understands block quotes, list containers, lazy paragraph continuation and delimiter-run widths, while this one kept the first sketch — which the review caught reading a mid-line ``` run as a fence opener. So take that one, byte for byte, tests included. It passes the fence-position cases this branch's review asked for, and identical files merge without a conflict.
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81cb088c14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Watching a pull request nobody fixes is a queue that reports work and does none. That is what happened: the drain was installed for one repository, every other repository in the fleet kept collecting review feedback, and one PR reached 36 unresolved threads with nothing to drain them. Nothing was broken — nothing was watching for fixes there. So dispatch is the default, and three things turn it off, each at its own scale: --no-dispatch for one run, `crq drain off <repo>` for one repository, and having no fix command configured at all. The last one observes with a log line instead of refusing to start: making the default setting break the plain command is not a default. `crq drain off` stops FIXING, not watching. The pull request is still observed and still reviewed, so feedback keeps arriving for a person to act on — the switch is about who writes the code, not about whether crq looks. It lives in the state ref alongside the reviewer overrides, for the reason recorded there: the daemon has no checkout of what it watches, and a daemon and an agent reading different configurations while writing one ref is a class of divergence worth not having. Also fixes what widening the fleet exposed within a minute: watchPass appended to the caller's repository slice. `crq watch -- <cmd>` splits argv at "--", so the flag half keeps capacity reaching into the command half and fs.Args() is a sub-slice of it — filling an empty repository list wrote the repository names over the fix command, and every dispatch in the fleet died with "fork/exec kristofferr/coderabbit-queue: no such file or directory". Fixed on both sides: the pass copies rather than appends, and the CLI caps the capacity it hands out. The drain-health alert caught it in three attempts, and no attempt was burned — a command that never started gives its attempt back.
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d21ee5523d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The gate added last commit deadlocked a session within the hour. Its own report:
Not pushed: `crq next --wait` returned `action: fix` twice for a duplicate
threadless finding, never `push`.
It had fixed everything, resolved both threads, run the repository's checks, and
committed — then waited for an authorization that could never come, because one
finding had no thread and nothing in reach could clear it. crq kept the worktree
("committed work that is on no remote branch"), gave the attempt back, and the
next pass started a fresh session to redo the same work.
The command for exactly this is `crq dismiss`, and it was not in the build the
drain installs. So this merges the dismiss branch in — the same way the workspace
branch was merged — and the prompt now names all three ways to account for a
finding, chosen by whether it HAS a thread. It also says what a repeating `fix`
means, because that is the shape the deadlock takes from inside a session.
A gate is only as good as the way out of it. This one shipped without it for an
hour, and one PR's worth of work was redone as a result.
The review of the gate found three more ways a dispatched session can never reach `push`, all the same shape as the one that stranded a commit an hour ago. Each verified against the code first. The gate itself was too narrow. Its purpose is to keep a head from moving under a reviewer who is reading THAT commit, and that is `hold`/`wait` — not every action other than `push`. `fix` is where a session that has just made changes usually lands, and refusing to push there is what turned each of the following into a deadlock rather than a delay. The prompt now pushes unless crq says a reviewer is mid-read. reviewRequested counted a dispatch hold as a requested review. ClaimDispatch mirrors a queued round into awaiting_retry so binaries that do not understand the claim still refuse to fire; reading that back as "a review was requested" told the holding session to wait for a reviewer that could not be asked — the claim is what makes the round ineligible — while its own heartbeat extended the window it was waiting on. DispatchHoldPhase records what the round was before the hold, which is the phase the question is about. claimDispatch deduped the head on ANY finding. A Codex or Bugbot finding says a co-reviewer looked, and they spend no account quota: marking the round completed on that evidence retires a primary review nobody ever requested, and Pump can then never fire it. Dedupe now needs the metered primary's own review; a co-reviewer finding leaves the round adoptable, so the queue can still buy the review that is actually missing. And `crq drain <typo>` listed instead of failing — an answer to a question nobody asked, in place of the instruction that was meant.
Resolving first and pushing second means a session that dies in between leaves GitHub threads marked addressed for a commit that is on no branch. The next pass filters those threads out, finds no findings, dispatches nobody — and the fix is stranded for good while the pull request records it as done. That is not a hypothetical: a session did exactly this an hour ago, reporting "resolved both review threads" and "Not pushed". Pushing first is recoverable in both directions. Die before the push and nothing was claimed to be fixed, so the next session redoes it. Die after it and the fix is on the branch, where the next review judges it on its merits. Nothing needed to move for this: with the gate now reading `hold`/`wait` rather than demanding `push`, a session that has just made changes is free to push at `fix`, which is where it lands. The steps only had to be swapped.
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44960f7af7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15317048e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd67d23272
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38c8e4668b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Owner
Author
|
@codex review |
autoReviewPass has always honoured the denylist; watchPass never looked at it. So the one setting that reads like "crq does not go here" covered half of what crq does — reviews stopped for an excluded repository and the watcher carried on observing it and starting fix sessions in it. Half a setting is worse than none: it is the kind an operator sets, verifies by watching reviews stop, and reasonably believes. Excluding a repository now excludes it from the pass too, and the documentation says so rather than naming only autoreview.
drainEnv wrote CRQ_REPOS and not CRQ_EXCLUDE, so an install could produce a service that watches a repository the operator had excluded — the same shape as the quota timings this file already learned to carry: the service does not inherit the shell that installed it, so anything that changes what it does has to be written down.
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This branch stacks on the repo-local workspace one, and had drifted eight commits behind it — enough that GitHub reported the pull request as conflicting with its own base. The whole of internal/workspace comes from there now, verbatim: it is that branch's file, and carrying a diverged copy of it here is how two versions of the same hardening end up reviewed twice and merged once. What this branch adds on top is only what it actually needs — LogDir, because a dispatched session's logs live under the workspace root and the layout of that root belongs to the package that owns it. Following the move: watch.go now names workspace.Checkout and workspace.TokenEnv rather than the local copies that no longer exist, and the drain switch validates a repository slug itself. That last one keeps the base branch's segment rules — "." and ".." are not repository names — because a state key recorded under a name nothing can match is a setting that silently does nothing.
# Conflicts: # AGENTS.md # README.md # cmd/crq/main.go # cmd/crq/main_test.go # internal/crq/config.go # internal/crq/config_test.go # internal/crq/next.go # internal/crq/next_test.go # internal/crq/service.go # internal/crq/service_test.go # internal/crq/skipmarker.go # internal/crq/skipmarker_test.go # internal/crq/state.go # internal/engine/next.go # internal/state/state.go # internal/workspace/workspace.go # llms.txt # skills/coderabbit-queue/SKILL.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Roadmap item J, the last piece of the v3 loop. Stacked on #53 (the workspace it checks out into).
The hole
crq could say a PR needed fixing, and nothing turned that into work. The supervisor I was running
polled, wrote the answer to a file, and logged the transition — then depended on somebody reading the
log. Tonight nobody did, and eight findings sat for an hour on an open PR while I reported the branch
as clean. The monitor was right; the design ended at "log it".
crq watch
A PR whose action is
fixgets a session started for it, in a worktree crq checked out at the headthe findings are about:
CRQ_DISPATCH_REPOowner/nameCRQ_DISPATCH_PRCRQ_DISPATCH_HEADCRQ_DISPATCH_FINDINGSAlready working — the first live
--oncepass found 9 findings on #52 that I had missed by reading astale file:
What this does not change
Issue #9's non-goal stands: crq does not write code or decide which findings are real. It starts
the session and says which PR to look at; the session judges. That is why this is a separate command
over the same
nextoracle rather than something the pump does, and why dispatch is off unless askedfor — watching is an observation, dispatching writes code.
Two bounds that make it safe to leave running
Claimed under CAS, with a heartbeat. Two watchers cannot both spawn a session for one PR. A
session that dies frees its round after
DispatchTTLinstead of holding it forever, and the heartbeatis what lets a long session outlive that TTL without being stolen. A token distinguishes two claims
from the same host, so a restarted watcher cannot release the claim of the process it replaced.
Bounded per head.
CRQ_DISPATCH_MAX_ATTEMPTS(default 3) stops a fix that keeps not working fromspending a review round every pass. The count survives release and is only cleared by a new head —
because reaching a new head means the attempt achieved something.
The command runs directly, not through a shell, so nothing expands that the operator did not write.
Verification
TestDispatchClaimwalks the whole claim lifecycle: a second watcher refused, heartbeat by the owneronly, a heartbeated claim not stolen, a stale one taken over keeping the attempt count, release
freeing the round, the bound stopping a fourth attempt, and a new head starting fresh.
TestWatchDispatchesAFixSessionWithItsContextruns a real session script against a real clonedrepository and asserts what it was handed: repo, PR, head, a findings file, and a working directory
inside crq's worktree rather than wherever crq was started. It also asserts the claim is released and
the worktree cleaned up afterwards.
gofmt -l .clean,go vet ./...clean,go test ./... -count=1green.Ref #42
Summary by CodeRabbit
New Features
crq watchcommand to monitor open pull requests at configurable intervals or run a single scan.Tests