fix(workspace-server): cascade-delete race + ACTIVITY_LOGGED body fidelity - #2188
Merged
hongmingwang-moleculeai merged 2 commits intoApr 27, 2026
Merged
Conversation
Cascade-deleting a 7-workspace org returned 500 with
"workspace marked removed, but 2 stop call(s) failed — please retry:
stop eeb99b5d-...: force-remove ws-eeb99b5d-607: Error response
from daemon: removal of container ws-eeb99b5d-607 is already in
progress"
even though the DB-side post-condition succeeded (removed_count=7) and
the containers WERE removed shortly after. The fanout fired Stop() on
every workspace concurrently and the orphan sweeper happened to reap
two of them at the same instant, so Docker rejected the second
ContainerRemove with "removal already in progress" — a race-condition
ack, not a real failure. Retrying just races the same in-flight
removal.
The post-condition we care about (the container WILL be gone) is
identical to a successful removal, so Stop() should treat it the
same way it already treats "No such container" — a no-op return nil
that lets the caller proceed with volume cleanup. Real daemon
failures (timeout, EOF, ctx cancel) still surface as errors.
Two pieces:
- New isRemovalInProgress() predicate using the same string-match
approach as isContainerNotFound (docker/docker has no typed
errdef for this; the CLI itself relies on the message).
- Stop() now treats the predicate as success, with a log line
distinct from the not-found path so debugging can tell which
race fired.
Both substrings ("removal of container" + "already in progress") must
match — "already in progress" alone would false-positive on unrelated
operations like image pulls. Truth table pinned in 7 new test cases.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
requested a review
from hongmingwang-moleculeai
as a code owner
April 27, 2026 20:25
…adcast
Canvas Agent Comms bubbles for outbound delegation showed only
"Delegating to <peer>" boilerplate during the live update window —
the actual task text only surfaced after a refresh re-fetched the row
from /workspaces/:id/activity. Symptom flagged today during a fresh
delegation manual test where the bubble said "Delegating to Perf
Auditor" instead of the user's "audit moleculesai.app for
performance" prompt.
Root cause: LogActivity's broadcast payload at activity.go:510-518
deliberately omitted request_body and response_body, so the canvas's
live-update path (AgentCommsPanel.tsx:271-289) saw `p.request_body =
undefined` and toCommMessage fell back to the
`Delegating to ${peerName}` template string. The DB row stored the
real task / reply, which is why GET-on-mount worked.
Fix: include both bodies in the broadcast as json.RawMessage values
(no re-marshal cost — they were already encoded for the DB insert
above). Same pattern as tool_trace, which has been included since #1814.
Each side is bounded by the workspace-side caller's own caps: the
runtime's report_activity helper caps error_detail at 4096 chars and
summary at 256; request/response are constrained by the runtime's
own limits — typical delegate_task payload is hundreds of chars to a
few KB. If a much-larger broadcast becomes a concern later, a soft
cap can be added at this site without breaking the contract.
Two regression tests pin the broadcast shape:
- request_body present → canvas renders the actual task text
- response_body present → canvas renders the actual reply text
- response_body nil → omitted from payload (no empty-bubble flicker)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
changed the title
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…provider catalog — platform bucket + correct llm_provider for moonshot/kimi-k2.6' (#2188) from fix/create-dialog-registry-provider-catalog into main
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…t/kimi NOT_CONFIGURED The moonshot/kimi incident: a canvas-created claude-code workspace with provider=Platform + model=moonshot/kimi-k2.6 booted NOT_CONFIGURED in prod because the generated config.yaml lacked the manifest-derived `provider:` key, so the adapter slash-split "moonshot/..." -> unregistered provider. Fixed by #2187 (ensureDefaultConfig stamps DeriveProvider->provider:platform) + #2188 (canvas). Unit tests passed; the REAL boot path was the gap. This adds comprehensive regression coverage so the CLASS cannot reship: Deterministic (no live infra, runs in the normal unit suite): workspace-server/internal/handlers/workspace_provision_platform_boot_test.go - TestEnsureDefaultConfig_StampsProviderForEverySSOTPlatformModel: enumerates the claude-code `platform` arm from the providers SSOT (providers.LoadManifest) and asserts ensureDefaultConfig stamps provider:platform (top-level AND runtime_config) for EVERY offered platform model — not just the single moonshot/kimi pin #2187 shipped. A newly-offered platform model gets a case for free and only passes if actually stamped (closes the offered-but-not-stamped divergence the bug rode in on). Mutation-verified: disabling the stamp fails the test. - TestPlatformModelDeriveProvider_SSOTConsistency: the upstream half — DeriveProvider maps every SSOT platform model to provider Name "platform". Real-boot (staging; I will run it): Extends the existing staging harness (no new harness) with a platform-managed path: E2E_LLM_PATH=platform pin-selects moonshot/kimi-k2.6, sends NO tenant key, and reuses the harness's online-wait + completion assertions to prove the workspace reaches status=online (not not_configured) and a completion returns 200. The BYOK branches never exercised the platform arm — the exact arm the bug shipped on. - tests/e2e/lib/model_slug.sh: platform path + override semantics - tests/e2e/test_model_slug.sh: 4 new pinned cases (16/16 green) - tests/e2e/test_staging_full_saas.sh: empty-secrets platform branch - .gitea/workflows/e2e-staging-saas.yml: new `E2E Staging Platform Boot` job (continue-on-error during de-flake; bp-required: pending #2187), + providers.yaml/model_slug.sh added to the path triggers. Coverage-audit theme: mc#1982 (continue-on-error masks; de-flake-then-gate). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cascade-deleting a 7-workspace org returned 500 with `workspace marked removed, but 2 stop call(s) failed — please retry: stop X: force-remove ws-X: Error response from daemon: removal of container ws-X is already in progress` even though the DB post-condition succeeded (`removed_count=7`) and the containers were in fact gone shortly after.
The platform fans out `Stop()` on every workspace in parallel, and the orphan sweeper happened to reap two of them at the same instant. Docker rejects the second `ContainerRemove` for the same container with `removal already in progress` — a race-condition ack, not a real failure. Retrying just races the same in-flight removal.
The post-condition (container WILL be gone) is identical to a successful removal, so `Stop()` should treat the race-ack the same way it already treats `No such container`: a no-op `return nil` that lets the caller proceed with volume cleanup. Real daemon failures (timeout, EOF, context cancel) still surface as errors.
Changes
Test plan
🤖 Generated with Claude Code