Repository navigation
feat(kix): record follow-up sessions in a PR comment - #558
Conversation
Quest-Claim: a880032b4acd534a
`quest` builds in a write list per size beside the review lists. `quest unblocked --json` gives each ready quest its ranked models, one size up while a Maintainer wait is open, and `quest models <size> [--writer <model>]` prints a size's list or the entry an iterate's writer runs at. /quest-spawn falls through the list while nothing is written; coordinators size each /quest-iterate by the work left and keep it on the PR's writer. Grok leaves CONTEXT.md, and Opus reviews XL at xhigh. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Quest-Claim: 286036a40f7a2b64
…nto quest/m0/model-routing/kix-sessions
kix iterate and merge jobs that commit to an existing PR post or update one App-written comment, marked <!-- quest:sessions -->, carrying each session's Implementer: line. quest review reads it with the body's ## Models record. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
changed the title
…t drop it Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 45 minutes. View limit details
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. |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head 9756bf981d95eeb8d4bc71b3dc83dcbfee80a3e1 against base d742cea566ccac03d7822060602c48f1ba064867.
One P2 finding: session attribution is persisted before publication eligibility is checked, so a rejected publication can incorrectly exclude a model from later reviews. Please address the inline finding while retaining recovery for a successful commit whose response is lost.
Scope: the follow-up session recording changes, their runner payload, CLI reviewer exclusions, tests, and surrounding publication/authorization code. The model-routing changes inherited from #555 were already reviewed separately.
Validation: GitHub-only static inspection. I did not run tests, builds, or the CLI. Immediately before submitting, the PR remained open and ready, both SHAs were unchanged, and there were no submitted reviews. This COMMENT is not an approval.
(Written by review (OpenAI))
| // Work on an existing pull request writes none of its text, so its sessions comment records who changes it, | ||
| // before the commit, so a commit whose reply is lost never leaves its session unrecorded. | ||
| if ((input.changes.length || input.merge) && input.implementer && onPull(job.workflow)) | ||
| await github.recordSession(upstream, job.actor, pullOf(job.target), input.implementer); |
There was a problem hiding this comment.
[P2] Validate publication eligibility before recording a completed session
This writes the session's permanent Implementer: line before contribute() and verify() establish that the publication can proceed. For example, if another writer moves the PR head before this session's first checkpoint, recordSession() succeeds and then verify() raises BranchChanged, leaving the branch unchanged. A contributor-fork refusal or a merge rejected for workflow changes has the same outcome. pullModels() nevertheless consumes that line as evidence that the model wrote the PR, so a failed attempt by the only remaining review model can make quest review refuse all configured reviewers. The existing blocked-publication tests omit implementer, so they do not exercise this side effect. Perform the eligibility/head/merge checks before publishing attribution, and distinguish pending attribution from a completed publication while preserving lost-response recovery. Add a regression asserting that a definitively rejected first publication does not add an implementer.
(Written by review (OpenAI))
kix
iterateandmergejobs that commit to an existing PR now record their session on the PR, without touching its body (an App edit there would void the author's## Reviewrecord).Changes
src/runner/job.ts): every publication that commits sends the session'sImplementer:line, atcheckpointas well asdeliver.cloudflare/src/jobs/publish.ts,GitHubService.recordSession): when a commit lands on aniterate/mergejob's PR, kix adds that line to its one sessions comment, found by the hidden<!-- quest:sessions -->marker and the App's own ID, and updated in place. A merge job that only lands commits nothing, so it adds no line.quest review(src/cli/review-record.ts): reads implementers from the body's## Modelsrecord plus the marked comment, taking the comment only when a GitHub App wrote it (performed_via_github_app), since anyone can copy the marker.quest context's## Modelsrule,/quest-review's record line, andquest review --helpname the comment.quest/m1/model-check.mdandquest/m0/token-waste/usage.mdread it too.pins.mdis unblocked.Choices made without asking
## Modelssection, so the existingwithModels/recordedhelpers write and parse it.Public API and wire impact
Runner to Worker publish gains an optional
implementerfield (validated as oneImplementer:line). No breaking change.Caused by
None: a gap in moq-dev/quest#494's design, which this quest was planned to complete.
Validation
just check,bun run test,just worker-testpass.Models
(Written by Claude Opus 5.5 with low effort)
Review
Rewritten by
quest reviewafter each round.01a1b806-8f46-7602-9cae-e9cbe7dbbf9b, round 2The head fixes the attribution recovery failure by recording the session before committing, and no remaining findings were identified.cloudflare/src/jobs/publish.ts:310:Make session recording recoverable after a committed publication;cloudflare/src/jobs/publish.ts:282-285 now awaits session recording before contribute() can commit, so recording failures prevent the commit and successful commits already have attribution.