feat: add workspace autostop reminder template#26429
Conversation
Docs preview📖 View docs preview for |
|
/coder-agents-review |
|
Chat: Review posted | View chat Review historydeep-review v0.7.1 | Round 3 | Last posted: Round 3, 3 findings (1 P2, 1 P3, 1 Nit), APPROVE. Review Finding inventoryFinding inventoryFindings
Round logRound 1Netero-only. 1 P2 (adjusted from Netero P0). Reviewed against 69a63a6..f3093b2. CI Round 2Panel (12 reviewers). CRF-1 addressed. 1 P3, 1 Nit new. 0 dropped. Reviewed against 69a63a6..2b7ff5f. Clean PR that follows the established notification template pattern. Primary concern is docs describing behavior that doesn't exist yet. Round 3Panel (4 reviewers + Netero advisory). CRF-2 and CRF-3 addressed. 0 new findings. All prior findings resolved. Reviewed against 2948714..f9cb091. APPROVE. About deep-reviewCRF = Coder Review Finding (P0-P4, Nit, Note)
|
There was a problem hiding this comment.
First-pass review (Netero). One P2 finding. This is a mechanical check only; the full review panel has not yet reviewed this PR. The panel will review after this finding is addressed.
The template definition follows the established notification pattern correctly: UUID in events.go, fallback icon, golden test, docs update. The DB migration lives in the parent commit, and no production code dispatches this notification yet, which is fine for a template-definition PR.
"The new file has zero \r characters." (Netero, on the SMTP golden file)
🤖 This review was automatically generated with Coder Agents.
f3093b2 to
2b7ff5f
Compare
|
/coder-agents-review |
69a63a6 to
2948714
Compare
2b7ff5f to
8baedb7
Compare
There was a problem hiding this comment.
Panel review (round 2, 12 reviewers). CRF-1 fix verified by all reviewers. 1 P3, 1 Nit new.
This is a well-executed template definition PR. The pattern is correct: UUID constant, fallback icon, golden test, docs entry. The golden test runs through the full notification pipeline and is structurally enforced by enumerateAllTemplates. Test density at 97.6% is strong. The stacked-PR decomposition (migration first, then template wiring, then sender) is clean.
Two process notes: the PR description is empty (just the default AI contribution HTML comment), and the commit scope coderd/notifications doesn't contain all changed files (coderd/inboxnotifications.go and docs/ are outside it). Neither blocks merge but worth fixing for future readers.
"I tried to build a case against this change on four different framings and could not." (Pariston)
🤖 This review was automatically generated with Coder Agents.
8baedb7 to
f9cb091
Compare
|
/coder-agents-review |
There was a problem hiding this comment.
All findings resolved. 3 rounds, 3 findings raised, 3 fixed.
CRF-1 (P2, R1): SMTP golden file line endings. Fixed by regenerating the golden file with CRLF.
CRF-2 (P3, R2): Docs described a notification that nothing sends. Fixed by removing the behavioral description.
CRF-3 (Nit, R2): Doc entry style. Fixed by the same change as CRF-2.
The template definition is clean. UUID matches the migration, labels match the template body, golden test runs the full pipeline, fallback icon is mapped, docs entry is correctly placed. The stacked-PR decomposition (migration, then template wiring, then sender) is reasonable.
"A clean stone, no inclusions." (Bisky)
🤖 This review was automatically generated with Coder Agents.
2948714 to
302d252
Compare
f9cb091 to
1988d15
Compare
302d252 to
33ccfed
Compare
1988d15 to
5885637
Compare
33ccfed to
585d717
Compare
5885637 to
d21121f
Compare
585d717 to
699113e
Compare
d21121f to
1c5a2f2
Compare
1c5a2f2 to
f9b398f
Compare
No description provided.