GeezerVison

Round 5 of the chat project delete review: the round 4 fixes hold (several checked by breaking the line and rerunning the test), the onSuccess placement dispute is closed, and this round adds 1 P2, 1 P3, 9 P4, and 2 Nit findings.

Most of these are tests and docs that did not follow the round 4 lock changes: the skill lock mode, the memory writer lock, and the docs that describe them.

🤖 This review was automatically generated with Coder Agents.

Round 6 of the chat project delete review: the round 5 fixes hold (checked by mutation), the memory lock dispute is closed, and this round adds 1 P2, 4 P4, and 1 Nit, plus one item reopened below.

Reopened: the ConsolidateMemory deleted-project response still has no test (reply on the memory.go:532 thread). If that thread still shows as resolved, it is because unresolving it failed again; the reply is current.

Two of the new findings are in the lock-wait probes of the new race tests, which count waiters from parallel subtests and can let a subtest pass without testing its own lock.

🤖 This review was automatically generated with Coder Agents.

Round 7 of the chat project delete review: every round 6 finding is verified fixed, most by breaking the fixed line and rerunning its test, and reviewers found no production defects. This round adds 2 P2 comment findings, 2 P4 test gaps, and 1 Nit.

🤖 This review was automatically generated with Coder Agents.

Round 8 of the chat project delete review: every round 7 finding is verified fixed, the two new 404 tests fail when their branches are removed, and reviewers found no production defects. This round adds 1 P2 test comment finding and 3 P4 cleanups.

🤖 This review was automatically generated with Coder Agents.

Round 9 is blocked: the head is still 36ba869, the commit reviewed in round 8, and none of the four round 8 threads has a reply, so no reviewers ran.

Needed before the next round, as a fix in this PR or a reply on the thread explaining why not:

• coderd/chat_projects_test.go:356 (P2, CRF-144): the table comment says "source project" and "the chat is gone too", which are wrong for the unarchive case.
• coderd/x/chatd/chattool/memory.go:53 (P4, CRF-145): the writeProjectMemories doc does not say what the returned int64 is.
• coderd/database/querier_test.go:2376 (P4, CRF-146): lockWaitCount returns 0 when its query fails, so a broken probe times out as a locking regression.
• coderd/exp_chats.go:1736 and :2945 (P4, CRF-147): the ForeignKeyChatsProjectID check is unreachable now that create and move hold the project FOR SHARE.

No production defects are open.

🤖 This review was automatically generated with Coder Agents.

Round 10 of the chat project delete review: all four round 8 findings are verified fixed in 6a77bb8, and the affected Postgres and HTTP tests pass. Reviewers found no production defects. This round adds 1 Nit.

Out of scope (needs a ticket or explicit acceptance by a human):

• coderd/exp_chats.go:2944: "between the check and the move" does not name validateChatProjectSelection, which runs about 200 lines earlier; the comment and its sibling at line 1735 exist at the base commit.

🤖 This review was automatically generated with Coder Agents.

Round 11 of the chat project delete review: CRF-149 is verified fixed in 7145b4a. A wildcard reviewer found two P4 issues in code that has not changed since round 4, both about sub-chats carrying their root's project.

coderd/x/chatd/project_instructions.go:17

P4 [CRF-153] "Subagent chats do not carry a project" is false after this PR. (Knov)

Migration 000630 backfills sub-chats, its trigger sets their project on insert, and SetFamilyProject now moves every member, so a sub-chat's own ProjectID is the source of truth. Rewrite the sentence to say sub-chats carry their root's project. Once CRF-152's trigger is in place, the root lookup at lines 29 to 41 only costs one GetChatByID per turn for sub-chats outside a project and can be removed.

🤖

🤖 This review was automatically generated with Coder Agents.

Round 12 of the chat project delete review: CRF-153 is verified fixed in ce93a26, and the CRF-152 defense is accepted. This round adds 2 P4 cleanups in the code ce93a26 touched.

🤖 This review was automatically generated with Coder Agents.

Round 13 of the chat project delete review: CRF-156 and CRF-157 are verified fixed in 3c6508d, and TestResolveProjectInstructions passes. No new findings in the PR's changes, and no findings are open.

Out of scope (needs a ticket or explicit acceptance by a human):

• coderd/x/chatd/project_instructions.go:20: the doc's second paragraph repeats the failure cases from the usableChatProject doc (projects.go:51); the paragraph is unchanged from the base commit.

🤖 This review was automatically generated with Coder Agents.

Docs LGTM