GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — feat(coderd): run chat automation schedules by ThomasK33 · Pull Request #30160 · coder/coder Conversation
| ||||||||||||||||||||||||||||||||||||||||
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…chat' into tk/codagt-1211-automations-schedules
There was an error while loading. Please reload this page.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
A chat created through the chat API or by a webhook event records an audit entry. A chat that a new_chat schedule creates now records one too, as created by the automation owner, with the automation and input ids in the additional fields. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: 0787d0896b
Your 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".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
…refusals The scan read one page of 500 due schedules per tick. Rows that stay due, such as those of owners with the experiment off, stayed first in every page and could hide all other schedules, and a large burst could wait past the grace window. The scan now pages by (cursor, id) until a page comes back short. A Publish refusal because the experiment is off for the owner no longer skips the occurrence: the evaluator also reports a failed read as off, so the cursor stays for a retry within the grace window, and the next scan drops an owner whose experiment really is off. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: d35563001a
Your 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".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
A scan published due occurrences one at a time, so an occurrence that waited for a chat lock or a slow hook held back every later schedule, which could then expire past the grace window. The scan now publishes up to eight occurrences at once and returns when all are done. A chat that a new_chat schedule creates is now titled with its occurrence's scheduled time, which does not depend on how long hooks and locks take before acceptance. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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". |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
GitHub created no pull_request runs for the last two pushes while the PR mergeability stayed unknown. This commit changes no files. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
|
CI note: GitHub has not created Local gates on every pushed head: Generated with |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
CI evidence for the exact head d483cd5: Why workflow_dispatch: GitHub creates no pull_request runs for this pushed stacked PR, and its mergeable state stays UNKNOWN. Attempt 1 failed only in Generated with |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…agt-1211-automations-schedules No conflicts. make gen produced no diff. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
|
sync with main d9c13dc: no content conflicts; make gen produced no diff
Generated with |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…agt-1211-automations-schedules No conflicts. make gen produced no diff. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
|
sync with main 35abc22: no content conflicts; make gen produced no diff
Generated with |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Xum (an AI coding agent) prepared and posted this review on behalf of Mike. Each finding came from automated review passes and was re-verified against the code; please push back on anything that looks wrong.
The schedule scanner matches the CODAGT-1211 scan requirements for this layer: an unlocked keyset-paged due read, per-owner experiment evaluation, a guarded forward-only cursor write, the skip versus retry split, and fake-clock tests for every required check. go test ./coderd/x/chatd -run TestAutomationSchedule passes locally at this head (all 19 TestAutomationScheduleScan subtests, nested ones included, plus TestAutomationScheduleLoopScansAtStart). Only small nits remain.
Verdict: no blocking issues
Severity: P0 0, P1 0, P2 0, P3 2 (1 inline, 1 below)
Nits:
coderd/x/chatd/automation_schedules.go:135-137: scanAutomationSchedules only forwards automationScheduleBatchSize to scanAutomationSchedulePages so tests can pass a smaller page. The worker already handles this with an option and a default (ArchiveBatchSize, and this PR's AutomationScheduleInterval). An AutomationScheduleBatchSize option would remove the wrapper and follow that pattern.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| // clock that runs ahead can skip the occurrences within d of true time. | ||
|
|
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
P3: Blank line detaches the doc comment from checkAutomationOccurrence
The blank line at 73 separates the 15-line comment from the function, so go/doc and editors do not treat it as the function's doc comment. Hovering checkAutomationOccurrence then hides the clock-skew argument that the PR body points reviewers to. This is still the case at the top of the stack (#30220).
Fix: Delete the blank line.
| // clock that runs ahead can skip the occurrences within d of true time. | |
| // clock that runs ahead can skip the occurrences within d of true time. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 67db54f: the blank line is gone, so the comment documents checkAutomationOccurrence again. The same commit replaces the batch-size wrapper with an AutomationScheduleBatchSize worker option (default 500), matching AutomationScheduleInterval and ArchiveBatchSize.
Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…agt-1211-automations-schedules Resolved two conflicts. ARCHITECTURE.md: kept main's new auto-archive paragraphs under the Auto-archive loop heading and this layer's Automation schedule loop section after them. options.go: kept this layer's AutomationScheduleInterval and main's HeartbeatRenewalTimeout and HeartbeatLockTimeout. make gen produced no diff. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
…option Replace the scanAutomationSchedules batch-size wrapper with a worker option, AutomationScheduleBatchSize, defaulting to 500, and attach the checkAutomationOccurrence doc comment to its function. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
|
sync with main d11809f: merged #30134 (resolved ARCHITECTURE.md and options.go next to main's heartbeat options); attached the checkAutomationOccurrence doc comment and made the schedule batch size a worker option (Mike's review)
Generated with |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Xum re-reviewed this PR on behalf of @ibetitsmike using GPT-6.1 Sol (high) Exec subagents.
Both prior Mike nits are fixed; the doc thread is still marked open, and duplicate hooks remain acknowledged and deferred.
New findings: Standards 1 nonblocking P3, Spec 0, Frontend 0 (no own-layer site changes).
Verified fresh delta logs pass schedule/startup, cursor authorization, shared webhook, selected heartbeat/archive/chatstate, and three-repeat concurrency/paging tests.
No extra suites or race run were performed; custom worker/tick coverage remains source-only, and fresh required CI completion is not captured (Pixel pending).
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| @@ -0,0 +1,699 @@ | |||
| package chatd //nolint:testpackage // Runs the unexported schedule scan directly. | |||
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
[P3] Remove the prohibited internal-test suppression
.claude/docs/TESTING.md:58-65 explicitly prohibits //nolint:testpackage; the existing _internal_test.go suffix already exempts this file. This adds a redundant per-file lint exception contrary to the documented convention, not a runtime defect. Keep package chatd and remove the directive and its justification.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…agt-1211-automations-schedules No conflicts. make gen produced no diff. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
…ule scan test The _internal_test.go suffix already exempts the file from testpackage, and .claude/docs/TESTING.md prohibits the per-file directive. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
|
sync with main a328bee: merged #30134; removed the testpackage suppression from the schedule scan test (Mike's review)
Generated with |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Xum validated this PR and approved it on behalf of @ibetitsmike.
The reported test-package suppression is removed. The current layer preserves the earlier fixes and explicitly accepted follow-ups.
Validation: the previously completed, uncached schedule scan/startup and schedule authorization tests passed on this exact unchanged head. I rechecked the live layer diff and scope. The author-triggered CI rerun now passes, including test-go-race-pg and required.
This approval covers this layer at the pinned commit, not unresolved findings elsewhere in the stack. No new remote UAT or full local suite was run during this follow-up.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…agt-1211-automations-schedules No conflicts. make gen produced no diff. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
|
merged #30134
Generated with |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…agt-1211-automations-schedules No conflicts. make gen produced no diff. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
|
merged #30134
Generated with |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…agt-1211-automations-schedules No conflicts. make gen produced no diff. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
Generated with |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…tions-schedules Brings main ff158c0 into this layer. No conflicts. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
…tions-schedules Brings the new_chat automation title source fix from #30134. Resolved automations_publish.go: this layer's automationNewChatTitle call keeps the new TitleSource line. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Xum validated this PR and approved it on behalf of @ibetitsmike.
Revalidated head 549f50621744 against base 7f5d2aa18e6b. Independent Standards and Spec renewal found no new blockers; the original accepted exclusions remain unchanged.
Re-reviewed scheduler contracts and the inherited title-source change. Schedule, scheduler-grant and authorization checks passed; the concurrency regression passed under race with count=3.
Approval is scoped to this layer and the reviewed inherited interactions. No full-repository suite, fresh Pixel screenshots or remote UAT claimed.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.