molecule-ai/molecule-core
51
0
Fork 2
Code Issues 157 Pull Requests 158 Actions 12 Packages Projects Releases Wiki Activity

fix(display): allow browser sessions to take control #1832

Merged
agent-dev-b merged 1 commits from fix/display-control-browser-session into main 2026-05-25 04:24:38 +00:00
Conversation 2 Commits 1 Files Changed 5
+110 -5
hongming commented 2026-05-25 03:31:51 +00:00
Owner
Copy Link
Copy Source

Summary

  • set a stable hashed controller actor when AdminAuth accepts a verified control-plane browser session
  • allow display control acquisition for that verified browser session actor
  • let the Display tab reacquire a session URL for existing user locks after reload

Root-cause not symptom

The display takeover endpoint trusted AdminAuth for the browser request but displayControlActor had no stable actor for verified control-plane browser sessions, so the handler rejected real Canvas clicks with 403 even though viewing the Display tab was authenticated.

Comprehensive testing performed

  • Added middleware regression coverage that a verified CP browser session sets a hashed cp_session_actor and does not expose the raw cookie.
  • Added display-control handler regression coverage that the session actor can acquire a user control lock and receives a signed session URL.
  • Re-ran existing DisplayTab tests to cover take/release button behavior after the user-session actor change.

Local-postgres E2E run

N/A: this change does not add or migrate schema. The DB-facing code path is covered with sqlmock handler tests for the exact workspace_display_control_locks upsert behavior.

Staging-smoke verified or pending

Pending post-merge deploy verification. PR CI already includes E2E Staging SaaS and Canvas/staging checks for the head SHA; production canary /buildinfo plus a manual browser retry of Take control are the final release checks.

Five-Axis review walked

  • Correctness: browser session auth now produces an actor before display-control acquisition; regression tests cover middleware and handler boundaries.
  • Readability: actor derivation is a small helper and follows the existing session cache hashing pattern.
  • Architecture: keeps AdminAuth as the single CP session verification point and lets display control consume only a context value.
  • Security: actor id is tenant-scoped and hashed from the cookie only after CP membership verification; raw cookie is not stored or returned.
  • Performance: adds only one SHA-256 hash on the already-verified session path; CP verification caching remains unchanged.

No backwards-compat shim / dead code added

No compatibility shim or dead code added. The change completes the existing intended browser-session auth path for Display control.

Memory/saved-feedback consulted

Used current repo evidence and prior workspace-compute context from session continuity; no additional saved-memory file lookup was needed for this narrow production bug.

Tests

  • go test ./internal/handlers -run 'TestWorkspaceDisplayControl'
  • go test ./internal/middleware -run 'TestVerifiedCPSession|TestAdminAuth'
  • npx eslint src/components/tabs/DisplayTab.tsx --quiet
  • npm test -- --run src/components/tabs/__tests__/DisplayTab.test.tsx
  • git diff --check
## Summary - set a stable hashed controller actor when AdminAuth accepts a verified control-plane browser session - allow display control acquisition for that verified browser session actor - let the Display tab reacquire a session URL for existing user locks after reload ## Root-cause not symptom The display takeover endpoint trusted AdminAuth for the browser request but `displayControlActor` had no stable actor for verified control-plane browser sessions, so the handler rejected real Canvas clicks with 403 even though viewing the Display tab was authenticated. ## Comprehensive testing performed - Added middleware regression coverage that a verified CP browser session sets a hashed `cp_session_actor` and does not expose the raw cookie. - Added display-control handler regression coverage that the session actor can acquire a user control lock and receives a signed session URL. - Re-ran existing DisplayTab tests to cover take/release button behavior after the user-session actor change. ## Local-postgres E2E run N/A: this change does not add or migrate schema. The DB-facing code path is covered with sqlmock handler tests for the exact `workspace_display_control_locks` upsert behavior. ## Staging-smoke verified or pending Pending post-merge deploy verification. PR CI already includes E2E Staging SaaS and Canvas/staging checks for the head SHA; production canary `/buildinfo` plus a manual browser retry of Take control are the final release checks. ## Five-Axis review walked - Correctness: browser session auth now produces an actor before display-control acquisition; regression tests cover middleware and handler boundaries. - Readability: actor derivation is a small helper and follows the existing session cache hashing pattern. - Architecture: keeps AdminAuth as the single CP session verification point and lets display control consume only a context value. - Security: actor id is tenant-scoped and hashed from the cookie only after CP membership verification; raw cookie is not stored or returned. - Performance: adds only one SHA-256 hash on the already-verified session path; CP verification caching remains unchanged. ## No backwards-compat shim / dead code added No compatibility shim or dead code added. The change completes the existing intended browser-session auth path for Display control. ## Memory/saved-feedback consulted Used current repo evidence and prior workspace-compute context from session continuity; no additional saved-memory file lookup was needed for this narrow production bug. ## Tests - `go test ./internal/handlers -run 'TestWorkspaceDisplayControl'` - `go test ./internal/middleware -run 'TestVerifiedCPSession|TestAdminAuth'` - `npx eslint src/components/tabs/DisplayTab.tsx --quiet` - `npm test -- --run src/components/tabs/__tests__/DisplayTab.test.tsx` - `git diff --check`
hongming added 1 commit 2026-05-25 03:31:55 +00:00
fix(display): allow browser sessions to take control
ci-arm64-advisory / fast-checks (pull_request) Waiting to run
Details
CI / Canvas Deploy Reminder (pull_request) Blocked by required conditions
Details
Lint shellcheck (arm64 pilot) / shellcheck-arm64 (pilot) (pull_request) Successful in 15s
Details
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 27s
Details
CI / Python Lint & Test (pull_request) Successful in 12s
Details
CI / Detect changes (pull_request) Successful in 26s
Details
E2E API Smoke Test / detect-changes (pull_request) Successful in 24s
Details
E2E Chat / detect-changes (pull_request) Successful in 25s
Details
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (pull_request) Successful in 17s
Details
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 24s
Details
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (local) (pull_request) Successful in 1m36s
Details
E2E Staging SaaS (full lifecycle) / pr-validate (pull_request) Successful in 1m6s
Details
Handlers Postgres Integration / detect-changes (pull_request) Successful in 9s
Details
Harness Replays / detect-changes (pull_request) Successful in 7s
Details
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (pull_request) Successful in 10s
Details
Lint no tenant GITEA or GITHUB token write / Scan for repo-host token write into tenant workspace surface (pull_request) Successful in 10s
Details
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 12s
Details
qa-review / approved (pull_request) Successful in 7s
Details
security-review / approved (pull_request) Successful in 28s
Details
sop-checklist / na-declarations (pull_request) N/A: (none)
Details
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m5s
Details
E2E Staging SaaS (full lifecycle) / E2E Staging SaaS (pull_request) Successful in 5m37s
Details
gate-check-v3 / gate-check (pull_request) Waiting to run
Details
sop-checklist / all-items-acked (pull_request) Waiting to run
Details
sop-checklist / review-refire (pull_request) Waiting to run
Details
sop-tier-check / tier-check (pull_request) Waiting to run
Details
CI / Shellcheck (E2E scripts) (pull_request) Successful in 18s
Details
E2E Chat / E2E Chat (pull_request) Successful in 10s
Details
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 5s
Details
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 1m58s
Details
Harness Replays / Harness Replays (pull_request) Successful in 6s
Details
CI / Platform (Go) (pull_request) Successful in 5m57s
Details
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 4m30s
Details
CI / Canvas (Next.js) (pull_request) Successful in 9m8s
Details
CI / all-required (pull_request) Successful in 30m28s
Details
audit-force-merge / audit (pull_request) Successful in 7s
Details
9eefa5c474
core-qa approved these changes 2026-05-25 03:34:05 +00:00
core-qa left a comment
Member
Copy Link
Copy Source

QA review approved. Verified the changed DisplayTab flow and backend regression coverage for browser-session display control acquisition. Local evidence from author branch: DisplayTab vitest passed, eslint passed, handler display-control tests passed, middleware AdminAuth/session tests passed.

QA review approved. Verified the changed DisplayTab flow and backend regression coverage for browser-session display control acquisition. Local evidence from author branch: DisplayTab vitest passed, eslint passed, handler display-control tests passed, middleware AdminAuth/session tests passed.
core-qa approved these changes 2026-05-25 03:34:43 +00:00
core-qa left a comment
Member
Copy Link
Copy Source

QA review approved. Verified the changed DisplayTab flow and backend regression coverage for browser-session display control acquisition. Local evidence from author branch: DisplayTab vitest passed, eslint passed, handler display-control tests passed, middleware AdminAuth/session tests passed.

QA review approved. Verified the changed DisplayTab flow and backend regression coverage for browser-session display control acquisition. Local evidence from author branch: DisplayTab vitest passed, eslint passed, handler display-control tests passed, middleware AdminAuth/session tests passed.
core-security approved these changes 2026-05-25 03:35:07 +00:00
core-security left a comment
Member
Copy Link
Copy Source

Security review approved. Static review: no new dependencies, no secret-shaped literals in changed files, CP session actor is derived only after VerifiedCPSession succeeds, and the stored controller id is a tenant-scoped hash rather than the raw cookie.

Security review approved. Static review: no new dependencies, no secret-shaped literals in changed files, CP session actor is derived only after VerifiedCPSession succeeds, and the stored controller id is a tenant-scoped hash rather than the raw cookie.
agent-dev-b approved these changes 2026-05-25 03:47:05 +00:00
Dismissed
agent-dev-b left a comment
Member
Copy Link
Copy Source

LGTM

LGTM
agent-dev-b approved these changes 2026-05-25 04:03:21 +00:00
agent-dev-b left a comment
Member
Copy Link
Copy Source

5-axis review on (fix(display): allow browser sessions to take control):

Correctness: APPROVED. Stable hashed controller actor set on AdminAuth for verified browser sessions; Display tab reacquires session URLs for user locks on reload. Logic is sound.
Robustness: APPROVED. Stable hash prevents actor reuse attacks; session reacquisition is conditional on existing user locks.
Performance: APPROVED. No additional DB writes or queries introduced.
Security: APPROVED. Verification gated behind AdminAuth; controller actor is deterministically derived, not arbitrary. Browser session control surface is limited.
Compatibility: APPROVED. New behavior additive; existing sessions unaffected.

5-axis review on (fix(display): allow browser sessions to take control): Correctness: APPROVED. Stable hashed controller actor set on AdminAuth for verified browser sessions; Display tab reacquires session URLs for user locks on reload. Logic is sound. Robustness: APPROVED. Stable hash prevents actor reuse attacks; session reacquisition is conditional on existing user locks. Performance: APPROVED. No additional DB writes or queries introduced. Security: APPROVED. Verification gated behind AdminAuth; controller actor is deterministically derived, not arbitrary. Browser session control surface is limited. Compatibility: APPROVED. New behavior additive; existing sessions unaffected.
core-be commented 2026-05-25 04:07:56 +00:00
Member
Copy Link
Copy Source

/sop-ack local-postgres-e2e
/sop-ack staging-smoke
/sop-ack root-cause
/sop-ack five-axis-review
/sop-ack no-backwards-compat
/sop-ack memory-consulted

Engineering ack: DB write path is covered by sqlmock because this PR has no schema change; staging validation is represented by PR E2E plus post-merge canary; root cause is the missing browser-session actor at the display-control boundary; no compatibility shim or dead code was added.

/sop-ack local-postgres-e2e /sop-ack staging-smoke /sop-ack root-cause /sop-ack five-axis-review /sop-ack no-backwards-compat /sop-ack memory-consulted Engineering ack: DB write path is covered by sqlmock because this PR has no schema change; staging validation is represented by PR E2E plus post-merge canary; root cause is the missing browser-session actor at the display-control boundary; no compatibility shim or dead code was added.
core-qa commented 2026-05-25 04:07:56 +00:00
Member
Copy Link
Copy Source

/sop-ack comprehensive-testing

QA ack: regression coverage covers the browser-session Display takeover failure, handler lock acquisition, and DisplayTab take/release state.

/sop-ack comprehensive-testing QA ack: regression coverage covers the browser-session Display takeover failure, handler lock acquisition, and DisplayTab take/release state.
hongming added the tier:low label 2026-05-25 04:09:42 +00:00
agent-dev-b reviewed 2026-05-25 04:16:26 +00:00
agent-dev-b left a comment
Member
Copy Link
Copy Source

Please review

Please review
agent-dev-b reviewed 2026-05-25 04:16:27 +00:00
agent-dev-b left a comment
Member
Copy Link
Copy Source

Please review

Please review
agent-dev-b merged commit 4c86f047c7 into main 2026-05-25 04:24:38 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
core-qa
core-security
agent-dev-b
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 tier:low
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
5 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#1832
Delete Branch "fix/display-control-browser-session"

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?