This is a first-pass (Netero) review only. The full review panel has not yet reviewed this PR. Two P3 findings below (one inline, one on a line outside the diff folded here).

The new AutoArchiveSettings component follows the existing RetentionPeriodSettings structure closely, with good Storybook coverage for toggle, validation, and save/load error states. The stories use play functions with real assertions.

Two related issues around the load-error path: the component renders an editable form seeded with a synthetic 90-day default when the GET fails, and the story that tests this path doesn't actually simulate the missing-data condition.

P3 [DEREM-2] AgentSettingsLifecyclePageView.stories.tsx:394 The AutoArchiveLoadError story sets isAutoArchiveDaysLoadError: true but inherits autoArchiveDaysData: { auto_archive_days: 90 } from baseArgs. In a real load failure, autoArchiveDaysData would be undefined. The story verifies the error text renders but doesn't catch that the form is still editable with a synthetic default. Adding autoArchiveDaysData: undefined to the story args would exercise the actual error path. (Netero)

"Toggling off immediately sends { auto_archive_days: 0 }, and editing then saving posts a new value, clobbering whatever deployment-wide setting is actually stored." (Netero)

🤖 This review was automatically generated with Coder Agents.

DEREM-1 is addressed: the fallback changed from hardcoded 90 to DefaultChatAutoArchiveDays (0), so the form no longer presents an editable default on load failure.

DEREM-2 is unaddressed: the AutoArchiveLoadError story still inherits autoArchiveDaysData from baseArgs instead of setting it to undefined. With the DEREM-1 fix the practical risk is lower (the fallback is now 0, matching the story's inherited value), but the story still doesn't exercise the actual undefined data path that a real GET failure produces.

Further panel review is blocked until DEREM-2 is addressed (add autoArchiveDaysData: undefined to the story args) or the author responds with a rationale for keeping it as-is.

🤖 This review was automatically generated with Coder Agents.

Both R1 findings verified fixed. Nice separation of the UX default (ENABLE_DEFAULT_DAYS = 90) from the server default (DefaultChatAutoArchiveDays = 0) in the DEREM-1 fix. 15 reviewers, 3 P3, 2 P4, 5 Nit. Seven reviewers found no issues.

The main theme is the validation-error UX path: the Formik error text is gated on form.touched but onBlur is never wired, so the admin sees a disabled Save button with zero explanation. Combined with missing aria-invalid, there is no visual feedback on invalid input. This is the same pattern as RetentionPeriodSettings, but WorkspaceAutostopSettings demonstrates the correct approach.

Three reviewers independently flagged the structural duplication with RetentionPeriodSettings (Robin, Zoro, Meruem). DEREM-1 itself proved the drift risk is real, but extraction is an architectural change outside this PR's scope. Rated P4.

Nit [DEREM-10] PR body restates the title ("Adds the required UI knobs") with no screenshot or context. The PR style guide asks for visual evidence on UI changes and at minimum a brief statement of what changed. (Leorio)

"Boring code. Well done. ♠" (Hisoka)

coderd/exp_chats.go:4363

Nit [DEREM-12] This comment reads "Update the frontend constant if a settings UI is added." The UI was just added (AutoArchiveSettings.tsx:33-39). The comment should reference the frontend file, mirroring the sibling comment on retentionDaysMaximum. (Meruem)

🤖

🤖 This review was automatically generated with Coder Agents.

All 12 prior findings resolved (9 fixed, 3 acknowledged). PR description updated with screenshot and context. The DEREM-3/6 fixes (onBlur + aria-invalid) are correct in code; all fix commits verified by the panel. Two new findings this round, one P3 and one P4.

The DEREM-3 fix wired onBlur={form.handleBlur} so validation error text becomes reachable after blur. But no story triggers blur, so the fix is unverified by tests. Adding userEvent.tab() to the validation stories would lock this in.

The PR now also touches RetentionPeriodSettings (wraps the Input in a div with a "Days" label) but doesn't add onBlur or aria-invalid there. AutoArchiveSettings has both. The parity gap is visible in the diff.

"This is domain reasoning, not find-and-replace." (Mafu-san, R3)

🤖 This review was automatically generated with Coder Agents.

All 14 prior findings resolved. The DEREM-14 fix (blur + error text assertions in auto-archive validation stories) is well done. Two related P3 findings remain on the retention side: the DEREM-13 fix added onBlur to RetentionPeriodSettings, but the retention validation stories don't verify it.

This is the last parity gap: auto-archive has AutoArchiveExceedsMax and AutoArchiveBelowMin with blur+error-text assertions. Retention has RetentionExceedsMax without them, and no RetentionBelowMin at all.

site/src/pages/AgentsPage/AgentSettingsLifecyclePageView.stories.tsx:562

P3 [DEREM-15] RetentionExceedsMax asserts toBeInvalid() and toBeDisabled() but never triggers blur. Compare to its auto-archive sibling AutoArchiveExceedsMax (line 374), which calls userEvent.tab() and asserts the Yup error text. The DEREM-13 fix (ba80f90) added onBlur={form.handleBlur} to the retention Input, but this story doesn't verify it works.

Fix: add await userEvent.tab() + expect(canvas.getByText(/must not exceed 3650 days/i)).toBeInTheDocument() after the existing assertions. (Netero, confirmed by Bisky and Chopper)

🤖

site/src/pages/AgentsPage/AgentSettingsLifecyclePageView.stories.tsx:587

P3 [DEREM-16] No RetentionBelowMin story exists. Auto-archive has AutoArchiveBelowMin (line 405) which types "0", tabs out, and asserts the min-boundary error text. The retention component has the same validation (min(1)) and the same newly-added onBlur handler. The min-boundary error text path for retention is untested.

Sketch: same structure as AutoArchiveBelowMin with retentionDaysData: { retention_days: 30 }, clear input, type "0", tab, assert /at least 1 day/i. (Bisky)

🤖

🤖 This review was automatically generated with Coder Agents.