Add display control lock endpoints #1718

Merged

hongming merged 1 commits from feat/1686-display-control-lock into main 2026-05-23 08:08:08 +00:00

Conversation 2 Commits 1 Files Changed 6

+717

hongming commented 2026-05-23 07:44:28 +00:00

Owner

Copy Link

Copy Source

Implements the next backend-only slice for #1686: a volatile display control lock contract for future native display takeover.

What changed

• Adds workspace_display_control_locks with TTL-backed per-workspace ownership state.
• Adds admin-gated endpoints:
  • GET /workspaces/:id/display/control
  • POST /workspaces/:id/display/control/acquire
  • POST /workspaces/:id/display/control/release
• Uses atomic acquire semantics: active locks can only be renewed by the same attributable actor; otherwise callers receive 409 with the current holder.
• Keeps browser session callers observe-only for mutation until middleware exposes a stable per-user/session id. Mutation requires ADMIN_TOKEN bearer or org-token actor; force release requires ADMIN_TOKEN.
• Validates acquire only works for workspaces with display enabled, so non-display instances cannot get control rows.
• Emits best-effort structure_events audit rows for acquire/release started/completed/failed.

Comprehensive testing performed

• go test ./internal/handlers -run 'TestWorkspaceDisplayControl'
• go test ./internal/handlers ./internal/router ./internal/db
• go test ./...
• go build ./cmd/server

Local-postgres E2E run

Ran Stage A with isolated temp services:

• Postgres: local initdb/pg_ctl on port 55432
• Redis: local redis-server on port 56379
• Server: DATABASE_URL=postgres://dev@localhost:55432/molecule?sslmode=disable REDIS_URL=redis://localhost:56379 PORT=18086 BIND_ADDR=127.0.0.1 ADMIN_TOKEN=stage-a-admin-token MOLECULE_ENV=development go run ./cmd/server

Evidence:

• Fresh server boot applied 20260523120000_workspace_display_control_locks.up.sql with all repo migrations.
• \d workspace_display_control_locks confirmed PK, FK cascade, controller check, controlled_by length check, and expires index.
• Endpoint probe on a display-enabled workspace:
  • GET /display/control -> {"controller":"none"}
  • POST /display/control/acquire -> {"controller":"user","controlled_by":"admin-token","expires_at":"..."}
  • DB row showed active = true
  • POST /display/control/release -> {"controller":"none"}
  • DB row count returned 0
• structure_events showed display.control.acquire.started, display.control.acquire.completed, display.control.release.started, and display.control.release.completed for the workspace.
• Up/down migration pair was run on the populated temp DB; down dropped the table and up recreated it cleanly.

Docker was not running locally. The server attempted Docker provisioner auto-detection and orphan-sweeper checks returned Docker connection errors without stopping any containers.

Staging-smoke verified or pending

Pending merge/deploy. This PR adds backend coordination endpoints only; no Canvas takeover UI is wired in this slice.

Root-cause not symptom

The display roadmap needs native computer-level takeover coordination before exposing a live screen/control UI. This adds the coordination primitive without leaking DCV/session details or changing GET /workspaces/:id/display availability semantics.

Five-Axis review walked

• Correctness: addressed reviewer findings by blocking coarse session mutation, checking RowsAffected on release, validating display-enabled workspace before acquire, and using a separate none DTO so zero timestamps are not serialized.
• Readability/simplicity: kept implementation in one display-control handler file with explicit DTOs and no Canvas changes.
• Architecture: stores volatile lock state outside workspaces.compute; existing display status remains backward-compatible.
• Security: routes stay under AdminAuth; SQL is parameterized; browser sessions cannot mutate until stable identity exists; force release requires actual ADMIN_TOKEN bearer.
• Performance: one indexed row per workspace; acquire is one atomic upsert plus conflict lookup only on contention.

No backwards-compat shim / dead code added

No compatibility shim. Existing display status endpoint behavior is unchanged. No dead code added.

Memory/saved-feedback consulted

Used the repo SOP from internal/runbooks/dev-sop.md and followed the Stage A / review checklist. No prior saved rollout memory was needed for this narrow backend slice.

Refs #1686

Implements the next backend-only slice for #1686: a volatile display control lock contract for future native display takeover. ## What changed - Adds `workspace_display_control_locks` with TTL-backed per-workspace ownership state. - Adds admin-gated endpoints: - `GET /workspaces/:id/display/control` - `POST /workspaces/:id/display/control/acquire` - `POST /workspaces/:id/display/control/release` - Uses atomic acquire semantics: active locks can only be renewed by the same attributable actor; otherwise callers receive `409` with the current holder. - Keeps browser session callers observe-only for mutation until middleware exposes a stable per-user/session id. Mutation requires `ADMIN_TOKEN` bearer or org-token actor; force release requires `ADMIN_TOKEN`. - Validates acquire only works for workspaces with display enabled, so non-display instances cannot get control rows. - Emits best-effort `structure_events` audit rows for acquire/release started/completed/failed. ## Comprehensive testing performed - `go test ./internal/handlers -run 'TestWorkspaceDisplayControl'` - `go test ./internal/handlers ./internal/router ./internal/db` - `go test ./...` - `go build ./cmd/server` ## Local-postgres E2E run Ran Stage A with isolated temp services: - Postgres: local `initdb`/`pg_ctl` on port `55432` - Redis: local `redis-server` on port `56379` - Server: `DATABASE_URL=postgres://dev@localhost:55432/molecule?sslmode=disable REDIS_URL=redis://localhost:56379 PORT=18086 BIND_ADDR=127.0.0.1 ADMIN_TOKEN=stage-a-admin-token MOLECULE_ENV=development go run ./cmd/server` Evidence: - Fresh server boot applied `20260523120000_workspace_display_control_locks.up.sql` with all repo migrations. - `\d workspace_display_control_locks` confirmed PK, FK cascade, controller check, controlled_by length check, and expires index. - Endpoint probe on a display-enabled workspace: - `GET /display/control` -> `{"controller":"none"}` - `POST /display/control/acquire` -> `{"controller":"user","controlled_by":"admin-token","expires_at":"..."}` - DB row showed `active = true` - `POST /display/control/release` -> `{"controller":"none"}` - DB row count returned `0` - `structure_events` showed `display.control.acquire.started`, `display.control.acquire.completed`, `display.control.release.started`, and `display.control.release.completed` for the workspace. - Up/down migration pair was run on the populated temp DB; down dropped the table and up recreated it cleanly. Docker was not running locally. The server attempted Docker provisioner auto-detection and orphan-sweeper checks returned Docker connection errors without stopping any containers. ## Staging-smoke verified or pending Pending merge/deploy. This PR adds backend coordination endpoints only; no Canvas takeover UI is wired in this slice. ## Root-cause not symptom The display roadmap needs native computer-level takeover coordination before exposing a live screen/control UI. This adds the coordination primitive without leaking DCV/session details or changing `GET /workspaces/:id/display` availability semantics. ## Five-Axis review walked - Correctness: addressed reviewer findings by blocking coarse session mutation, checking `RowsAffected` on release, validating display-enabled workspace before acquire, and using a separate `none` DTO so zero timestamps are not serialized. - Readability/simplicity: kept implementation in one display-control handler file with explicit DTOs and no Canvas changes. - Architecture: stores volatile lock state outside `workspaces.compute`; existing display status remains backward-compatible. - Security: routes stay under `AdminAuth`; SQL is parameterized; browser sessions cannot mutate until stable identity exists; force release requires actual `ADMIN_TOKEN` bearer. - Performance: one indexed row per workspace; acquire is one atomic upsert plus conflict lookup only on contention. ## No backwards-compat shim / dead code added No compatibility shim. Existing display status endpoint behavior is unchanged. No dead code added. ## Memory/saved-feedback consulted Used the repo SOP from `internal/runbooks/dev-sop.md` and followed the Stage A / review checklist. No prior saved rollout memory was needed for this narrow backend slice. Refs #1686

hongming added 1 commit 2026-05-23 07:44:29 +00:00

Add display control lock endpoints 08ca29fdad

hongming added the securitytier:medium labels 2026-05-23 07:44:57 +00:00

hongming requested review from core-qa 2026-05-23 07:44:58 +00:00

hongming requested review from core-security 2026-05-23 07:44:58 +00:00

core-qa approved these changes 2026-05-23 07:45:38 +00:00

core-qa left a comment

Member

Copy Link

Copy Source

QA review: approved. Local handler/router/db tests and full workspace-server suite are green; Stage A endpoint probe against temp Postgres/Redis verified acquire/release and audit rows.

QA review: approved. Local handler/router/db tests and full workspace-server suite are green; Stage A endpoint probe against temp Postgres/Redis verified acquire/release and audit rows.

core-security approved these changes 2026-05-23 07:45:39 +00:00

core-security left a comment

Member

Copy Link

Copy Source

Security review: approved. New control mutation endpoints stay under AdminAuth; SQL is parameterized; browser sessions are observe-only until stable identity exists; force release requires ADMIN_TOKEN.

Security review: approved. New control mutation endpoints stay under AdminAuth; SQL is parameterized; browser sessions are observe-only until stable identity exists; force release requires ADMIN_TOKEN.

hongming merged commit 665f0a2405 into main 2026-05-23 08:08:08 +00:00

hongming deleted branch feat/1686-display-control-lock 2026-05-23 08:08:08 +00:00

hongming referenced this issue from a commit 2026-05-23 08:08:10 +00:00

Merge pull request 'Add display control lock endpoints' (#1718) from feat/1686-display-control-lock into main

hongming referenced this pull request 2026-05-23 08:08:24 +00:00

RFC: Per-workspace EC2 configurability — instance type, volume size, OS, GPU #1686

hongming referenced this pull request 2026-05-23 08:41:17 +00:00

Add display control state to Display tab #1726

core-qa referenced this pull request 2026-05-23 08:49:45 +00:00

Add display control state to Display tab #1726

hongming referenced this pull request 2026-05-23 09:02:32 +00:00

[main-red] molecule-ai/molecule-core: def18f28fa #1638

Sign in to join this conversation.

Reviewers

No Reviewers

core-qa

core-security

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 security tier:medium

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

3 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#1718