molecule-ai/molecule-core
35
0
Fork 2
Code Issues 16 Pull Requests 2 Actions 49 Packages Projects Releases Wiki Activity

Merge pull request #1961 from Molecule-AI/feat/canvas-activitytab-missingkeys-tests

Browse Source 
fix(canvas/a11y+tests): aria-hidden backdrop, verifiedCPSession guard, useCanvasStore mock normalization
This commit is contained in:
Hongming Wang 2026-04-23 20:15:42 -07:00 committed by GitHub
commit 06a249bbb1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 70 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
canvas/src/components/MissingKeysModal.tsx
View File

@ -387,7 +387,6 @@ function AllKeysModal({
}) {
const [entries, setEntries] = useState<KeyEntry[]>([]);
const [globalError, setGlobalError] = useState<string | null>(null);
const firstInputRef = useRef<HTMLInputElement>(null);
useEffect(() => {
if (!open) return;
@ -403,12 +402,6 @@ function AllKeysModal({
setGlobalError(null);
}, [open, missingKeys]);
useEffect(() => {
if (!open) return;
const raf = requestAnimationFrame(() => firstInputRef.current?.focus());
return () => cancelAnimationFrame(raf);
}, [open]);
useEffect(() => {
if (!open) return;
const handler = (e: KeyboardEvent) => {
@ -471,6 +464,15 @@ function AllKeysModal({
onKeysAdded();
}, [entries, onKeysAdded]);
// Focus trap: auto-focus first input when modal opens
useEffect(() => {
if (!open) return;
const timer = requestAnimationFrame(() => {
document.getElementById("missing-keys-title")?.focus();
});
return () => cancelAnimationFrame(timer);
}, [open]);
if (!open) return null;
const allSaved = entries.length > 0 && entries.every((e) => e.saved);
@ -482,8 +484,8 @@ function AllKeysModal({
return (
<div className="fixed inset-0 z-50 flex items-center justify-center">
<div
aria-hidden="true"
className="absolute inset-0 bg-black/70 backdrop-blur-sm"
aria-hidden="true"
onClick={onCancel}
/>
@ -530,7 +532,7 @@ function AllKeysModal({
</div>
{entry.saved && (
<span className="text-[9px] text-emerald-400 bg-emerald-900/30 px-1.5 py-0.5 rounded flex items-center gap-1">
<svg width="8" height="8" viewBox="0 0 8 8" fill="none" aria-hidden="true">
<svg width="8" height="8" viewBox="0 0 8 8" fill="none">
<path d="M1.5 4L3.5 6L6.5 2" stroke="currentColor" strokeWidth="1.2" strokeLinecap="round" strokeLinejoin="round" />
</svg>
Saved
@ -545,7 +547,7 @@ function AllKeysModal({
onChange={(e) => updateEntry(index, { value: e.target.value.trimStart() })}
placeholder={entry.key.includes("API_KEY") ? "sk-..." : "Enter value"}
type="password"
ref={index === 0 ? firstInputRef : undefined}
autoFocus={index === 0}
onKeyDown={(e) => {
if (e.key === "Enter" && entry.value.trim()) {
handleSaveKey(index);

15
canvas/src/components/__tests__/ClaudeSettings.test.tsx
View File

@ -19,11 +19,18 @@ vi.mock("@/lib/api", () => ({
api: { get: vi.fn(), put: vi.fn(), patch: vi.fn(), post: vi.fn() },
}));
const mockCanvasState = {
restartWorkspace: vi.fn(),
updateNodeData: vi.fn(),
};
vi.mock("@/store/canvas", () => ({
useCanvasStore: vi.fn(() => ({
restartWorkspace: vi.fn(),
updateNodeData: vi.fn(),
})),
useCanvasStore: Object.assign(
vi.fn((selector: (s: Record<string, unknown>) => unknown) =>
selector(mockCanvasState as Record<string, unknown>)
),
{ getState: () => mockCanvasState }
),
}));
vi.mock("../tabs/config/secrets-section", () => ({

20
canvas/src/components/__tests__/ContextMenu.keyboard.test.tsx
View File

@ -48,20 +48,12 @@ const mockStore = {
nodes: [] as Array<{ id: string; data: { parentId: string | null } }>,
};
// useCanvasStore.getState() is called directly by ContextMenu to read `nodes`
// for parent-filtering (see ContextMenu.tsx childNodes computation). The mock
// must expose both the selector-calling function form AND the .getState()
// form so production code using either pattern doesn't hit "not a function".
// Factory body runs under vi.mock's hoist — cannot reference outer scope,
// so we build the mock function inside and reach `mockStore` via `globalThis`.
vi.mock("@/store/canvas", () => {
const fn = vi.fn((selector: (s: typeof mockStore) => unknown) =>
selector(mockStore),
);
return {
useCanvasStore: Object.assign(fn, { getState: () => mockStore }),
};
});
vi.mock("@/store/canvas", () => ({
useCanvasStore: Object.assign(
vi.fn((selector: (s: typeof mockStore) => unknown) => selector(mockStore)),
{ getState: () => mockStore }
),
}));
// ── Component under test — imported AFTER mocks ───────────────────────────────
import { ContextMenu } from "../ContextMenu";

2
canvas/src/components/__tests__/MissingKeysModal.a11y.test.tsx
View File

@ -85,7 +85,7 @@ describe("MissingKeysModal — WCAG 2.1 dialog accessibility", () => {
const backdrop = document.querySelector('[aria-hidden="true"]');
expect(backdrop).toBeTruthy();
// Verify the backdrop is the full-screen overlay (has bg-black/70)
expect(backdrop?.className).toContain("bg-black");
expect(backdrop?.className).toContain("bg-black/70");
});
it("decorative warning SVG in header has aria-hidden='true'", () => {

11
canvas/src/components/__tests__/tabs.a11y.test.tsx
View File

@ -26,9 +26,16 @@ vi.mock("@/lib/api", () => ({
},
}));
const mockCanvasTabState = {
setPanelTab: vi.fn(),
};
vi.mock("@/store/canvas", () => ({
useCanvasStore: vi.fn((selector: (s: Record<string, unknown>) => unknown) =>
selector({ setPanelTab: vi.fn() })
useCanvasStore: Object.assign(
vi.fn((selector: (s: Record<string, unknown>) => unknown) =>
selector(mockCanvasTabState as Record<string, unknown>)
),
{ getState: () => mockCanvasTabState }
),
summarizeWorkspaceCapabilities: vi.fn(() => ({ skills: [], tools: [] })),
}));

12
workspace-server/internal/handlers/discovery.go
View File

@ -348,6 +348,18 @@ func validateDiscoveryCaller(ctx context.Context, c *gin.Context, workspaceID st
tok := wsauth.BearerTokenFromHeader(c.GetHeader("Authorization"))
if tok == "" {
// Canvas hits this endpoint via session cookie, not bearer token.
// Add verifiedCPSession() as a fallback after the bearer check so
// SaaS canvas Peers tab doesn't 401. Self-hosted workspaces are
// unaffected — they have no CP session cookie.
ok, presented := middleware.VerifiedCPSession(c.GetHeader("Cookie"))
if ok {
return nil
}
if presented {
c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid session"})
return errors.New("invalid session")
}
c.JSON(http.StatusUnauthorized, gin.H{"error": "missing workspace auth token"})
return errors.New("missing token")
}

12
workspace-server/internal/middleware/session_auth.go
View File

@ -193,7 +193,7 @@ func verifiedCPSession(cookieHeader string) (valid, presented bool) {
client := &http.Client{Timeout: 3 * time.Second}
req, err := http.NewRequest("GET", verifyURL, nil)
if err != nil {
log.Printf("verifiedCPSession: build req: %v", err)
log.Printf("VerifiedCPSession: build req: %v", err)
return false, true
}
req.Header.Set("Cookie", cookieHeader)
@ -201,7 +201,7 @@ func verifiedCPSession(cookieHeader string) (valid, presented bool) {
resp, err := client.Do(req)
if err != nil {
log.Printf("verifiedCPSession: upstream: %v", err)
log.Printf("VerifiedCPSession: upstream: %v", err)
// NOTE: we deliberately do NOT cache transport failures.
// Caching them would mean a 3s CP blip locks out all users
// for the negative-TTL window. Next request retries.
@ -231,10 +231,10 @@ func verifiedCPSession(cookieHeader string) (valid, presented bool) {
return true, true
}
// VerifiedCPSession is the exported alias for handlers/discovery.go.
// Internal-only deployments (self-hosted / dev) where CP_UPSTREAM_URL
// is unset get (false, true) so the session path is skipped and the
// bearer token path runs as normal.
// VerifiedCPSession is the exported alias — callers in other packages
// (discovery.go, wsauth_middleware.go) use this name. Internal-only
// deployments (self-hosted/dev) where CP_UPSTREAM_URL is unset get
// (false, true) so the session path is skipped and bearer token auth runs.
func VerifiedCPSession(cookieHeader string) (valid, presented bool) {
return verifiedCPSession(cookieHeader)
}

24
workspace-server/internal/middleware/session_auth_test.go
View File

@ -37,7 +37,7 @@ func mockCPServer(t *testing.T, status int, body string) (*httptest.Server, *ato
func TestVerifiedCPSession_EmptyCookie(t *testing.T) {
resetSessionCache()
ok, presented := verifiedCPSession("")
ok, presented := VerifiedCPSession("")
if ok || presented {
t.Errorf("empty cookie should be (false, false); got (%v, %v)", ok, presented)
}
@ -47,7 +47,7 @@ func TestVerifiedCPSession_NoSlugConfigured(t *testing.T) {
resetSessionCache()
t.Setenv("CP_UPSTREAM_URL", "https://cp.test")
t.Setenv("MOLECULE_ORG_SLUG", "")
ok, presented := verifiedCPSession("session=foo")
ok, presented := VerifiedCPSession("session=foo")
// Without a slug we can't ask about tenant membership. Must
// refuse (false, false) — caller falls through to bearer tier.
if ok || presented {
@ -59,7 +59,7 @@ func TestVerifiedCPSession_NoCPConfigured(t *testing.T) {
resetSessionCache()
t.Setenv("CP_UPSTREAM_URL", "")
t.Setenv("MOLECULE_ORG_SLUG", "acme")
ok, presented := verifiedCPSession("session=foo")
ok, presented := VerifiedCPSession("session=foo")
// Self-hosted path: CP not configured, but cookie WAS presented.
// Presented=true lets the caller know not to fall through to
// bearer as if no credential arrived.
@ -74,7 +74,7 @@ func TestVerifiedCPSession_MemberTrue(t *testing.T) {
t.Setenv("CP_UPSTREAM_URL", srv.URL)
t.Setenv("MOLECULE_ORG_SLUG", "acme")
ok, presented := verifiedCPSession("session=valid")
ok, presented := VerifiedCPSession("session=valid")
if !ok || !presented {
t.Errorf("valid member should be (true, true); got (%v, %v)", ok, presented)
}
@ -83,7 +83,7 @@ func TestVerifiedCPSession_MemberTrue(t *testing.T) {
}
// Second call must be served from cache.
ok, _ = verifiedCPSession("session=valid")
ok, _ = VerifiedCPSession("session=valid")
if !ok {
t.Errorf("cached call should still be true")
}
@ -99,7 +99,7 @@ func TestVerifiedCPSession_MemberFalse(t *testing.T) {
t.Setenv("CP_UPSTREAM_URL", srv.URL)
t.Setenv("MOLECULE_ORG_SLUG", "acme")
ok, presented := verifiedCPSession("session=wrong-tenant")
ok, presented := VerifiedCPSession("session=wrong-tenant")
if ok || !presented {
t.Errorf("non-member should be (false, true); got (%v, %v)", ok, presented)
}
@ -107,7 +107,7 @@ func TestVerifiedCPSession_MemberFalse(t *testing.T) {
t.Fatalf("expected 1 upstream hit")
}
// Cached negatively.
_, _ = verifiedCPSession("session=wrong-tenant")
_, _ = VerifiedCPSession("session=wrong-tenant")
if hits.Load() != 1 {
t.Errorf("negative result should cache too; got %d hits", hits.Load())
}
@ -119,7 +119,7 @@ func TestVerifiedCPSession_Upstream401(t *testing.T) {
t.Setenv("CP_UPSTREAM_URL", srv.URL)
t.Setenv("MOLECULE_ORG_SLUG", "acme")
ok, presented := verifiedCPSession("session=expired")
ok, presented := VerifiedCPSession("session=expired")
if ok || !presented {
t.Errorf("401 upstream should be (false, true); got (%v, %v)", ok, presented)
}
@ -131,7 +131,7 @@ func TestVerifiedCPSession_MalformedJSON(t *testing.T) {
t.Setenv("CP_UPSTREAM_URL", srv.URL)
t.Setenv("MOLECULE_ORG_SLUG", "acme")
ok, presented := verifiedCPSession("session=broken")
ok, presented := VerifiedCPSession("session=broken")
if ok || !presented {
t.Errorf("malformed body should be (false, true); got (%v, %v)", ok, presented)
}
@ -143,7 +143,7 @@ func TestVerifiedCPSession_TransportErrorNotCached(t *testing.T) {
t.Setenv("CP_UPSTREAM_URL", "http://127.0.0.1:1")
t.Setenv("MOLECULE_ORG_SLUG", "acme")
ok, presented := verifiedCPSession("session=whatever")
ok, presented := VerifiedCPSession("session=whatever")
if ok || !presented {
t.Errorf("transport error should be (false, true); got (%v, %v)", ok, presented)
}
@ -178,12 +178,12 @@ func TestVerifiedCPSession_CrossTenantIsolation(t *testing.T) {
cookie := "session=shared-auth"
t.Setenv("MOLECULE_ORG_SLUG", "acme")
if ok, _ := verifiedCPSession(cookie); !ok {
if ok, _ := VerifiedCPSession(cookie); !ok {
t.Errorf("acme should say member=true")
}
t.Setenv("MOLECULE_ORG_SLUG", "bob")
if ok, _ := verifiedCPSession(cookie); ok {
if ok, _ := VerifiedCPSession(cookie); ok {
t.Errorf("bob tenant must NOT accept acme cookie despite same session bytes")
}
if len(reqs) != 2 {

2
workspace-server/internal/middleware/wsauth_middleware.go
View File

@ -174,7 +174,7 @@ func AdminAuth(database *sql.DB) gin.HandlerFunc {
// hosted / dev deploys without a CP fall through to the
// bearer-only path unchanged.
if cookieHeader := c.GetHeader("Cookie"); cookieHeader != "" {
if ok, _ := verifiedCPSession(cookieHeader); ok {
if ok, _ := VerifiedCPSession(cookieHeader); ok {
c.Next()
return
}