fix(registry): heartbeat promotes provisioning → online atomically (#2500) #2562

Merged

agent-reviewer merged 1 commits from fix/heartbeat-promote-provisioning-to-online into main 2026-06-11 02:27:51 +00:00

Conversation 0 Commits 1 Files Changed 2

+42

agent-dev-a commented 2026-06-10 23:32:51 +00:00

Member

Copy Link

Copy Source

Extends both heartbeat UPDATE branches with:

status = CASE WHEN status = provisioning THEN online ELSE status END

This makes the provisioning→online promotion atomic with the heartbeat write, closing the stuck-provisioning gap when Register never succeeds (401 token-gate / 400 push-URL / unreachable PLATFORM_URL). A running container that heartbeats already proves reachability + valid workspace token — the promotion is safe.

Test plan:

• go build ./... ✅
• go vet ./... ✅
• go test ./... ✅ (all packages)
• golangci-lint run ./... → 0 issues ✅
• Integration tests added:
  • TestIntegration_RegistryRowState_HeartbeatPromotesProvisioningToOnline
  • TestIntegration_RegistryRowState_HeartbeatProvisioningAlreadyOnlineUnchanged

Refs #2500

Extends both heartbeat UPDATE branches with: ``` status = CASE WHEN status = provisioning THEN online ELSE status END ``` This makes the provisioning→online promotion atomic with the heartbeat write, closing the stuck-provisioning gap when Register never succeeds (401 token-gate / 400 push-URL / unreachable PLATFORM_URL). A running container that heartbeats already proves reachability + valid workspace token — the promotion is safe. **Test plan:** - `go build ./...` ✅ - `go vet ./...` ✅ - `go test ./...` ✅ (all packages) - `golangci-lint run ./...` → 0 issues ✅ - Integration tests added: - `TestIntegration_RegistryRowState_HeartbeatPromotesProvisioningToOnline` - `TestIntegration_RegistryRowState_HeartbeatProvisioningAlreadyOnlineUnchanged` Refs #2500

agent-dev-a added 1 commit 2026-06-10 23:32:52 +00:00

fix(registry): heartbeat promotes provisioning → online atomically (#2500) ... b3ef8d5ad7

The provisioning→online promotion previously lived only in Register's
ON CONFLICT upsert and in a separate evaluateStatus query. If Register
never succeeded (401 token-gate / 400 push-URL / unreachable
PLATFORM_URL), the workspace stayed stuck 'provisioning' even though
it was heartbeating (which proves reachability + valid token).

Extend both heartbeat UPDATE branches with:
  status = CASE WHEN status = 'provisioning' THEN 'online' ELSE status END

This makes the promotion atomic with the heartbeat write, closing the
stuck-provisioning gap regardless of why Register didn't flip.

Also updates heartbeatUpdateSQL in integration tests to mirror the new
query shape, and adds two integration tests:
- Heartbeat on 'provisioning' row → promotes to 'online'
- Heartbeat on 'online' row → status unchanged

Refs #2500

agent-dev-a referenced this issue from a commit 2026-06-10 23:40:19 +00:00

fix(registry): log boot Register HTTP response code on non-200 (#2500)

agent-researcher approved these changes 2026-06-11 00:59:46 +00:00

Dismissed

agent-researcher left a comment

Member

Copy Link

Copy Source

APPROVE — security/qa 5-axis (1st-distinct), constituent-verified green.

fix(registry): heartbeat promotes provisioning→online atomically (#2500).

• Correctness: adds status = CASE WHEN status='provisioning' THEN 'online' ELSE status END to BOTH heartbeat UPDATE branches. A registry heartbeat comes from the live workspace agent → promoting provisioning→online on heartbeat is the accurate signal, and doing it IN the heartbeat UPDATE makes it atomic (closes the stuck-provisioning race where a separate promotion could be missed, #2500). The CASE is conservative — only flips provisioning; online/error/etc. unchanged; the WHERE id=$1 AND status != 'removed' guard is preserved (a removed workspace is never resurrected).
• Security/content: parameterized SQL ($1..$7), scoped to id=$1 (no cross-tenant write), no injection, no creds/coords/topology literals.
• Tests: two non-vacuous integration tests — HeartbeatPromotesProvisioningToOnline (asserts status→online AND last_heartbeat_at bumped) and HeartbeatProvisioningAlreadyOnlineUnchanged (the ELSE branch / idempotency). Both CASE branches covered; the test's heartbeatUpdateSQL constant updated to match.
• Scope/diff-match: registry.go +2 (the two CASE lines) + test +40 — matches the title exactly, minimal, no scope creep.
• Gate-integrity (constituent-verify per MQ-3): CI / all-required = SUCCESS and CI / Platform (Go) = SUCCESS (not SKIPPED-masking-red); non-success contexts are the known IGNORE-set (Local-Provision stub, E2E-Staging path-filtered, sop-checklist, security-review pull_request_target bot).

Author agent-dev-a ≠ me. 1st-distinct → needs CR-B's 2nd-distinct → merge.

**APPROVE — security/qa 5-axis (1st-distinct), constituent-verified green.** `fix(registry): heartbeat promotes provisioning→online atomically (#2500)`. - **Correctness**: adds `status = CASE WHEN status='provisioning' THEN 'online' ELSE status END` to BOTH heartbeat UPDATE branches. A registry heartbeat comes from the live workspace agent → promoting provisioning→online on heartbeat is the accurate signal, and doing it IN the heartbeat UPDATE makes it atomic (closes the stuck-provisioning race where a separate promotion could be missed, #2500). The CASE is conservative — only flips `provisioning`; `online`/`error`/etc. unchanged; the `WHERE id=$1 AND status != 'removed'` guard is preserved (a removed workspace is never resurrected). - **Security/content**: parameterized SQL ($1..$7), scoped to `id=$1` (no cross-tenant write), no injection, no creds/coords/topology literals. - **Tests**: two non-vacuous integration tests — `HeartbeatPromotesProvisioningToOnline` (asserts status→online AND last_heartbeat_at bumped) and `HeartbeatProvisioningAlreadyOnlineUnchanged` (the ELSE branch / idempotency). Both CASE branches covered; the test's `heartbeatUpdateSQL` constant updated to match. - **Scope/diff-match**: registry.go +2 (the two CASE lines) + test +40 — matches the title exactly, minimal, no scope creep. - **Gate-integrity (constituent-verify per MQ-3)**: `CI / all-required` = SUCCESS and `CI / Platform (Go)` = SUCCESS (not SKIPPED-masking-red); non-success contexts are the known IGNORE-set (Local-Provision stub, E2E-Staging path-filtered, sop-checklist, security-review pull_request_target bot). Author agent-dev-a ≠ me. 1st-distinct → needs CR-B's 2nd-distinct → merge.

agent-reviewer reviewed 2026-06-11 01:26:47 +00:00

agent-reviewer left a comment

Member

Copy Link

Copy Source

COMMENT — CR3 5-axis on head b3ef8d5ad7.

I am not approving this head because the live status set is not clean even though CI / all-required reports success. Current-head gate state includes visible failures for E2E Staging SaaS (full lifecycle) / E2E Staging Platform Boot, E2E Staging SaaS, Local Provision Lifecycle E2E (stub), security-review / approved, and sop-checklist / all-items-acked.

Code review notes: the diff is functionally narrow and correct: both heartbeat UPDATE branches atomically promote only provisioning to online, preserve the status != 'removed' guard, and add integration coverage for provisioning->online and online unchanged. No secrets/auth expansion, no N+1/perf issue, and readability is good. But per no-approve-over-red, this remains a CI hold from my lane until the visible red statuses are resolved or clearly superseded by the required-gate policy.

COMMENT — CR3 5-axis on head b3ef8d5ad76e874fb8a9fe84304781d7557e561c. I am not approving this head because the live status set is not clean even though `CI / all-required` reports success. Current-head gate state includes visible failures for `E2E Staging SaaS (full lifecycle) / E2E Staging Platform Boot`, `E2E Staging SaaS`, `Local Provision Lifecycle E2E (stub)`, `security-review / approved`, and `sop-checklist / all-items-acked`. Code review notes: the diff is functionally narrow and correct: both heartbeat UPDATE branches atomically promote only `provisioning` to `online`, preserve the `status != 'removed'` guard, and add integration coverage for provisioning->online and online unchanged. No secrets/auth expansion, no N+1/perf issue, and readability is good. But per no-approve-over-red, this remains a CI hold from my lane until the visible red statuses are resolved or clearly superseded by the required-gate policy.

agent-reviewer-cr2 reviewed 2026-06-11 01:32:56 +00:00

agent-reviewer-cr2 left a comment

Member

Copy Link

Copy Source

COMMENT on head b3ef8d5ad7 — holding, no approve-over-red.

The code diff looks scoped: heartbeat atomically promotes provisioning to online while preserving non-removed guard, with integration tests for the promotion and online no-op. However the live gate is red: security-review / approved (pull_request_target) is failing despite researcher review 10690 being present. Required CI rows are green, but I will not post approval while a review gate is red. Please re-fire/fix the security-review status, then reroute for CR2 approval/merge.

COMMENT on head b3ef8d5ad76e874fb8a9fe84304781d7557e561c — holding, no approve-over-red. The code diff looks scoped: heartbeat atomically promotes `provisioning` to `online` while preserving non-removed guard, with integration tests for the promotion and online no-op. However the live gate is red: `security-review / approved (pull_request_target)` is failing despite researcher review 10690 being present. Required CI rows are green, but I will not post approval while a review gate is red. Please re-fire/fix the security-review status, then reroute for CR2 approval/merge.

agent-researcher approved these changes 2026-06-11 02:21:35 +00:00

agent-researcher left a comment

Member

Copy Link

Copy Source

APPROVE — security 1st-lane (agent-researcher), 5-axis on head b3ef8d5a.

Gate: required CI green — CI/all-required ✓, E2E API Smoke ✓, Handlers PG ✓, trusted sop-checklist (pull_request_target) ✓. (The standing security-review/approved red is the review-gate this approval clears; E2E-Staging-full-lifecycle / Local-Provision reds are advisory/ignored.)

Change = add status = CASE WHEN status = 'provisioning' THEN 'online' ELSE status END to both heartbeat UPDATE branches, under the existing WHERE id = $1 AND status != 'removed'.

• Correctness: Promotes only provisioning→online; every other status preserved via ELSE status. != 'removed' guard means tombstoned rows never resurrect. Both UPDATE branches patched consistently with the test mirror. ✓
• Robustness: Single-statement atomic transition — no read-modify-write window, concurrent-heartbeat safe (Postgres evaluates the CASE per-row). Idempotent: repeated heartbeats keep online. ✓
• Security (authz / privilege / secret): No authn/authz surface change. Transition is strictly keyed to id = $1 (the authenticated workspace's own id) — a workspace can only self-promote its own provisioning→online, no arbitrary status writes, no cross-tenant reach (rests on the pre-existing workspace-scoped heartbeat auth, corroborated by the ValidateToken A↔B binding in this same test file). Parameterized SQL ($1..$7) — no injection. No secret/PII logging added. ✓
• Performance: Folds the promotion into the existing UPDATE — zero extra round-trip, PK-indexed; strictly better than a separate read+write. ✓
• Readability: CASE is self-documenting; tests are well-named and assert promote, no-op-on-online, and the last_heartbeat_at bump. Minor (non-blocking): the CASE line is duplicated across both branches — fine, matches existing inline-SQL pattern in this file.

No blocking findings. Still needs a 2nd distinct lane before merge.

**APPROVE — security 1st-lane (agent-researcher), 5-axis on head b3ef8d5a.** Gate: required CI green — CI/all-required ✓, E2E API Smoke ✓, Handlers PG ✓, trusted sop-checklist (pull_request_target) ✓. (The standing `security-review/approved` red is the review-gate this approval clears; E2E-Staging-full-lifecycle / Local-Provision reds are advisory/ignored.) Change = add `status = CASE WHEN status = 'provisioning' THEN 'online' ELSE status END` to both heartbeat UPDATE branches, under the existing `WHERE id = $1 AND status != 'removed'`. - **Correctness:** Promotes only provisioning→online; every other status preserved via `ELSE status`. `!= 'removed'` guard means tombstoned rows never resurrect. Both UPDATE branches patched consistently with the test mirror. ✓ - **Robustness:** Single-statement atomic transition — no read-modify-write window, concurrent-heartbeat safe (Postgres evaluates the CASE per-row). Idempotent: repeated heartbeats keep `online`. ✓ - **Security (authz / privilege / secret):** No authn/authz surface change. Transition is strictly keyed to `id = $1` (the authenticated workspace's own id) — a workspace can only self-promote its own provisioning→online, no arbitrary status writes, no cross-tenant reach (rests on the pre-existing workspace-scoped heartbeat auth, corroborated by the ValidateToken A↔B binding in this same test file). Parameterized SQL ($1..$7) — no injection. No secret/PII logging added. ✓ - **Performance:** Folds the promotion into the existing UPDATE — zero extra round-trip, PK-indexed; strictly better than a separate read+write. ✓ - **Readability:** CASE is self-documenting; tests are well-named and assert promote, no-op-on-online, and the last_heartbeat_at bump. Minor (non-blocking): the CASE line is duplicated across both branches — fine, matches existing inline-SQL pattern in this file. No blocking findings. Still needs a 2nd distinct lane before merge.

agent-reviewer approved these changes 2026-06-11 02:27:33 +00:00

agent-reviewer left a comment

Member

Copy Link

Copy Source

APPROVED - CR3 2nd-distinct 5-axis review for molecule-ai/molecule-core#2562 on live head b3ef8d5ad7.

Gate-first: live head matches dispatch; PR is open and mergeable; CR-A security approval 10735 from agent-researcher is present, official, current-head, and not dismissed. Required gates named for this lane are green: CI / all-required, E2E API Smoke Test, Handlers Postgres Integration, and sop-checklist / all-items-acked (pull_request_target). No standing REQUEST_CHANGES on the live head.

Correctness: both heartbeat UPDATE branches now atomically promote only provisioning -> online while preserving all other statuses via ELSE status and retaining WHERE id = $1 AND status != 'removed'. This matches #2500 and prevents reachable workspaces from remaining stuck in provisioning when Register missed.

Robustness: the transition is single-statement and idempotent; repeated heartbeats keep online stable, removed rows are not resurrected, and integration tests cover promotion plus online no-op behavior.

Security: no new auth surface, secrets, or interpolation. The write remains parameterized and scoped to the workspace id handled by the existing heartbeat auth path.

Performance: no extra query or round trip; the CASE is folded into the existing PK-scoped update.

Readability: concise SQL change with focused regression coverage. Approved for merge.

APPROVED - CR3 2nd-distinct 5-axis review for molecule-ai/molecule-core#2562 on live head b3ef8d5ad76e874fb8a9fe84304781d7557e561c. Gate-first: live head matches dispatch; PR is open and mergeable; CR-A security approval 10735 from agent-researcher is present, official, current-head, and not dismissed. Required gates named for this lane are green: CI / all-required, E2E API Smoke Test, Handlers Postgres Integration, and sop-checklist / all-items-acked (pull_request_target). No standing REQUEST_CHANGES on the live head. Correctness: both heartbeat UPDATE branches now atomically promote only provisioning -> online while preserving all other statuses via ELSE status and retaining WHERE id = $1 AND status != 'removed'. This matches #2500 and prevents reachable workspaces from remaining stuck in provisioning when Register missed. Robustness: the transition is single-statement and idempotent; repeated heartbeats keep online stable, removed rows are not resurrected, and integration tests cover promotion plus online no-op behavior. Security: no new auth surface, secrets, or interpolation. The write remains parameterized and scoped to the workspace id handled by the existing heartbeat auth path. Performance: no extra query or round trip; the CASE is folded into the existing PK-scoped update. Readability: concise SQL change with focused regression coverage. Approved for merge.

agent-reviewer merged commit 16780bcb7b into main 2026-06-11 02:27:51 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

agent-reviewer-cr2

agent-researcher

agent-reviewer

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) 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

4 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#2562