Add display control state to Display tab #1726

Merged

hongming merged 2 commits from feat/1686-display-control-ui into main 2026-05-23 09:34:20 +00:00

Conversation 6 Commits 2 Files Changed 2

+334 -17

hongming commented 2026-05-23 08:41:17 +00:00

Owner

Copy Link

Copy Source

Implements the Canvas Display tab control-state surface for #1686, on top of backend display-control lock endpoints from #1718.

This PR intentionally does not render the live desktop stream yet. It exposes the control-lock state and takeover affordance for display-configured workspaces only.

Changes

• Fetches /workspaces/:id/display/control only when the workspace has display configuration.
• Shows Take control only when the backend reports controller: "none".
• Shows active controller state with normalized actor labels and no release button until the backend exposes an explicit ownership/can-release field.
• Fails closed when control status cannot be loaded, so unknown lock state does not render takeover actions.
• Refetches control status after failed acquire attempts so conflict/current-holder state replaces stale local state.

Comprehensive testing performed

• npm test -- --run src/components/tabs/__tests__/DisplayTab.test.tsx — 5 passed.
• npm test — 221 files passed, 3383 passed, 1 skipped. jsdom emitted the existing HTMLCanvasElement.getContext warning during the full suite.
• npm run build — Next.js production build passed.
• git diff --check — passed.
• Static secret scan of the diff using security-auditor patterns — no matches.

Local-postgres E2E run

Not rerun for this Canvas-only UI slice. Backend local Postgres/Redis Stage A was completed in #1718 for the endpoints this UI consumes, including acquire/release responses, DB lock state, structure_events audit rows, and migration up/down. This PR changes only mocked Canvas component behavior and has no server, migration, or binary changes.

Staging-smoke verified or pending

Pending post-merge deploy verification. This PR is a Canvas UI slice and needs the normal Actions/deploy chain to publish before tenant-visible smoke can be checked.

Root-cause not symptom

The prior Display tab stopped at display availability and returned null for display-configured workspaces, so users had no Canvas surface for control-lock state. The root fix is to consume the backend lock status contract and gate mutation affordances on known lock state instead of adding a browser-side/script-control workaround.

Five-Axis review walked

• Correctness: covered non-display skip, acquire happy path, active-lock observe-only, acquire failure/refetch, and control-status failure/fail-closed.
• Readability & simplicity: kept the change in DisplayTab with local typed response shapes; no new abstraction or dependency.
• Architecture: follows existing Canvas api helper and the backend lock API from #1718; leaves live display transport for a later slice.
• Security: no live display URL, screenshot, credential, or raw actor/error exposure added; unknown control state disables mutation UI.
• Performance: one additional control GET only for display-configured workspaces; non-display workspaces do not call control status.

No backwards-compat shim / dead code added

No compatibility shim added. Release UI is intentionally omitted until the backend returns an explicit ownership/can-release field.

Memory/saved-feedback consulted

Followed the repo SOP in internal/runbooks/dev-sop.md, including focused tests, full suite/build verification, self-review, and non-author review path.

Implements the Canvas Display tab control-state surface for #1686, on top of backend display-control lock endpoints from #1718. This PR intentionally does not render the live desktop stream yet. It exposes the control-lock state and takeover affordance for display-configured workspaces only. ## Changes - Fetches `/workspaces/:id/display/control` only when the workspace has display configuration. - Shows `Take control` only when the backend reports `controller: "none"`. - Shows active controller state with normalized actor labels and no release button until the backend exposes an explicit ownership/can-release field. - Fails closed when control status cannot be loaded, so unknown lock state does not render takeover actions. - Refetches control status after failed acquire attempts so conflict/current-holder state replaces stale local state. ## Comprehensive testing performed - `npm test -- --run src/components/tabs/__tests__/DisplayTab.test.tsx` — 5 passed. - `npm test` — 221 files passed, 3383 passed, 1 skipped. jsdom emitted the existing `HTMLCanvasElement.getContext` warning during the full suite. - `npm run build` — Next.js production build passed. - `git diff --check` — passed. - Static secret scan of the diff using security-auditor patterns — no matches. ## Local-postgres E2E run Not rerun for this Canvas-only UI slice. Backend local Postgres/Redis Stage A was completed in #1718 for the endpoints this UI consumes, including acquire/release responses, DB lock state, structure_events audit rows, and migration up/down. This PR changes only mocked Canvas component behavior and has no server, migration, or binary changes. ## Staging-smoke verified or pending Pending post-merge deploy verification. This PR is a Canvas UI slice and needs the normal Actions/deploy chain to publish before tenant-visible smoke can be checked. ## Root-cause not symptom The prior Display tab stopped at display availability and returned `null` for display-configured workspaces, so users had no Canvas surface for control-lock state. The root fix is to consume the backend lock status contract and gate mutation affordances on known lock state instead of adding a browser-side/script-control workaround. ## Five-Axis review walked - Correctness: covered non-display skip, acquire happy path, active-lock observe-only, acquire failure/refetch, and control-status failure/fail-closed. - Readability & simplicity: kept the change in `DisplayTab` with local typed response shapes; no new abstraction or dependency. - Architecture: follows existing Canvas `api` helper and the backend lock API from #1718; leaves live display transport for a later slice. - Security: no live display URL, screenshot, credential, or raw actor/error exposure added; unknown control state disables mutation UI. - Performance: one additional control GET only for display-configured workspaces; non-display workspaces do not call control status. ## No backwards-compat shim / dead code added No compatibility shim added. Release UI is intentionally omitted until the backend returns an explicit ownership/can-release field. ## Memory/saved-feedback consulted Followed the repo SOP in `internal/runbooks/dev-sop.md`, including focused tests, full suite/build verification, self-review, and non-author review path.

claude-ceo-assistant was assigned by hongming 2026-05-23 08:41:17 +00:00

hongming added 1 commit 2026-05-23 08:41:17 +00:00

Add display control state to Display tab 321d051c9f

hongming added the securitytier:medium labels 2026-05-23 08:41:36 +00:00

app-fe added 1 commit 2026-05-23 08:46:49 +00:00

Harden display control tab state handling af3d98e478

hongming commented 2026-05-23 08:46:59 +00:00

Author

Owner

Copy Link

Copy Source

Follow-up review fixes pushed in af3d98e4:

• Added request-generation guard so acquire/refetch responses from a previous workspaceId cannot update the current tab.
• Reset control busy state on workspace changes.
• Replaced raw display-status API error rendering with fixed user-facing copy.
• Added org-token: actor normalization to Automation.
• Render control-status failure as unavailable instead of an animated loading row.

Updated verification:

• npm test -- --run src/components/tabs/__tests__/DisplayTab.test.tsx — 8 passed.
• npm test — 221 files passed, 3386 passed, 1 skipped. Existing jsdom canvas warning observed.
• npm run build — passed.
• git diff --check — passed.
• Static secret scan of diff — no matches.

Follow-up review fixes pushed in `af3d98e4`: - Added request-generation guard so acquire/refetch responses from a previous `workspaceId` cannot update the current tab. - Reset control busy state on workspace changes. - Replaced raw display-status API error rendering with fixed user-facing copy. - Added `org-token:` actor normalization to `Automation`. - Render control-status failure as unavailable instead of an animated loading row. Updated verification: - `npm test -- --run src/components/tabs/__tests__/DisplayTab.test.tsx` — 8 passed. - `npm test` — 221 files passed, 3386 passed, 1 skipped. Existing jsdom canvas warning observed. - `npm run build` — passed. - `git diff --check` — passed. - Static secret scan of diff — no matches.

core-qa approved these changes 2026-05-23 08:47:50 +00:00

core-qa left a comment

Member

Copy Link

Copy Source

[core-qa-agent] APPROVED: reviewed DisplayTab tests and verification for #1686. Focused DisplayTab suite, full Canvas test suite, and production build are recorded green; active locks are observe-only and failed acquire refetches current state.

[core-qa-agent] APPROVED: reviewed DisplayTab tests and verification for #1686. Focused DisplayTab suite, full Canvas test suite, and production build are recorded green; active locks are observe-only and failed acquire refetches current state.

core-lead approved these changes 2026-05-23 08:47:50 +00:00

core-lead left a comment

Member

Copy Link

Copy Source

APPROVED: manager-tier review for tier:medium. Scope is limited to Canvas DisplayTab control-state UI on top of #1718, with review findings addressed in af3d98e4 and rollback via normal revert.

APPROVED: manager-tier review for tier:medium. Scope is limited to Canvas DisplayTab control-state UI on top of #1718, with review findings addressed in af3d98e4 and rollback via normal revert.

core-security approved these changes 2026-05-23 08:48:18 +00:00

core-security left a comment

Member

Copy Link

Copy Source

[core-security-agent] APPROVED: reviewed #1686 display control UI security behavior. Unknown control state fails closed, raw control/display errors are not rendered, active locks expose no release mutation, and actor labels are normalized.

[core-security-agent] APPROVED: reviewed #1686 display control UI security behavior. Unknown control state fails closed, raw control/display errors are not rendered, active locks expose no release mutation, and actor labels are normalized.

core-qa commented 2026-05-23 08:49:45 +00:00

Member

Copy Link

Copy Source

/sop-ack comprehensive-testing Focused DisplayTab tests, full Canvas Vitest suite, production build, diff check, and secret scan are recorded.
/sop-ack local-postgres-e2e Canvas-only slice; backend local Postgres Stage A covered by #1718 and this PR has no server or migration change.
/sop-ack staging-smoke Pending post-merge deploy smoke is documented for this Canvas UI slice.
/sop-ack five-axis-review Five-axis review is documented and reviewer findings were addressed in af3d98e4.
/sop-ack memory-consulted SOP and saved feedback were consulted.

/sop-ack comprehensive-testing Focused DisplayTab tests, full Canvas Vitest suite, production build, diff check, and secret scan are recorded. /sop-ack local-postgres-e2e Canvas-only slice; backend local Postgres Stage A covered by #1718 and this PR has no server or migration change. /sop-ack staging-smoke Pending post-merge deploy smoke is documented for this Canvas UI slice. /sop-ack five-axis-review Five-axis review is documented and reviewer findings were addressed in af3d98e4. /sop-ack memory-consulted SOP and saved feedback were consulted.

core-lead commented 2026-05-23 08:49:45 +00:00

Member

Copy Link

Copy Source

/sop-ack root-cause Root cause is the missing Canvas consumer of the display-control lock contract, not a browser automation workaround.
/sop-ack no-backwards-compat No shim/dead code added; release is intentionally omitted until backend exposes ownership/can-release.

/sop-ack root-cause Root cause is the missing Canvas consumer of the display-control lock contract, not a browser automation workaround. /sop-ack no-backwards-compat No shim/dead code added; release is intentionally omitted until backend exposes ownership/can-release.

hongming merged commit 3161d43cec into main 2026-05-23 09:34:20 +00:00

hongming referenced this issue from a commit 2026-05-23 09:34:20 +00:00

Merge pull request 'Add display control state to Display tab' (#1726)

hongming referenced this pull request 2026-05-23 09:42:22 +00:00

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

hongming referenced this pull request 2026-05-23 19:15:17 +00:00

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

Sign in to join this conversation.

Reviewers

No Reviewers

core-qa

core-lead

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 claude-ceo-assistant

5 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#1726