feat: add CLI commands for managing chat context from workspaces#24105
feat: add CLI commands for managing chat context from workspaces#24105ibetitsmike merged 40 commits intomainfrom
Conversation
kylecarbs
left a comment
There was a problem hiding this comment.
Good implementation structure — the auto-detect pattern (0/1/N chats), AgentAuth reuse, and graceful sub-agent context copy are well done.
Deep review found one showstopper and several issues to address: 1 P0, 2 P1, 3 P2 across 6 inline comments.
This review contains findings that may need attention before merge.
🤖 Generated by Coder Agents
mafredri
left a comment
There was a problem hiding this comment.
Round 1 review from a 10-reviewer panel (one timed out). The design is sound: agent-authenticated endpoints with explicit ownership checks and AsSystemRestricted bypass. The sub-agent context inheritance is a nice addition. Prior-round findings (RBAC scope, ownership check, Count field, part filtering, parts cap, serpent env fallback) are all resolved in f75c9a9.
Severity count: 4x P1, 5x P2, 2x P3, plus notes.
What this does well: the resolveAgentChat auto-detection pattern is clean. The dbauthz wrappers follow existing patterns correctly. The clear endpoint's graceful handling of zero active chats (return OK with nil ChatID) is good UX.
Notable convergences: all 9 reviewers independently flagged the string-based error dispatch; 7 flagged the skill-parts asymmetry between add/clear/copy; 4 flagged the ContentVersion 0 issue; 4 flagged the missing SanitizePromptText.
Process note: 884 lines of new code across 5 subsystems (CLI, handlers, DB queries, SDK, chatd) with zero handler/integration tests. The dbauthz mocks verify RBAC wiring but not behavior. This is the single largest risk for regression.
Batching: copyParentContextFiles inserts one message per parent context-file message. InsertChatMessages supports batch params. Accumulating into a single call would reduce round-trips. (Knov P3, Knuckle P2, Kite P3, Zoro Note)
"Oh, this test suite looks lovely! Fifty rows, full coverage, green across the board. It's fake. Every row hits the same code path. You dressed up one test in fifty outfits." ...except here there are zero outfits. -- Bisky
🤖 This review was automatically generated with Coder Agents.
Documentation CheckUpdates Needed
New Documentation Needed
Automated review via Coder Tasks |
0640b30 to
b25e5fd
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0640b30d09
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
b25e5fd to
f990c4d
Compare
|
@codex review |
mafredri
left a comment
There was a problem hiding this comment.
Round 2 review from a 4-reviewer panel. All 18 prior findings from rounds 0 and 1 are resolved. The fixes are solid: proper sentinel errors, active-status checks, SanitizePromptText, MaxBytesReader, ContentVersion V1, partial index on agent_id, transactional child chat creation, and skills covered by both clear and copy.
Severity count: 2x P1, 3x P2, 1x P3.
What was addressed well: the refactored resolveAgentChat now returns a full database.Chat with ownership and active-status validation. The sentinel error system (errChatNotFound, errChatNotActive, errChatDoesNotBelongToAgent, multipleActiveChatsError) is clean. The transactional createChildSubagentChat with post-commit signalWake correctly eliminates the race condition reported by chatgpt-codex-connector.
What remains: the handler constructs and serializes messages via a parallel code path (raw json.Marshal + manual params construction) instead of using the canonical chatprompt.MarshalParts + newChatMessage/appendChatMessage builder that every other insertion site uses. Using the builder would fix three findings (NUL encoding, manual construction, content field handling) in a single change. The copyParentContextMessages code in subagent.go, refactored in this same PR, correctly uses the builder, which makes the handler's omission more visible.
Test coverage: 919 lines of new code across 5 subsystems, still zero handler/integration tests after 3 rounds of review. Every prior finding (ContentVersion=0, string-based dispatch, active-status bypass) was a bug that tests would have caught.
"919 lines of untested production code behind agent auth." -- Kite
🤖 This review was automatically generated with Coder Agents.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b328df938
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
@codex review |
mafredri
left a comment
There was a problem hiding this comment.
Round 3 review from a 3-reviewer panel. All 24 prior findings from rounds 0-2 are verified resolved and correct. The round 2 fixes are solid.
Severity count: 1x P1, 2x P3.
All 7 fixes from 6d17906 confirmed by all 3 reviewers: MarshalParts for NUL encoding, BuildSingleChatMessageInsertParams for canonical builder, ContextFileAgentID on all parts, copyParentContextMessages error propagation, transactional clearAgentChatContext with row lock, correct message ordering (system -> context -> user), and last_injected_context cache updates.
The refactored createChildSubagentChat transaction was audited in detail (Kite) and matches CreateChat's structure: usage limit check, InsertChat params, pubsub/signalWake after commit, nil-slice normalization.
One finding persists: zero test coverage for 977 lines of new code after 3 rounds. Everything else is clean.
🤖 This review was automatically generated with Coder Agents.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d17906cdb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
@codex review |
mafredri
left a comment
There was a problem hiding this comment.
Round 4 review from a 2-reviewer panel. All 30 prior findings verified resolved. The compression-boundary fix (6f4aca5) is confirmed correct by both reviewers.
Severity count: 1x P1, 1x P3.
The code is structurally sound. Every correctness, security, and consistency finding raised across 4 rounds has been addressed. One P3 consistency issue and the persistent test coverage gap remain.
🤖 This review was automatically generated with Coder Agents.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f4aca5ab7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
@codex review |
mafredri
left a comment
There was a problem hiding this comment.
Round 5 review. All 32 prior findings verified resolved. Both delta fixes (copyParentContextMessages compression boundary, CLI dir validation) confirmed correct.
Severity count: 1x P2, 1x P3 (new), plus the persistent P1 test coverage gap.
The code is mature. Every correctness, security, consistency, and edge-case finding raised across 5 rounds has been addressed. Two minor observations remain.
🤖 This review was automatically generated with Coder Agents.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 013c2b689c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
@codex review |
mafredri
left a comment
There was a problem hiding this comment.
Round 6 review from a 2-reviewer panel. All 34 prior findings verified resolved or acknowledged. Both reviewers independently verified every sentinel code path (skill-only, context-only, mixed, empty) and confirmed the sentinel is transparent to callers, properly cleared by SoftDeleteContextFileMessages, and correctly detected by contextFileAgentID in the turn pipeline.
Severity count: 0x P0-P2, 2x P3 (cosmetic).
The cache rebuild (updateAgentChatLastInjectedContextFromMessagesBestEffort) correctly reads all persisted messages and writes the cumulative set. responsePartCount is captured before sentinel prepend. clearAgentChatContext transaction properly handles sentinel+skill messages. New slice allocation in prependAgentChatContextSentinelIfNeeded avoids mutating the caller's slice. All verified.
Two cosmetic P3s noted below. The code is ready for merge pending the persistent test coverage gap (P1, every round).
🤖 This review was automatically generated with Coder Agents.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4180fd1ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7830c22bd7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1dcd6d4940
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1dcd6d4940
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0302d6fc18
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ 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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9363409f13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ 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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ 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". |
Adds
coder exp chat context addandcoder exp chat context clearcommands that run inside a workspace to manage chat context files via the agent token.addreads instruction and skill files from a directory (defaulting to cwd) and inserts them as context-file messages into an active chat. Multiple calls are additive —instructionFromContextFilesalready accumulates all context-file parts across messages.clearsoft-deletes all context-file messages, causingcontextFileAgentID()to return!foundon the next turn, which triggersneedsInstructionPersist=trueand re-fetches defaults from the agent.Both commands auto-detect the target chat via
CODER_CHAT_ID(already set byagentprocon chat-spawned processes), or fall back to single-active-chat resolution for the agent. The--chatflag overrides both.Also adds sub-agent context inheritance:
createChildSubagentChatnow copies parent context-file messages to child chats at spawn time, so delegated sub-agents share the same instruction context without independently re-fetching from the workspace agent.Implementation details
New files:
cli/exp_chat.go— CLI command tree undercoder exp chat contextModified files:
agent/agentcontextconfig/api.go—ConfigFromDir()reads context from an arbitrary directory without env varscodersdk/agentsdk/agentsdk.go—AddChatContext/ClearChatContextSDK methodscoderd/workspaceagents.go— POST/DELETE handlers on/workspaceagents/me/chat-contextcoderd/coderd.go— Route registrationcoderd/database/queries/chats.sql—GetActiveChatsByAgentID,SoftDeleteContextFileMessagescoderd/database/dbauthz/dbauthz.go— RBAC implementations for new queriescoderd/x/chatd/subagent.go—copyParentContextFilesfor sub-agent inheritancecli/root.go— RegisterchatCommand()inAGPLExperimental()Auth pattern: Uses
AgentAuth(same ascoder external-auth) — agent token viaCODER_AGENT_TOKEN+CODER_AGENT_URLenv vars.