Commit Graph

2 Commits

Author SHA1 Message Date
Hongming Wang
3f11df031c fix: six UX bugs (peers auth, scroll, chat tabs, config persist, + visibility)
Six bugs reported from a live session — all shippable in one commit:

1. Peers tab 401 on local Docker. The /registry/:id/peers endpoint
   demands a workspace-scoped bearer token (validateDiscoveryCaller)
   which the canvas session doesn't hold. Added the same Tier-1b
   dev-mode fail-open hatch that AdminAuth and WorkspaceAuth already
   use — gated by MOLECULE_ENV=development + empty ADMIN_TOKEN, so
   SaaS production stays strict. Exported IsDevModeFailOpen from the
   middleware package for the handler layer to reuse.

2. Org Templates list unscrollable. OrgTemplatesSection was rendered
   in the TemplatePalette footer — a div without overflow — so when
   it expanded to 15+ entries the list extended past the viewport
   with no scroll. Moved it to the top of the flex-1 overflow-y-auto
   container. Tall lists now scroll naturally.

3. Chat tab: "My Chat" and "Agent Comms" rendered stacked instead
   of switching. HTML `hidden` attribute was being overridden by
   Tailwind's `flex` class (display: flex beats the attribute),
   so both tabpanels rendered concurrently. Swapped to a conditional
   Tailwind `hidden`/`flex` class so the inactive panel is
   display:none with proper CSS specificity.

4. Hermes Config form never persists. handleSave wrote config.yaml
   but name / tier / runtime / model all live on the workspace row
   (or the dedicated /workspaces/:id/model endpoint) — the form
   edited in-memory, the request returned 200, the next reload
   wiped everything back. Hermes + external runtimes manage their
   own config inside the container anyway, so writing config.yaml
   is a no-op for them; skip it. Always diff and PATCH the DB-backed
   fields that actually changed.

5. Channels "+ Connect" dropdown empty on first open. ChannelsTab's
   load() used Promise.all with a silent catch — if EITHER the
   channels or adapters fetch failed, both setters were skipped
   with no error visible. Switched to Promise.allSettled so each
   endpoint settles independently, and the adapters failure now
   surfaces via the top-level error state.

6. Plugin registry always "No plugins in registry". Same silent
   catch pattern in SkillsTab.tsx — load errors for /plugins,
   /plugins/sources, and /workspaces/:id/plugins swallowed without
   logging. Replaced the empty catches with console.warn so future
   failures are at least visible in devtools.

Tests: 923 passing (unchanged). Go handler tests pass. Server
rebuilt and running with the peers-auth + collapsed-persistence
fixes (pid 15875).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-23 20:18:30 -07:00
Hongming Wang
47d3ef5b9e refactor(middleware): extract dev-mode fail-open predicate
AdminAuth and WorkspaceAuth both carried the same 5-line
`ADMIN_TOKEN == "" && MOLECULE_ENV in {development, dev}` check. If a
third middleware ever needs the hatch — or if "dev mode" semantics
change (new env name, allowlist, runtime flag) — the previous shape
made N places to keep in sync and N places a security reviewer has to
audit.

This commit factors the predicate into a single `isDevModeFailOpen()`
helper in `internal/middleware/devmode.go`. Each call site becomes

    if isDevModeFailOpen() { c.Next(); return }

`devmode.go` carries the full rationale (why the hatch exists, why
it's safe for SaaS) so call sites don't need to restate it.

### Also

- Moved the dev-mode env-value set to a package-level `devModeEnvValues`
  map so adding aliases is one line. Matches the existing convention
  (`handlers/admin_test_token.go`) of treating `MOLECULE_ENV != "production"`
  as dev — but stays explicit about which values opt IN rather than
  blanket-accepting everything non-prod.
- Added case-insensitive compare + trim on the env value so operators
  don't have to remember exact casing.
- New `devmode_test.go` unit-tests the predicate directly: 6 cases
  covering happy path, both opt-out signals (ADMIN_TOKEN, production
  mode), short alias, case-insensitive + whitespace tolerance, and an
  explicit negative-space sweep of arbitrary non-dev values
  ("staging", "preview", "test", "devel", "") to lock in that typos
  don't silently enable the hatch.

Existing AdminAuth/WorkspaceAuth integration tests still exercise the
helper indirectly via HTTP — they pass unchanged, confirming the
behaviour is preserved.

### No behavioural change

Before and after this commit, `go test -race ./internal/middleware/`
reports identical results. Zero production surface change — this is a
pure refactor, but it collapses the dev-mode seam from two inline
blocks into one named predicate, which is the shape future
contributors (and security reviewers) can follow.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-23 14:55:34 -07:00