molecule-ai/molecule-core
52
0
Fork 2
Code Issues 186 Pull Requests 52 Actions 8 Packages Projects Releases Wiki Activity

fix(ci): remove || true suppression from lint steps #2035

Closed
core-be wants to merge 15 commits from fix/ci-lint-suppression-1062 into staging
pull from: fix/ci-lint-suppression-1062
merge into: molecule-ai:staging
Conversation 1 Commits 15 Files Changed 21
+301 -58
core-be commented 2026-06-01 03:36:05 +00:00
Member
Copy Link
Copy Source
No description provided.
core-be added 17 commits 2026-06-01 03:36:05 +00:00
fix(workspace-server): CP orphan sweeper closes deprovision split-write race (#2989) 4c2d22010d 
The deprovision path marks `workspaces.status='removed'` BEFORE calling
the controlplane DELETE. If that CP call fails (transient 5xx, network
hiccup, AWS provider error), the DB row stays at 'removed' with
`instance_id` populated and there's no retry — the EC2 lives forever.
9 prod orphans accumulated over 3 days under this bug.

Adds a SaaS-mode counterpart to the existing Docker `orphan_sweeper`:
- 60s tick (matches the Docker sweeper cadence)
- LIMIT 100 per cycle so a sustained CP outage drains over multiple
  cycles without blowing the request timeout
- Re-issues `cpProv.Stop` for any workspace at status='removed' with a
  non-NULL `instance_id`. Stop is idempotent (AWS terminate on
  already-terminated is a no-op; CP's Deprovision tolerates already-
  deleted DNS) so retries are safe.
- On Stop success, NULLs `instance_id` so the next cycle skips the row.
- On Stop failure, leaves `instance_id` populated for next cycle.

The existing Docker sweeper is gated on `prov != nil`; the new sweeper
is gated on `cpProv != nil`. SaaS tenants get exactly one of the two,
self-hosted tenants get the Docker one — no overlap.

Why this shape over option A (CP-first ordering) or B (durable outbox):
the existing inline path already returns a loud 500 to the user when
CP fails — the only missing piece is automatic retry, which a 60s
sweeper provides without protocol changes, new tables, or new workers.
~30 LOC of production code vs. ~400 for an outbox. RFC discussion in
#2989 comment chain.

Tests:
- 9 unit tests covering happy path, Stop failure, UPDATE failure,
  multiple orphans (one-fails-others-still-process), DB query error,
  nil-DB defense, nil-reaper short-circuit, and the boot-immediate-then-
  tick cadence contract.
- Mutation-tested: status='running' substitution and removed-UPDATE-
  block both fail at least one test.

Out of scope:
- Backfilling the 9 named orphans — they'll heal automatically on the
  first sweep cycle after this lands; no manual cleanup needed.
- Long-term durable-outbox architecture — separate RFC.
review: 5-axis review of PR #3029 and PR #3033 43d76dcc2b 
- PR #3029: approve with nit (CP orphan sweeper + registry prefix).
- PR #3033: approve with blocker — README undercounts runtimes by 1
  because #3029 adds codex but #3033 claims 8 runtimes and omits it.
- All tests pass locally.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
fix(http): replace http.DefaultClient in auth-adjacent paths c66cca8f24 
Replaces http.DefaultClient in MCP bridge (mcp_tools.go) and CP config
fetch (cp_config.go) with dedicated clients that have transport-level
timeouts. Eliminates a fragility class where global mutable client +
missing context timeout could hang forever on dead workspaces or slow CP.

- mcpHTTPClient: 5s dial, 30s response header, 5s TLS handshake
- cp_config fetch: 10s client timeout matching context deadline

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
fix(delegation): pass context through updateDelegationStatus 8edab170e9 
updateDelegationStatus was using context.Background() internally,
discarding any caller deadline. This meant status updates could hang
indefinitely if the DB was slow, and cancellation signals from HTTP
request contexts were ignored.

Change signature to accept context.Context and update all call sites.
No behavioral change for happy path; fixes context-timeout hygiene.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
fix(context): propagate ctx to DB queries in provision + discovery e966624563 
Two production code paths used context-less sql.DB methods:

1. buildProvisionerConfig (workspace_provision.go:263) called db.DB.QueryRow
   instead of QueryRowContext(ctx, ...) — losing request cancellation.

2. queryPeerMaps (discovery.go:307) called db.DB.Query instead of
   QueryContext(ctx, ...) and did not accept a context parameter. Updated
   signature and all 4 call sites in Peers handler to pass ctx.

No behavioral change for happy path; fixes context-timeout hygiene so
slow queries can be cancelled when the HTTP client disconnects.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
fix(tx): add deferred rollback guard to workspace creation transaction 0ab32a8867 
workspace.go BeginTx had explicit Rollback() on every error path but
no deferred safety net. A panic (or future refactor adding an early
return) would leak the tx hold. Add defer tx.Rollback() immediately
after BeginTx — harmless no-op after Commit, critical safety net on
unexpected exits.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
fix(context): defer rows.Close in restart_context secret key queries c504941cfb 
Two QueryContext calls in restart_context used explicit rows.Close()
after the loop. If a panic or early return occurred during iteration,
the underlying connection would leak back to the pool with an active
result set. Switch to defer rows.Close() inside each if-block for
 RAII-style cleanup.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
fix(channels): recover panics in async webhook HandleInbound goroutine b5efc09fb5 
The async goroutine that processes inbound webhooks had no recover().
A panic inside HandleInbound (e.g., from the A2A proxy or adapter layer)
would crash the entire platform process. Add defer recover() with
workspace-scoped logging so the goroutine exits cleanly and the process
stays alive.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
fix(a2a_proxy): recover panics in fire-and-forget logging goroutines 3f14dda91e 
Four fire-and-forget goroutines in a2a_proxy_helpers.go had no recover():
logA2AFailure, logA2ASuccess, logA2AReceiveQueued, and last_outbound_at
update. A panic inside LogActivity or db.DB.ExecContext would crash the
platform process. Add defer recover() with workspace-scoped logging to
each so the goroutine exits cleanly.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
fix(mcp_tools): recover panics in async delegate_task goroutine e74ce28c1a
fix(goroutines): recover panics in background and fire-and-forget goroutines dcf2f13f0e 
Add panic recovery to goroutines that are NOT wrapped by supervised.RunWithRecover:
- session_auth.go: init() cache sweeper (process-level background task)
- ratelimit.go + mcp_ratelimit.go: bucket cleanup tickers
- bundle/importer.go: fire-and-forget provision start during import
- plugins_install.go: delayed restart after plugin uninstall
- terminal.go: WebSocket bridge goroutines (stdout, PTY, stdin)
- a2a_proxy.go: SSE idle watcher

A panic in any of these would previously crash the process or terminate
a subsystem silently. Now they log and swallow the panic, preserving
process stability.
fix(restart_context): use NewTicker instead of time.After in poll loop cd862093c3 
Prevents bounded timer accumulation during the 30s workspace-online wait.
Each time.After created a timer that couldn't be GC'd until it fired;
with ~60 iterations this was minor but unnecessary.
fix(time.After): replace time.After with NewTimer/NewTicker in loops b17ea4eb11 
Prevents timer accumulation in long-running loops:
- restart_context.go: poll loop uses NewTicker instead of time.After
- supervised.go: backoff sleep uses NewTimer with proper Stop
- telegram.go: poll loop uses NewTimer for both retryAfter and poll interval

Each time.After created a timer that couldn't be GC'd until it fired.
In long-running goroutines this caused bounded but unnecessary memory
growth.
fix(bundle): check errors in markFailed and provision URL update e375258cb7 
Add error checking + logging to previously ignored db.ExecContext and
broadcaster.RecordAndBroadcast calls in bundle importer. Improves
observability when provision failures or URL updates fail silently.
fix(errcheck): log ignored errors in channels, telegram, approvals afe18a5987 
Apply unchecked-error fixes from errcheck audit (#1062):
- channels/manager.go: log db.ExecContext and broadcaster errors in HandleInbound/SendOutbound; return json.Unmarshal errors in loadChannel
- channels/telegram.go: log bot.Send errors for callback ack and edit message
- handlers/approvals.go: log broadcaster, db.QueryRowContext, and db.ExecContext errors

Improves observability when secondary operations fail silently.
fix(workspace): check error on external workspace status update 115dc0d2da
fix(ci): remove || true suppression from lint steps (#1062)
E2E API Smoke Test / E2E API Smoke Test (pull_request) Blocked by required conditions
Details
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Blocked by required conditions
Details
Harness Replays / Harness Replays (pull_request) Blocked by required conditions
Details
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Blocked by required conditions
Details
branch-protection drift check / Branch protection drift (pull_request) Successful in 4s
Details
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 15s
Details
cascade-list-drift-gate / check (pull_request) Successful in 3s
Details
Check merge_group trigger on required workflows / Required workflows have merge_group trigger (pull_request) Successful in 4s
Details
Check migration collisions / Migration version collision check (pull_request) Successful in 7s
Details
CI / Detect changes (pull_request) Successful in 6s
Details
CodeQL / Analyze (${{ matrix.language }}) (go) (pull_request) Failing after 3s
Details
CodeQL / Analyze (${{ matrix.language }}) (javascript-typescript) (pull_request) Failing after 3s
Details
CodeQL / Analyze (${{ matrix.language }}) (python) (pull_request) Failing after 3s
Details
E2E API Smoke Test / detect-changes (pull_request) Successful in 7s
Details
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 5s
Details
Handlers Postgres Integration / detect-changes (pull_request) Successful in 5s
Details
Lint curl status-code capture / Scan workflows for curl status-capture pollution (pull_request) Successful in 9s
Details
Harness Replays / detect-changes (pull_request) Successful in 10s
Details
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 14s
Details
qa-review / approved (pull_request) Successful in 6s
Details
security-review / approved (pull_request) Successful in 4s
Details
Secret scan / Scan diff for credential-shaped strings (pull_request) Failing after 36s
Details
sop-checklist / review-refire (pull_request) Has been skipped
Details
sop-checklist / na-declarations (pull_request) N/A: (none)
Details
Ops Scripts Tests / Ops scripts (unittest) (pull_request) Successful in 35s
Details
Runtime Pin Compatibility / PyPI-latest install + import smoke (pull_request) Successful in 1m38s
Details
gate-check-v3 / gate-check (pull_request) Successful in 6s
Details
sop-checklist / all-items-acked (pull_request) Successful in 6s
Details
sop-tier-check / tier-check (pull_request) Successful in 5s
Details
CI / Shellcheck (E2E scripts) (pull_request) Successful in 24s
Details
CI / Platform (Go) (pull_request) Failing after 1m29s
Details
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 5s
Details
CI / Canvas (Next.js) (pull_request) Failing after 2m25s
Details
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Details
CI / Python Lint & Test (pull_request) Failing after 7m5s
Details
bd428717db 
The `|| true` suffix on `go vet` and `golangci-lint` commands caused
the CI job to pass even when lint errors were present, masking real
issues and defeating the purpose of static analysis.

Remove the suppression so lint failures correctly fail the build.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
core-be changed target branch from main to staging 2026-06-01 03:39:56 +00:00
core-be force-pushed fix/ci-lint-suppression-1062 from bd428717db to e78659faba 2026-06-01 04:22:24 +00:00 Compare
core-be added 1 commit 2026-06-01 12:41:00 +00:00
ci: remove unused canvasUserMessage type to fix lint on staging
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 5s
Details
CI / Detect changes (pull_request) Successful in 8s
Details
CI / Shellcheck (E2E scripts) (pull_request) Successful in 33s
Details
E2E API Smoke Test / detect-changes (pull_request) Successful in 19s
Details
E2E Chat / detect-changes (pull_request) Successful in 8s
Details
CI / Platform (Go) (pull_request) Successful in 5m21s
Details
Handlers Postgres Integration / detect-changes (pull_request) Successful in 11s
Details
CI / Canvas (Next.js) (pull_request) Successful in 6m50s
Details
CI / Python Lint & Test (pull_request) Successful in 7m9s
Details
CI / all-required (pull_request) Successful in 6m10s
Details
Harness Replays / detect-changes (pull_request) Successful in 7s
Details
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 11s
Details
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 5s
Details
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m9s
Details
gate-check-v3 / gate-check (pull_request_target) Successful in 4s
Details
qa-review / approved (pull_request_target) Successful in 6s
Details
security-review / approved (pull_request_target) Successful in 9s
Details
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: comprehensive-testing, local-postgres-e2
Details
sop-checklist / na-declarations (pull_request) N/A: (none)
Details
sop-checklist / all-items-acked (pull_request_target) Successful in 6s
Details
sop-tier-check / tier-check (pull_request_target) Successful in 7s
Details
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 2m28s
Details
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Details
Harness Replays / Harness Replays (pull_request) Successful in 17s
Details
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 13s
Details
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 2m22s
Details
E2E Chat / E2E Chat (pull_request) Failing after 8m20s
Details
audit-force-merge / audit (pull_request_target) Has been skipped
Details
34cecc2b64 
internal/handlers/a2a_proxy_helpers.go:412 had an unused struct that
causes golangci-lint `unused` failure on every PR targeting staging.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
devops-engineer commented 2026-06-01 18:28:37 +00:00
Member
Copy Link
Copy Source

Closing — based on the dead staging fork; superseded/junk relative to main. this edits .github/workflows/ci.yml, a dead GitHub-Actions workflow that does not exist on main (CI is now .gitea/workflows/), and commits a stray review-pr3029-pr3033.md review artifact. The bundled Go error-check sweep is already present on main independently. Per the staging-deprecation cleanup; target main for future PRs.

Closing — based on the dead `staging` fork; superseded/junk relative to `main`. this edits `.github/workflows/ci.yml`, a dead GitHub-Actions workflow that does not exist on `main` (CI is now `.gitea/workflows/`), and commits a stray `review-pr3029-pr3033.md` review artifact. The bundled Go error-check sweep is already present on `main` independently. Per the staging-deprecation cleanup; target `main` for future PRs.
devops-engineer closed this pull request 2026-06-01 18:28:39 +00:00
Some required checks failed
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 5s
Details
CI / Detect changes (pull_request) Successful in 8s
Details
CI / Shellcheck (E2E scripts) (pull_request) Successful in 33s
Details
E2E API Smoke Test / detect-changes (pull_request) Successful in 19s
Details
E2E Chat / detect-changes (pull_request) Successful in 8s
Details
CI / Platform (Go) (pull_request) Successful in 5m21s
Details
Handlers Postgres Integration / detect-changes (pull_request) Successful in 11s
Details
CI / Canvas (Next.js) (pull_request) Successful in 6m50s
Details
CI / Python Lint & Test (pull_request) Successful in 7m9s
Details
CI / all-required (pull_request) Successful in 6m10s
Required
Details
Harness Replays / detect-changes (pull_request) Successful in 7s
Details
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 11s
Details
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 5s
Details
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m9s
Details
gate-check-v3 / gate-check (pull_request_target) Successful in 4s
Details
qa-review / approved (pull_request_target) Successful in 6s
Details
security-review / approved (pull_request_target) Successful in 9s
Details
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: comprehensive-testing, local-postgres-e2
Required
Details
sop-checklist / na-declarations (pull_request) N/A: (none)
Details
sop-checklist / all-items-acked (pull_request_target) Successful in 6s
Details
sop-tier-check / tier-check (pull_request_target) Successful in 7s
Details
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 2m28s
Details
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Details
Harness Replays / Harness Replays (pull_request) Successful in 17s
Details
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 13s
Details
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 2m22s
Details
E2E Chat / E2E Chat (pull_request) Failing after 8m20s
Details
audit-force-merge / audit (pull_request_target) Has been skipped
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
area/ci
CI/CD pipeline issues
 do-not-auto-merge
Opt out of autonomous merge-queue merging
 kind/infrastructure
Infrastructure-related issues
 merge-queue
Ready for serialized Gitea merge queue
 merge-queue-hold
Temporarily hold PR in merge queue
 platform/go
Go platform test issues
 release-blocker
Blocks the staging→main promotion / a release
 release-test
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
 wip
Work in progress — do not auto-merge
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent-dev-a agent-dev-b agent-pm agent-researcher agent-reviewer agent-reviewer-1 agent-reviewer-cr2 app-fe (Molecule AI · app-fe) app-lead (Molecule AI · app-lead) app-qa (Molecule AI · app-qa) claude-ceo-assistant claude-ci-reader claude-status-reaper core-be (Molecule AI · core-be) core-devops (Molecule AI · core-devops) core-fe (Molecule AI · core-fe) core-lead (Molecule AI · core-lead) core-offsec (Molecule AI · core-offsec) core-qa (Molecule AI · core-qa) core-security (Molecule AI · core-security) core-uiux (Molecule AI · core-uiux) cp-be (Molecule AI · cp-be) cp-lead (Molecule AI · cp-lead) cp-qa (Molecule AI · cp-qa) cp-security (Molecule AI · cp-security) cui (Zhanlin Cui) dev-lead (Molecule AI · dev-lead) devops-engineer documentation-specialist (Molecule AI · documentation-specialist) fullstack-engineer (Molecule AI · fullstack-engineer) godwin hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 infra-lead (Molecule AI · infra-lead) infra-runtime-be (Molecule AI · infra-runtime-be) infra-sre (Molecule AI · infra-sre) integration-tester (Molecule AI · integration-tester) mc-drift-bot molecule-code-reviewer plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot pypi-publisher (Molecule AI PyPI Publisher (RFC#596)) release-manager (Molecule AI · release-manager) sdk-dev (Molecule AI · sdk-dev) sdk-lead (Molecule AI · sdk-lead) sop-drift-bot sop-tier-bot (SOP Tier-Check Bot) technical-writer (Molecule AI · technical-writer) triage-operator (Molecule AI · triage-operator)
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#2035
Delete Branch "fix/ci-lint-suppression-1062"

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?