You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Inbox posting, day one: quiet once commented, top severity only, post from a moved-on checkout, ↑ on a just-posted row, description for the live agent #111
The first day of postAlerts on a real inbox showed six things, all seen on NCBackend3#13866, #14530 and #14605:
The daemon posted an alert on a pull request the reviewer had already commented on — the reviewer's own routine had flagged #13866 a month earlier, and on #14530 the reviewer had posted a review from diffity at the very head the daemon then posted to.
A pull request that keeps moving got a fresh automated review at every head: #14605 has three. Once per head is too often when the same person is on the thread already.
The agent named P2 and P3 findings in ALERT-FINDINGS, so they went out too. Only the top severity should ever be posted automatically.
A review prepared at head A cannot be posted once the author pushes B: create-review answers 409 "out of sync", and so does the comment pull. The findings are anchored to A and GitHub accepts a review on A.
A row the reviewer has just posted from moves to Handled at once but has no ↑ until the next poll flips its status.
1. quietOnceCommented — the reviewer's own comment means they have seen it
Config quietOnceCommented: boolean (default false), in InboxSettings, SETTINGS_KEYS, settingsHost.get, the Settings panel (a checkbox under "Notify me if": "Once I have commented on a pull request, stop alerting me about it"), the README table.
When on: after the agent has prepared the review and the discussion is known, if any issue comment, review or inline review comment on the pull request is authored by the viewer (viewerLogin), the alert is dropped — alert: null, alertFindings: [] on the result, nothing posted, one log line <id>: alert dropped — you have commented on this pull request — and the review is still prepared, so it lands under Ready rather than Alerted and the tab stays quiet. The daemon's own posts are in the reviewer's name, so they count: a pull request is alerted at most once, which is what the reviewer's routine did by editing its one comment in place.
Plumbing: PrepareDeps.prContext returns { path, context: PrContext } | null instead of the path alone (the file is still written); PrepareOpts.viewerLogin: string | null from the tick (deps.forge.viewerLogin() is already read once per tick — pass it through prepareOne and prepareBumped); a pure commentedBy(context, login): boolean in prepare.ts or a small helper module, tested on its own.
2. No automated post where the reviewer has already reviewed that head
Independent of the setting: postAlertFindings skips the post when store.latestHandled(id) has headSha === head — passed in as PrepareOpts.reviewedHead: string | null by prepareOne — logging <id>: not posted — you reviewed this head yourself. The alert still shows.
3. Only the top severity goes out
Config postSeverities: string[] (default ["P1", "must-fix"]), editable in the Settings panel beside the prefix as a comma-separated field, README row. alertComments keeps only named findings whose first comment opens with one of those labels — reuse the label parsing summarizeFindings in summary.ts already does (P1: … or [must-fix]), do not write a second parser. Dropped findings are logged by count. If the agent named findings and none is left after the severity and status filters, nothing is posted (the existing rule); a named-none alert still posts body-only.
4. Posting and pulling from a checkout the pull request has moved past
packages/cli/src/server.ts, both /api/github/create-review and /api/github/pull-comments: when localHead !== details.headSha, ask the forge whether the local head is a commit of the pull request (gh api repos/{owner}/{repo}/pulls/{n}/commits --paginate, new prCommits(owner, repo, n) in packages/github/src/pr.ts, cached per request). If it is not, keep the 409 exactly as it is. If it is:
create-review posts with commit_id = localHead. createReview gains an options argument { commitSha, patch }: the commentable lines come from the diff of that commit against the base branch (gh api -H "Accept: application/vnd.github.diff" repos/{owner}/{repo}/compare/{baseRefName}...{sha}, new getCompareDiff), not from the pull request's current patch. ReviewResult gains commitSha: string so the UI can say "posted against the commit you reviewed; the pull request has moved on since" in its success message (packages/ui/src/lib/review-submission.ts / the submit dialog — one line, no redesign). GitHub marks any of those lines the author has since changed as outdated, which is the correct outcome.
pull-comments pulls, and for each incoming remote thread checks that the file exists in the checkout and has at least endLine lines before creating it; the ones that do not map are counted and reported in the response as unmapped instead of being created at a wrong line. Threads already known re-pull as before.
Tests: route tests with a fake gh for both paths — local head is an older PR commit (posts with that commit_id, uses the compare diff), local head is not a PR commit (409 unchanged), pull with one mappable and one unmappable thread.
5. ↑ on a row the reviewer has just posted from
toRow/buildView: a prepared or stale row whose latest handled mark is newer than preparedAt (the same test isHandled makes) is bumpable — prepareUrl set — and resolveBump accepts it (it takes the store, so it can read latestHandled). The bump then behaves as on a handled row: a fresh preparation at the current head. Tests in inbox-open.test.ts and the view tests.
6. The live agent gets the description and the discussion
composeLivePrompt gains body: string (cut at MAX_PROMPT_BODY, framed exactly as the prepare prompt frames it) and contextPath: string | null. The attendant reads them from <dataDirFor(worktree)>/pr-context.json when the file exists — the description is the body field there — and passes null/'' otherwise. Tests in inbox-attendant.test.ts.
Not in scope
Moving an opened checkout to the pull request's new head with the threads carried over. Separate design.
Why
The first day of
postAlertson a real inbox showed six things, all seen on NCBackend3#13866, #14530 and #14605:ALERT-FINDINGS, so they went out too. Only the top severity should ever be posted automatically.One pull request, six changes.
1.
quietOnceCommented— the reviewer's own comment means they have seen itConfig
quietOnceCommented: boolean(defaultfalse), inInboxSettings,SETTINGS_KEYS,settingsHost.get, the Settings panel (a checkbox under "Notify me if": "Once I have commented on a pull request, stop alerting me about it"), the README table.When on: after the agent has prepared the review and the discussion is known, if any issue comment, review or inline review comment on the pull request is authored by the viewer (
viewerLogin), the alert is dropped —alert: null,alertFindings: []on the result, nothing posted, one log line<id>: alert dropped — you have commented on this pull request— and the review is still prepared, so it lands under Ready rather than Alerted and the tab stays quiet. The daemon's own posts are in the reviewer's name, so they count: a pull request is alerted at most once, which is what the reviewer's routine did by editing its one comment in place.Plumbing:
PrepareDeps.prContextreturns{ path, context: PrContext } | nullinstead of the path alone (the file is still written);PrepareOpts.viewerLogin: string | nullfrom the tick (deps.forge.viewerLogin()is already read once per tick — pass it throughprepareOneandprepareBumped); a purecommentedBy(context, login): booleaninprepare.tsor a small helper module, tested on its own.2. No automated post where the reviewer has already reviewed that head
Independent of the setting:
postAlertFindingsskips the post whenstore.latestHandled(id)hasheadSha === head— passed in asPrepareOpts.reviewedHead: string | nullbyprepareOne— logging<id>: not posted — you reviewed this head yourself. The alert still shows.3. Only the top severity goes out
Config
postSeverities: string[](default["P1", "must-fix"]), editable in the Settings panel beside the prefix as a comma-separated field, README row.alertCommentskeeps only named findings whose first comment opens with one of those labels — reuse the label parsingsummarizeFindingsinsummary.tsalready does (P1:… or[must-fix]), do not write a second parser. Dropped findings are logged by count. If the agent named findings and none is left after the severity and status filters, nothing is posted (the existing rule); a named-none alert still posts body-only.4. Posting and pulling from a checkout the pull request has moved past
packages/cli/src/server.ts, both/api/github/create-reviewand/api/github/pull-comments: whenlocalHead !== details.headSha, ask the forge whether the local head is a commit of the pull request (gh api repos/{owner}/{repo}/pulls/{n}/commits --paginate, newprCommits(owner, repo, n)inpackages/github/src/pr.ts, cached per request). If it is not, keep the 409 exactly as it is. If it is:commit_id = localHead.createReviewgains an options argument{ commitSha, patch }: the commentable lines come from the diff of that commit against the base branch (gh api -H "Accept: application/vnd.github.diff" repos/{owner}/{repo}/compare/{baseRefName}...{sha}, newgetCompareDiff), not from the pull request's current patch.ReviewResultgainscommitSha: stringso the UI can say "posted against the commit you reviewed; the pull request has moved on since" in its success message (packages/ui/src/lib/review-submission.ts/ the submit dialog — one line, no redesign). GitHub marks any of those lines the author has since changed as outdated, which is the correct outcome.endLinelines before creating it; the ones that do not map are counted and reported in the response asunmappedinstead of being created at a wrong line. Threads already known re-pull as before.Tests: route tests with a fake
ghfor both paths — local head is an older PR commit (posts with thatcommit_id, uses the compare diff), local head is not a PR commit (409 unchanged), pull with one mappable and one unmappable thread.5. ↑ on a row the reviewer has just posted from
toRow/buildView: apreparedorstalerow whose latest handled mark is newer thanpreparedAt(the same testisHandledmakes) is bumpable —prepareUrlset — andresolveBumpaccepts it (it takes the store, so it can readlatestHandled). The bump then behaves as on ahandledrow: a fresh preparation at the current head. Tests ininbox-open.test.tsand the view tests.6. The live agent gets the description and the discussion
composeLivePromptgainsbody: string(cut atMAX_PROMPT_BODY, framed exactly as the prepare prompt frames it) andcontextPath: string | null. The attendant reads them from<dataDirFor(worktree)>/pr-context.jsonwhen the file exists — the description is thebodyfield there — and passes null/''otherwise. Tests ininbox-attendant.test.ts.Not in scope