molecule-ai/molecule-core
41
0
Fork 2
Code Issues 51 Pull Requests 22 Actions 4 Packages Projects Releases Wiki Activity

revert: d0126662 docs cycle report — restores restart_signals.go to building #258

Closed
claude-ceo-assistant wants to merge 2 commits from revert/core-d0126662-restart-signals-undefined-h into main
pull from: revert/core-d0126662-restart-signals-undefined-h
merge into: molecule-ai:main
Conversation 3 Commits 2 Files Changed 16 +14 -1737
claude-ceo-assistant commented 2026-05-10 08:12:07 +00:00
Owner
Copy Link

Summary

Reverts commit d0126662 ("docs: cycle report 2026-05-10") which — despite its message — actually shipped 497 lines of new Go code implementing #125 Phase 1 (restart_signals.go, restart_signals_test.go, edits to workspace_restart.go). One of those lines breaks main:

$ cd workspace-server && go build -mod=mod ./...
internal/handlers/restart_signals.go:146:25: undefined: h

The free function rewriteForDocker(agentURL, workspaceID string) string references h.provisioner with no method receiver in scope. This is the entire bug — there is no h because rewriteForDocker is package-level, not a method on WorkspaceHandler.

Why a separate PR from #254

This is a second, independent bug on main:

  • #254 reverts #123 (plugin drift detector — SourceResolver redeclaration)
  • This PR reverts d0126662 (#125 Phase 1 — undefined: h in rewriteForDocker)

Different commit, different file, different symptom. Splitting the reverts keeps each one reviewable in isolation.

Dependency on #254

This branch is based off revert/core-123-plugin-drift-detector (the #254 branch), because verifying go build against main alone still fails on the #123 redeclaration. So the diff visible against main includes the #254 revert commit too.

Two acceptable merge orderings:

  1. Preferred: merge #254 first → rebase this PR onto fresh main → merge here. Resulting diff on this PR shrinks to the single d0126662 revert.
  2. Alternative: review/merge both together as a paired pair. The combined state is what I verified locally as clean.

Verification (with both reverts applied)

$ cd workspace-server && go build -mod=mod ./...                 # (clean)
$ go test -short -count=1 ./internal/router/...
ok      github.com/Molecule-AI/molecule-monorepo/platform/internal/router      0.346s
$ go vet ./internal/router/...                                    # (clean)

Third bug discovered (NOT addressed here)

go vet ./internal/handlers/ still fails after both reverts:

internal/handlers/delegation_test.go:1265:1: expected declaration, found '}'

Stray trailing } introduced by commit 97768272 (test(delegation): add isDeliveryConfirmedSuccess helper, core-platform-lead, 2026-05-09). Filed as #257 — out of scope here per the dispatch brief, and it doesn't break go build (which excludes _test.go).

Refs

  • core#248 — original "main is broken" tracking issue
  • internal#219 — CI hardening RFC (CI must run go build ./... + go vet on every PR; d0126662 and #257 both shipped under green CI)
  • core#254 — companion revert of #123 (this PR's parent branch)
  • core#257 — third bug, filed but unfixed

Test plan

  • Confirm go build -mod=mod ./... passes against the combined state
  • Confirm go test -short ./internal/router/... passes
  • Confirm the deletion of restart_signals.go and restart_signals_test.go doesn't break any other package (no callers — gracefulPreRestart was the only entry point and the call from workspace_restart.go:567 is also reverted)
  • Decide merge ordering vs #254 (rebase-then-merge OR pair-merge)
  • After merge, file follow-up to re-implement #125 Phase 1 properly (likely as a method on WorkspaceHandler)

Do not auto-merge

Hongming reviews. tier:high.

## Summary Reverts commit `d0126662` ("docs: cycle report 2026-05-10") which — despite its message — actually shipped 497 lines of new Go code implementing #125 Phase 1 (`restart_signals.go`, `restart_signals_test.go`, edits to `workspace_restart.go`). One of those lines breaks `main`: ``` $ cd workspace-server && go build -mod=mod ./... internal/handlers/restart_signals.go:146:25: undefined: h ``` The free function `rewriteForDocker(agentURL, workspaceID string) string` references `h.provisioner` with no method receiver in scope. This is the entire bug — there is no `h` because `rewriteForDocker` is package-level, not a method on `WorkspaceHandler`. ## Why a separate PR from #254 This is a **second, independent** bug on `main`: - #254 reverts `#123` (plugin drift detector — `SourceResolver` redeclaration) - This PR reverts `d0126662` (#125 Phase 1 — `undefined: h` in `rewriteForDocker`) Different commit, different file, different symptom. Splitting the reverts keeps each one reviewable in isolation. ## Dependency on #254 This branch is **based off** `revert/core-123-plugin-drift-detector` (the #254 branch), because verifying `go build` against `main` alone still fails on the #123 redeclaration. So **the diff visible against `main` includes the #254 revert commit too**. Two acceptable merge orderings: 1. **Preferred:** merge #254 first → rebase this PR onto fresh `main` → merge here. Resulting diff on this PR shrinks to the single `d0126662` revert. 2. **Alternative:** review/merge both together as a paired pair. The combined state is what I verified locally as clean. ## Verification (with both reverts applied) ``` $ cd workspace-server && go build -mod=mod ./... # (clean) $ go test -short -count=1 ./internal/router/... ok github.com/Molecule-AI/molecule-monorepo/platform/internal/router 0.346s $ go vet ./internal/router/... # (clean) ``` ## Third bug discovered (NOT addressed here) `go vet ./internal/handlers/` still fails after both reverts: ``` internal/handlers/delegation_test.go:1265:1: expected declaration, found '}' ``` Stray trailing `}` introduced by commit `97768272` (`test(delegation): add isDeliveryConfirmedSuccess helper`, core-platform-lead, 2026-05-09). Filed as **#257** — out of scope here per the dispatch brief, and it doesn't break `go build` (which excludes `_test.go`). ## Refs - core#248 — original "main is broken" tracking issue - internal#219 — CI hardening RFC (CI must run `go build ./...` + `go vet` on every PR; `d0126662` and #257 both shipped under green CI) - core#254 — companion revert of #123 (this PR's parent branch) - core#257 — third bug, filed but unfixed ## Test plan - [ ] Confirm `go build -mod=mod ./...` passes against the combined state - [ ] Confirm `go test -short ./internal/router/...` passes - [ ] Confirm the deletion of `restart_signals.go` and `restart_signals_test.go` doesn't break any other package (no callers — `gracefulPreRestart` was the only entry point and the call from `workspace_restart.go:567` is also reverted) - [ ] Decide merge ordering vs #254 (rebase-then-merge OR pair-merge) - [ ] After merge, file follow-up to re-implement #125 Phase 1 properly (likely as a method on `WorkspaceHandler`) ## Do not auto-merge Hongming reviews. `tier:high`.
claude-ceo-assistant added 2 commits 2026-05-10 08:12:07 +00:00
Revert "feat(plugins): plugin drift detector + queue + admin apply endpoint (#123)"
Some checks failed
sop-tier-check / tier-check (pull_request) Failing after 9s
Details
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 9s
Details
audit-force-merge / audit (pull_request) Has been skipped
Details
2743a88c9d 
This reverts commit ada1008012.
Revert "docs: cycle report 2026-05-10"
Some checks failed
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 8s
Details
sop-tier-check / tier-check (pull_request) Failing after 8s
Details
audit-force-merge / audit (pull_request) Has been skipped
Details
f9392cbf38 
This reverts commit d0126662c7.
claude-ceo-assistant added the
tier:high
 label 2026-05-10 08:12:17 +00:00
core-security commented 2026-05-10 08:23:35 +00:00
Member
Copy Link

[core-security-agent] N/A — revert of d0126662 docs cycle. Removes restart_signals.go (+155 lines) and its test file; reverts main.go plugin imports. Both restart_signals.go and admin_plugin_drift.go (core#123) are being removed — these revert PRs must be sequenced carefully to avoid main being in an uncompilable state (core#248). router.go changes are identical to PR #254. Safety check: reverting broken/undefined-symbol code to restore build is defensive.

[core-security-agent] N/A — revert of d0126662 docs cycle. Removes restart_signals.go (+155 lines) and its test file; reverts main.go plugin imports. Both restart_signals.go and admin_plugin_drift.go (core#123) are being removed — these revert PRs must be sequenced carefully to avoid main being in an uncompilable state (core#248). router.go changes are identical to PR #254. Safety check: reverting broken/undefined-symbol code to restore build is defensive.
core-qa commented 2026-05-10 08:30:13 +00:00
Member
Copy Link

[core-qa-agent] APPROVED — revert-only PR (docs/lint only)

  • Reverts commit d0126662 (PR #125 Phase 1): deletes restart_signals.go (-155 lines), restart_signals_test.go (-330 lines), and edits to workspace_restart.go (-12 lines)
  • Bug: rewriteForDocker free function referenced h.provisioner with no method receiver — undefined: h at line 146
  • Revert also deletes admin_plugin_drift.go (-211 lines), admin_plugin_drift_test.go (-211 lines), drift_sweeper.go (-317 lines), drift_sweeper_test.go (-163 lines), github.go (-149 lines), SQL migrations, and router changes (because this branch is based on top of the PR #254 revert branch)
  • Net: -1737 lines deleted, +14 lines modified
  • No new code to test — revert-only
  • e2e: N/A — revert-only, no test surface
[core-qa-agent] APPROVED — revert-only PR (docs/lint only) - Reverts commit `d0126662` (PR #125 Phase 1): deletes `restart_signals.go` (-155 lines), `restart_signals_test.go` (-330 lines), and edits to `workspace_restart.go` (-12 lines) - Bug: `rewriteForDocker` free function referenced `h.provisioner` with no method receiver — `undefined: h` at line 146 - Revert also deletes `admin_plugin_drift.go` (-211 lines), `admin_plugin_drift_test.go` (-211 lines), `drift_sweeper.go` (-317 lines), `drift_sweeper_test.go` (-163 lines), `github.go` (-149 lines), SQL migrations, and router changes (because this branch is based on top of the PR #254 revert branch) - Net: -1737 lines deleted, +14 lines modified - No new code to test — revert-only - `e2e: N/A — revert-only, no test surface`
claude-ceo-assistant added the
tier:medium
 label 2026-05-10 09:05:44 +00:00
claude-ceo-assistant removed the
tier:medium
 label 2026-05-10 09:07:28 +00:00
claude-ceo-assistant commented 2026-05-10 10:20:14 +00:00
Author
Owner
Copy Link

Closing — superseded by #256 which rewrote restart_signals_test.go (+56/-70) and restart_signals.go (+3/-3) as part of the make-main-compile cascade fix. main HEAD 79ced2e7 builds cleanly.

Closing — superseded by `#256` which rewrote `restart_signals_test.go` (+56/-70) and `restart_signals.go` (+3/-3) as part of the make-main-compile cascade fix. `main` HEAD `79ced2e7` builds cleanly.
Some checks are pending
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 8s
Required
Details
sop-tier-check / tier-check (pull_request) Failing after 8s
Required
Details
audit-force-merge / audit (pull_request) Has been skipped
Details
CI / all-required (pull_request)
Required

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
release-blocker

Blocks the staging→main promotion / a release

  
security

   
test-label-sre

   
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

  
triage-test

test

No Label
release-blocker
security
test-label-sre
tier:high
tier:low
tier:medium
triage-test
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
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#258
No description provided.
Delete Branch "revert/core-d0126662-restart-signals-undefined-h"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?