Repository navigation
test(coderd/x/chatd): remove end-to-end root and child tool list tests - #30992
ibetitsmike wants to merge 3 commits into
Conversation
TestSubagentChatExcludesWorkspaceProvisioningTools booted coderd with a provisioner and AI Bridge to compare root and child tool names, and a tail of the basic Anthropic prompt preparation test ran a plan child turn to check read_file and write_file. The Root, GeneralChild and PlanChild rows of TestBuildTurnEnvironmentToolPolicy check the same names. Also remove the plan mode MCP invocation test TestPlanModeRootChatApprovedExternalMCPToolInvocation: its steps and checks are the first half of the kept propose_plan workflow test.
|
@codex review
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ 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. 🎉 Reviewed commit: ℹ️ 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". |
This PR deletes three end-to-end checks of model request tool lists (309 lines); the rows from #30990 check the same lists on the turn environment.
TestSubagentChatExcludesWorkspaceProvisioningToolsbooted coderd with a provisioner and AI Bridge to compare root and child tool names.TestActiveServer_BasicAssistantGenerationAndPromptPreparationcheckedread_fileandwrite_file. Its only helper,createPlanSubagentChatWithHistory, goes too.TestPlanModeRootChatApprovedExternalMCPToolInvocationis the first half ofTestPlanModeRootChatApprovedExternalMCPWorkflowCanReachProposePlan, with the same three checks.Why it is safe
Each mutant of
turn_environment.gobelow failed the deleted check and fails the remaining one:Rootask_user_questionorpropose_planRootask_user_questiontoolsGeneralChildask_user_questionin childrenread_fileand nowrite_filePlanChildwaiting, and sends the result to the next callPlanRootThe split reads only
ParentChatID,Mode, andPlanModefrom the chat row (turn_environment.go:258-261), so a table child made withchatstate.CreateChatgets the tools of aspawn_agentchild made withchatstate.CreateChatWithID. TheTestSpawnAgent_*tests cover spawning. The deleted sub-agent test replied with an escaped NUL sequence but never checked the stored reply;TestNulEscapeRoundTrip/BackslashU0000Textcovers that text.Coverage: in 6 runs per head of
go test ./coderd/x/chatd/... -count=1 -p 4 -covermode=set -coverpkg=./coderd/x/chatd/..., every block that all runs at #30990 reach is also reached at #30993. The other differences are blocks that some runs of the same head reach and others miss, mostly error paths that depend on timing, so one run per head can show them as lost or gained.Timing
From 10 runs, the two deleted tests took 1.14 s summed (median per test). The basic test went from 0.51 s to 0.41 s (median of 5).
Validation
go test -overlaymutant runs of the whole./coderd/x/chatd/package before and after this PR: results abovego vet ./coderd/x/chatd/: clean; the pre-commit hook (gen, fmt, lint, slim build) passedStack
The stack moves advisor eligibility and the root versus child tool split into
TestBuildTurnEnvironmentToolPolicyand deletes the end-to-end tests that only checked them, finishing #30868. It is based on #30911.