Skip to content

fix(thread): stop thread ask from sending its prompt twice - #141

Merged
nwm-54 merged 3 commits into
mainfrom
fix/thread-ask-single-send
Nov 10, 2026
Merged

nwm-54 merged 3 commits into
mainfrom
fix/thread-ask-single-send

Conversation

@nwm-54

@nwm-54 nwm-54 commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Why

Every polylane thread ask produced two identical user messages on the new thread. The API's thread create call already sends the prompt as the first user message (coreplanelabs/nominal#2516), and the CLI then posted the same prompt again as a separate message.

What changed

  • thread ask no longer calls the messages endpoint after creating the thread; the prompt travels only in the create call.
  • Removes the retired Conductor integration from the CLI picker, type list, credential instructions and help. The backend removed it in coreplanelabs/nominal#6150; stale CLI entries caused CI type errors.
  • A new test records every request a --no-wait ask makes and expects exactly one POST to the threads endpoint.

Scope

  • Covers thread ask only. thread continue posts to an existing thread, so its single messages call is correct.
  • Leaves threads already created with duplicate messages as they are.

Blast Radius

The ask command stops sending duplicate prompts. The integration picker also stops offering the retired Conductor service. The server contract it now relies on is the same one the console has used since the create call started sending the first message.

Verification

  • Before the fix, running the installed CLI against a test workspace created a thread with two user messages 211 ms apart, then one assistant reply.
  • After the fix, running the patched source against the same workspace created a thread with one user message and one assistant reply, and --verbose showed a single POST.
  • After the Conductor cleanup: npm run typecheck, npm run lint, npm run test (623 pass), and npm run build pass locally on Node 26.8.1. Built CLI --version and --help smoke checks pass.
  • The duplicate-send regression test fails when the deleted messages POST is restored. The updated picker test fails with the retired Conductor option restored.
  • The Node 20/22/24 matrix is verified separately by CI.

🤖 Generated with Claude Code

Vi Tran and others added 2 commits November 9, 2026 22:55
The API's thread create call already sends the prompt as the first user
message (nominal#2516), so a second messages post duplicates it. This
test records every request a --no-wait ask makes and expects one.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
threads.post sends the prompt as the first user message server-side
(nominal#2516), and the follow-up messages post sent the same prompt
again, so every ask produced two identical user messages.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@polylane

polylane Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Warning

Polylane could not verify the production impact of this pull request.

The create route carries the prompt: nominal-api-threads-prod served 83,310 requests over 72h at 0 errors, and the server builds the first user message from the create body, so the deleted POST was a duplicate. The Conductor entry has no remaining reference in the repo and the CLI cannot change the server's support for it.

Check Result
Impact assessment Production impact not verified
Security review Passed · 8 ruled out

View the full analysis →

Also considered · 2 refuted
  • Refuted · Removed messages POST leaves an ask with no first message or no agent turn · The prompt travels in the create body and the route builds the first user message from it, so the deleted call was the duplicate the pull request removes.
  • Refuted · Conductor removal strands workspaces that still connect the Conductor executor · The CLI flag would already fail against the production API contract, because the client is generated from the live spec that omits conductor; the removal aligns the picker with that contract.

nominal-api-threads-prod · requests per hour

Analysed against 8 cloud accounts and 1 repository
  • Cloud accounts: coreplane-prod, baseberry-uat, coreplane-infra, coreplane, coreplane-gtm, 251714435813, Polylane, nominal-493716
  • Repository: coreplanelabs/cli

View in Polylane Pause assessments

Polylane could not find the cloud resources this repository manages, so this review looked at the entire cloud account. Connect this repository to its resources and the next review will focus on exactly what this code deploys to.

Connect resources

Polylane analysed 59d87bb for production impact and security. You can ask follow-ups by mentioning @polylane in a comment.

Previous verdicts (1)
Head Verdict Analysis
f9910fb Production impact not verified analysis

@nwm-54 nwm-54 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review complete. Merge readiness is blocked by the existing Conductor API contract mismatch.

I am Codex writing on behalf of Vi Tran.

Reviewed f9910fb in the current coding client, including a second independent read-only review. No correctness or security findings in the two-file diff. The threads create route sends the initial prompt server-side. The waiting path only polls, and thread continue retains its legitimate message POST.

Verification completed locally: npm ci, codegen, lint, and all 623 tests pass on Node 26.8.1. The new regression test fails when the deleted messagesPost call is restored and passes with this PR. The Node 20/22/24 CI matrix is not green.

Typecheck fails at src/commands/integration/connect.ts:422 and :1497 because the live production OpenAPI spec omits conductor. I reproduced both errors on current main at 9885576 using a separate worktree. The integration connect file is identical between main and this head. A cache-busted production spec request also omits conductor. The same two diagnostics fail the current PR CI run: https://github.com/coreplanelabs/cli/actions/runs/42771979121

Required next action: reconcile the production API spec with the existing Conductor integration contract, then rerun the checks. This requires a separate contract repair; this duplicate-send fix does not cause the errors. No source changes were needed for this review. No unresolved review threads are present.

@nwm-54 nwm-54 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM: the prompt is sent once, and the CLI no longer offers the retired Conductor integration.

I am Codex writing on behalf of Vi Tran.

Reviewed current head 59d87bb in this coding client, including independent review of the final diff. No actionable findings remain.

Correction to my earlier review: the backend deliberately removed Conductor in coreplanelabs/nominal#6150. The API spec is correct; the CLI entries were stale. Commit 59d87bb removes those entries from the connect type, picker, credential instructions and help, and updates the picker expectations. This clears both reported type errors without weakening the generated API types.

Local verification on Node 26.8.1: codegen, typecheck, lint, all 623 tests, build, and built CLI --version/--help pass. Both behavior tests were verified red before their fixes and green after. The independent reviewer also ran the two affected test files, with all 13 tests passing. The PR description reflects the final scope.

CI must complete on this new head before merge readiness is confirmed. No merge was performed.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved: nwm-54 reviewed this PR and posted an LGTM verdict (see its review). A repo admin enabled this via the auto-approve workflow.

@nwm-54
nwm-54 merged commit 279c22c into main Nov 10, 2026
6 checks passed
@nwm-54
nwm-54 deleted the fix/thread-ask-single-send branch November 10, 2026 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant