GeezerVison

Copy link

Copy Markdown

Collaborator

ibetitsmike added this pull request to stack #30896 October 16, 2026 12:42

ibetitsmike marked this pull request as ready for review October 16, 2026 14:00

ibetitsmike requested a review from a team as a code owner October 16, 2026 14:00

chatgpt-codex-connector Bot reviewed Oct 16, 2026

View reviewed changes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

pr-queue-manager Bot requested a review from jaaydenh October 16, 2026 14:24

chatgpt-codex-connector Bot reviewed Oct 16, 2026

View reviewed changes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jaaydenh reviewed Oct 18, 2026

View reviewed changes

ibetitsmike requested a review from jaaydenh October 18, 2026 15:19

jaaydenh approved these changes Oct 19, 2026

View reviewed changes

chatd can now move a root chat, together with its subagent chats, into a
project, to another project, or out of its project. Nothing calls this
yet. #30894 adds the API.

**Stack** (moving chats between projects, bottom first):
1. **This PR**: the move in chatd
2. #30894: `PATCH /api/v2/chats/{chat}` takes `project_id`
3. #30895: **Move to project** in the chat menu
4. #30795: drag a chat onto a project folder in the sidebar

**Why:** today a chat keeps the project it was created with. A move must
follow the family rules of archiving, because a step that runs during a
move would still have the memory tools of the old project.

- `chatstate.Tx.SetProject` runs only from W, E0 and E1, like
`SetArchived`. `SetFamilyProject()` locks the root row before its
subagents, in the same order as archiving, so a busy family gets
`ErrTransitionNotAllowed` and nothing changes.
- A move to the project the chat is already in changes nothing, even
while the chat runs. The check reads the locked root row, so it waits
for a move that is still in flight.
- Only the root row stores `project_id`, and subagents use the project
of the root. After the commit, chatd publishes a new `project_change`
watch event for each chat of the family. The move keeps `updated_at`, so
lists keep their order.
- A root chat outside a project now has an empty memory index. When a
chat leaves its project, or its project is deleted, its next turn start
commits one memory index update that removes the memories the model saw
there.

**What to scrutinize:** the lock order and the same-project check under
the lock in `SetFamilyProject()` (`chatstate/family.go`), the turn-start
condition in `memoryIndexMessage()` (`memory.go`), and whether the
`ARCHITECTURE.md` text matches them.

**Generated** (`make gen`): `queries.sql.go`, `querier.go`, `dbmock` and
`dbmetrics` come from `queries/chats.sql`. `coderd/apidoc`,
`docs/reference/api/schemas.md` and `typesGenerated.ts` come from the
new watch event kind, which is also why two `chats.test.ts` fixtures
gain `project_change: false`.

**Architecture doc:** `coderd/x/chatd/ARCHITECTURE.md` lists
`SetProject` among the transitions and in the execution state diagram
(W, E0 and E1 back to themselves). It also says that a root chat's
project can change after creation, and that a moved chat's next turn
starts with a memory index update from the old project to the new one.

## Testing

- `make pre-commit` passes. `go test` passes for
`coderd/x/chatd/chatstate`, `coderd/database/dbauthz`
(`TestMethodTestSuite`) and the chatd memory tests.
- `TestSetFamilyProjectLocksRootFirst` fails with 55P03 when a subagent
row is locked first. `TestSetFamilyProjectKeepsTheCurrentProject` covers
the same-project move during a run. The transition matrix test gains
`SetProject` cases for W, E0 and E1.
- `TestMovedChatUsesNewProject` runs two real turns around a move: the
second turn uses the instructions and memories of the new project, and
`OutOfProject` fails without the memory index removal.

> Xum opened this PR on behalf of @ibetitsmike.

Base automatically changed from mike/move-chats-api to main October 19, 2026 14:01

ibetitsmike requested a review from nickvigilante as a code owner October 19, 2026 14:01

`PATCH /api/v2/chats/{chat}` now takes `project_id`. A project ID moves
the chat into that project, and `uuid.Nil` removes it from its project.

**Stack** (moving chats between projects, bottom first):
1. #30893: the move in chatd
2. **This PR**: the chat update API takes `project_id`
3. #30895: **Move to project** in the chat menu
4. #30795: drag a chat onto a project folder in the sidebar

**Why:** #30893 can move a chat family, and the UI layers above need an
endpoint to call.

- A move accepts the same projects as chat creation, through
`validateChatProjectSelection()`, which this PR extracts from
`postChats()` without a change in behavior.
- A project that the chat's owner cannot use gives 404, even for an
admin. The owner must own the project or hold a grant on it
(`chatd.ChatProjectUsableBy`, the check that chat creation uses since
#30439). So an admin cannot put a member's chat into a project that the
member has no access to and expose its memory.
- A child chat, an archived chat or a disabled `chat-projects`
experiment gives 400. A family that is not idle gives 409 ("Cannot move
an active chat. Interrupt or wait for the chat to finish first."). A
project deleted between the check and the move gives 404.
- The move is the last write in `patchChat()`, so a refused move keeps
the earlier writes of the same request, as a refused archive does.

**What to scrutinize:** the access check in
`validateChatProjectSelection()` and the order of the refusals in
`patchChat()` (`coderd/exp_chats.go`).

**Architecture doc:** the `PATCH /api/v2/chats/{chat}` section of
`coderd/x/chatd/ARCHITECTURE.md` lists the `SetProject` states of a move
and its refusals, and no longer says that only an `archived` change
emits transitions.

**Generated** (`make gen`): `coderd/apidoc`,
`docs/reference/api/chats.md`, `docs/reference/api/schemas.md` and
`typesGenerated.ts` come from `UpdateChatRequest.ProjectID` in
`codersdk/chats.go`.

## Testing

- `make pre-commit` passes, and so does `go test ./coderd -run
TestPatchChatProject` (14 subtests).
- `TestPatchChatProject/AnotherOwnersProject` gets 404 for another
user's project, then moves the chat after the project is shared with the
chat's owner. It fails without the access check, and also with an
owner-only check. `MoveBackWaitsForAPendingMove` ends in the wrong
project when the handler skips the move before the lock.
`ActiveFamilyReturnsConflict` checks the 409 for a running, interrupting
or action-waiting root and for a running subagent.
`PublishesProjectChange` and `AuditsTheMove` cover the watch event and
the audit entry.
- Untested: the 404 when the project is deleted between the check and
the move.

> Xum opened this PR on behalf of @ibetitsmike.

27 checks passed

ibetitsmike deleted the mike/move-chats-menu branch October 19, 2026 14:22