Replace Node UI token bootstrap with dashboard sessions#1428
Conversation
…wups Refine dashboard login follow-up boundaries
| // widening the api.ts surface. | ||
| async function post<T = unknown>(path: string, body: unknown): Promise<T> { | ||
| const res = await fetch(path, { | ||
| const res = await apiFetch(path, { |
There was a problem hiding this comment.
🔴 Bug: Ontology install helper breaks the new daemonFetch path contract
What's wrong
The PR introduces a typed same-origin fetch boundary for dashboard-session requests, but this changed call passes a plain string into that boundary. That weakens the contract and is likely to break TypeScript compilation for ontology install.
Example
post('/api/sub-graph/create', ...) is valid at runtime, but inside post the literal has been widened to string, so apiFetch(path, ...) no longer satisfies the new DaemonPath contract.
Suggested direction
Keep the typed same-origin boundary by using DaemonPath at the helper boundary or validating with daemonPath.
For Agents
In packages/node-ui/src/ui/lib/ontologyInstall.ts, change the helper to accept DaemonPath or wrap the argument with daemonPath(path) before calling apiFetch. Preserve the same-origin path guard and type-check this file.
| try { | ||
| const { ensureDashboardCredentials } = await import('../daemon/dashboard-credentials.js'); | ||
| return await ensureDashboardCredentials(); | ||
| } catch (err: any) { |
There was a problem hiding this comment.
🟡 Issue: Init dashboard credential failure path is untested
What's wrong
The new init flow intentionally catches dashboard credential creation failures so dkg init can continue, but the added init tests only cover success and auth-disabled paths. A regression that rethrows here would break first-run setup when a stale or corrupt credential file exists, and the current init test suite would not catch it.
Example
A focused regression test can create an invalid dashboard.credentials.json, call ensureDashboardCredentialsForInit(true), and assert it resolves null plus logs the reset-password guidance instead of throwing.
Suggested direction
Add an init-specific regression test for invalid or unreadable dashboard credentials so the best-effort contract is pinned.
For Agents
Add coverage in packages/cli/test/init-dashboard-credentials.test.ts for the catch path in ensureDashboardCredentialsForInit: seed an invalid credential file under DKG_HOME, call the helper with auth enabled, assert no throw, null result, and warning guidance. Preserve existing create/rerun/auth-disabled behavior.
…ry-smoke Harden dashboard auth boundaries and static UI smoke
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } catch (err) { | ||
| if (!res.headersSent) { | ||
| res.writeHead(500, { 'Content-Type': 'text/plain' }); | ||
| res.end(err instanceof Error ? err.stack : String(err)); |
| // Run CLI-owned setup side effects after the node config home is known and | ||
| // before wallets / daemon startup. Best-effort so adapter setup remains | ||
| // recoverable and daemon/UI-driven setup can omit terminal-only side effects. | ||
| const afterConfigBootstrap = deps.afterConfigBootstrap ?? deps.ensureDashboardCredentials; |
There was a problem hiding this comment.
🟡 Issue: Post-config setup hook handling is duplicated across setup orchestrators
What's wrong
The PR adds the same optional lifecycle hook, deprecated dashboard-specific fallback, and try/catch best-effort execution to multiple setup flows. That spreads one policy across packages and makes the general adapter setup APIs carry dashboard credential terminology they otherwise tried to avoid.
Example
Changing the post-config hook failure policy or removing the deprecated ensureDashboardCredentials fallback currently requires coordinated edits in OpenClaw, Hermes, and MCP setup code.
Suggested direction
Centralize the hook selection and best-effort execution behind a shared helper or setup lifecycle abstraction. Keep adapter-facing deps generic and contain the deprecated dashboard-specific compatibility shim at the CLI boundary where possible.
Confidence note
This assumes the three setup paths are expected to keep identical lifecycle semantics; if the divergence is intentional, the shared helper should make those differences explicit.
For Agents
Look at packages/adapter-openclaw/src/setup.ts, packages/adapter-hermes/src/setup.ts, and packages/cli/src/mcp-setup.ts. Introduce one shared post-config setup hook runner or a common setup lifecycle dependency type. Preserve dry-run skipping, best-effort error handling, and legacy JS fallback behavior, then update the existing setup hook tests to target the shared contract once.
…loseout Close dashboard auth boundary follow-up
| * omits it. Required setup work should stay explicit instead of being hidden | ||
| * behind this optional hook. | ||
| */ | ||
| afterConfigBootstrap?: PostConfigSetupHook; |
There was a problem hiding this comment.
🟡 Issue: A generic post-config hook turns setup into a side-effect injection point
What's wrong
The hook is intentionally broad, optional, and best-effort, while the actual behavior is specific dashboard credential provisioning. This leaks CLI-owned side effects into adapter setup through an abstraction that does not describe what it does. It also duplicates the same fallback and warning pattern in multiple setup orchestrators, which is the kind of hook-based growth that tends to become permanent setup debt.
Example
Dashboard credential provisioning is the only new use case, but the adapter API now exposes a generic post-config lifecycle hook plus a deprecated alias. The same hook fallback and catch/log behavior is repeated across Hermes, OpenClaw, and MCP setup.
Suggested direction
Make the dependency explicit to the behavior being added, or centralize a typed setup-step runner reused by Hermes/OpenClaw/MCP. Avoid a generic lifecycle hook that can hide arbitrary work and duplicated error handling.
Confidence note
This is a structural concern from the diff shape; the exact public compatibility constraints for the adapter setup dependency objects may need maintainer confirmation.
For Agents
Review the setup dependency surfaces in Hermes, OpenClaw, and MCP. Preserve the ordering: config bootstrap before wallet creation/daemon start, dry-run skip, and best-effort credential provisioning. Replace the generic hook with an explicit named setup concern or a shared post-config step runner so future setup side effects do not accumulate behind an untyped lifecycle slot.
There was a problem hiding this comment.
🟡 Issue: Avoid spreading a generic post-config hook across setup flows
What's wrong
The new afterConfigBootstrap abstraction is generic, optional, duplicated, and currently only exists to run dashboard credential provisioning. It hides a feature-specific side effect behind a lifecycle hook and repeats the same control-flow policy in multiple setup implementations.
Example
Adding another CLI-owned setup side effect after config bootstrap would require deciding whether it goes through the generic hook, then auditing Hermes, OpenClaw, and MCP to keep the same ordering/dry-run/best-effort behavior.
Suggested direction
Prefer an explicit, shared setup lifecycle helper or an explicit dashboardCredentialSetup dependency with one runner for dry-run/deprecated-alias/best-effort handling. That would keep adapter setup order readable without giving every setup path an open-ended side-effect slot.
For Agents
Inspect packages/adapter-hermes/src/setup.ts, packages/adapter-openclaw/src/setup.ts, packages/cli/src/mcp-setup.ts, and the CLI setup wrappers. Preserve dry-run skipping, wallet-before-daemon ordering, and best-effort dashboard credential creation. Replace the repeated generic hook with a shared setup lifecycle helper or an explicit dashboard-credential dependency composed in the CLI-owned setup layer.
…identity-followup Tighten daemon route auth identity handoff
3 participants
Summary
window.__DKG_TOKEN__bootstrap from production/uiand Vite dev serving, so the static SPA no longer receives a reusable daemon bearer token.HttpOnlySameSite=Strictcookies, loopback bootstrap, token exchange, logout/status/CSRF endpoints, and CSRF checks for unsafe dashboard API requests.Related
Files changed
packages/cli/src/daemon/dashboard-session.tspackages/cli/src/auth.tspackages/cli/src/daemon/lifecycle.tspackages/node-ui/src/api.ts,packages/node-ui/vite.config.tspackages/node-ui/src/ui/**apiFetch, removes EventSource query tokens, and avoids sending dashboard credentials to external RPC URLs.packages/node-ui/e2e/**,packages/node-ui/test/**,packages/cli/test/**scripts/devnet-test-node-ui-smoke.sh.github/workflows/ci.ymlbuild:uicontract check for no__DKG_TOKEN__and valid referenced assets.agent-docs/**Test plan
pnpm --filter @origintrail-official/dkg exec vitest run --config vitest.unit.config.ts test/auth.test.ts test/dashboard-session.test.tspassed: 44 tests.pnpm --filter @origintrail-official/dkg-node-ui exec vitest run test/api-routes.test.ts test/ui-compat.test.ts test/ui-api-pure.test.ts test/pca-api.test.ts test/web3-clients.test.ts test/use-current-agent.test.ts test/openclaw-bridge.test.ts --no-file-parallelismpassed: 296 passed, 38 skipped.pnpm --filter @origintrail-official/dkg-node-ui exec tsc --noEmitpassed.pnpm --filter @origintrail-official/dkg exec tsc --noEmitpassed.pnpm --filter @origintrail-official/dkg-node-ui run build:uipassed; existing chunk-size / mock-subgraph export warnings only.packages/node-ui/dist-ui/index.htmlpassed: no__DKG_TOKEN__, referenced assets present.pnpm --filter @origintrail-official/dkg run buildpassed.scripts/devnet-test-node-ui-smoke.shagainst a one-node WSL devnet passed:PASS=11 FAIL=0.PLAYWRIGHT_DEVNET_TIMEOUT_MS=300000 PW_HEADLESS=1 pnpm exec playwright test e2e/specs/auth-session.spec.tspassed on the standard four-node real-node devnet: 1 passed.