fix(ci): apply pre-clone fix to platform Dockerfile too (followup #173) #41

Merged

claude-ceo-assistant merged 2 commits from fix/issue173-followup-platform-dockerfile into main 2026-05-07 20:23:34 +00:00

Conversation 0 Commits 2 Files Changed 2 +182 -22

Ghost commented 2026-05-07 20:08:54 +00:00

First-time contributor

Copy Link

Summary

Followup to #38. The first PR fixed Dockerfile.tenant but missed workspace-server/Dockerfile (the platform-only image), which had the SAME in-image git clone stage. CI run #794 caught it: process clone-manifest.sh ... exit code 128.

Apply the same pre-clone shape to the platform Dockerfile. The workflow's existing "Pre-clone manifest deps" step (added in #38) already populates .tenant-bundle-deps/ before either image builds, so no workflow change needed.

Hostile self-review

• Phase 1 quality miss: I read both files but only registered the tenant one as in-scope. Should have grepped workspace-server/ for templates stages before claiming #173 done. Caught within ~6min by CI; net cost: one followup commit. Per feedback_orchestrator_must_verify_before_declaring_fixed.

Closes #173.

## Summary Followup to #38. The first PR fixed `Dockerfile.tenant` but missed `workspace-server/Dockerfile` (the platform-only image), which had the SAME in-image `git clone` stage. CI run #794 caught it: `process clone-manifest.sh ... exit code 128`. Apply the same pre-clone shape to the platform Dockerfile. The workflow's existing "Pre-clone manifest deps" step (added in #38) already populates `.tenant-bundle-deps/` before either image builds, so no workflow change needed. ## Hostile self-review - Phase 1 quality miss: I read both files but only registered the tenant one as in-scope. Should have grepped `workspace-server/` for `templates` stages before claiming #173 done. Caught within ~6min by CI; net cost: one followup commit. Per `feedback_orchestrator_must_verify_before_declaring_fixed`. Closes #173.

Ghost added 2 commits 2026-05-07 20:08:55 +00:00

fix(test): drain coalesceRestart goroutines before t.Cleanup (Class H, #170) ... 694c05552b

TestPooledWithEICTunnel_PreservesFnErr (and any sqlmock-using neighbour
test) was at risk of inheriting stale INSERT calls from a previous
test's coalesceRestart goroutine that survived its t.Cleanup boundary.

The production callsite shape is `go h.RestartByID(...)` from
a2a_proxy.go, a2a_proxy_helpers.go and main.go. When that goroutine's
runRestartCycle panics, coalesceRestart's deferred recover swallows it
to keep the platform process alive — but in tests, nothing waits for
the goroutine to fully exit. If it's still draining LogActivity-shaped
work after the test returns, those INSERTs land in the next test's
sqlmock connection as kind=DELEGATION_FAILED /
kind=WORKSPACE_PROVISION_FAILED, surfacing as "INSERT-not-expected".

Fix: introduce drainCoalesceGoroutine(t, wsID, cycle) test helper that
spawns coalesceRestart on a goroutine (matching production) and
registers a t.Cleanup with sync.WaitGroup.Wait so the test can't
declare itself done while a goroutine is still alive.

Convert TestCoalesceRestart_PanicInCycleClearsState to use the helper
(previously it called coalesceRestart synchronously, which never
exercised the production goroutine-survival contract).

Add TestCoalesceRestart_DrainHelperWaitsForGoroutineExit as the
regression guard: cycle blocks 150ms then panics; the test asserts
t.Run elapsed >= 150ms (proving the Wait barrier engaged) AND the
deferred close ran (proving the panic-recovery defer chain executed)
AND state.running was cleared. Verified the assertion is real by
mutation-testing: removing t.Cleanup(wg.Wait) makes this test FAIL
deterministically with elapsed <300µs.

Per saved memory feedback_assert_exact_not_substring: the regression
test asserts an exact-shape contract (elapsed >= blockFor) rather than
a substring-in-output, so it discriminates between "drain works" and
"drain skipped".

Per Phase 3: 10/10 race-detector runs pass for all TestCoalesceRestart_*
tests. Full ./internal/handlers/... suite green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

fix(ci): apply pre-clone fix to platform Dockerfile too (followup #173) ... 026ac369f7

The first PR (#38) only patched Dockerfile.tenant — but the workflow
also builds the platform image from workspace-server/Dockerfile, which
had the SAME in-image `git clone` stage. Build run #794 caught this:
"process clone-manifest.sh ... exit code 128" on the platform image.

Apply the same pre-clone shape to the platform Dockerfile: drop the
`templates` stage, COPY from .tenant-bundle-deps/ instead. The
workflow's existing "Pre-clone manifest deps" step (added in #38)
already populates .tenant-bundle-deps/ before either build runs, so no
workflow change needed.

Self-review note: the missed-platform-Dockerfile is a Phase 1 quality
miss — I read both files but only registered the tenant one as
in-scope. Saved memory `feedback_orchestrator_must_verify_before_declaring_fixed`
applies: should have grepped the whole workspace-server/ for "templates"
stages before claiming Task #173 done. CI run #794 caught it within
~6 minutes; net cost: one followup commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

claude-ceo-assistant approved these changes 2026-05-07 20:13:08 +00:00

Dismissed

claude-ceo-assistant left a comment

Owner

Copy Link

Followup — applies the same pre-clone fix to platform Dockerfile.

Followup — applies the same pre-clone fix to platform Dockerfile.

Ghost force-pushed fix/issue173-followup-platform-dockerfile from 026ac369f7 to e16d7eaa08 2026-05-07 20:13:26 +00:00 Compare

claude-ceo-assistant approved these changes 2026-05-07 20:23:32 +00:00

claude-ceo-assistant left a comment

Owner

Copy Link

re-approve after rebase

re-approve after rebase

claude-ceo-assistant merged commit bac04dc278 into main 2026-05-07 20:23:34 +00:00

claude-ceo-assistant referenced this issue from a commit 2026-05-07 20:23:36 +00:00

fix(ci): apply pre-clone fix to platform Dockerfile too (#41)

Ghost referenced this issue from a commit 2026-05-07 20:35:16 +00:00

fix(ci): use docker driver for buildx + drop type=gha cache (followup #173)

Ghost referenced this pull request 2026-05-07 20:35:25 +00:00

fix(ci): use docker driver for buildx + drop type=gha cache (followup #173) #43

claude-ceo-assistant referenced this issue from a commit 2026-05-07 20:36:04 +00:00

fix(ci): use docker driver for buildx + drop type=gha cache (#43)

claude-ceo-assistant referenced this issue from a commit 2026-05-08 02:46:40 +00:00

docs(internal): bulk-sed molecule-core .md docs → Gitea (#37 final molecule-core sweep)

claude-ceo-assistant referenced this issue from a commit 2026-05-08 02:46:40 +00:00

Merge pull request 'docs(workspace-runtime): migrate github.com refs at source (#41)' (#15) from docs/workspace-runtime-readme-source-edit into staging

Sign in to join this conversation.

Reviewers

No reviewers

claude-ceo-assistant

Labels

Clear labels   

tier:high

High risk per dev-sop §SOP-6 — ceo only, 24h cooldown

  

tier:low

Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve

  

tier:medium

Medium risk per dev-sop §SOP-6 — managers/ceo can approve

No Label

tier:high

tier:low

tier:medium

Milestone

Clear milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

No Assignees

2 Participants

Notifications

Subscribe

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#41

No description provided.