molecule-core/workspace-server/internal
Hongming Wang f39b595a9c fix(workspace files API): EIC parity for ListFiles + DeleteFile (closes #2999 PR-A)
## User-visible bug

Canvas Files tab returns "0 files / No config files yet" for every
SaaS workspace, every root (/configs, /home, /workspace, /plugins).
Reported by user (canvas screenshot, hongming.moleculesai.app,
Hongming Personal Brand Agent — claude-code, T4, online).

## Root cause

`ListFiles` (templates.go) was missing the SSH-via-EIC branch that
ReadFile (PR #2785) and WriteFile (PR #1702) already have. On SaaS,
dockerCli is nil → findContainer returns "" → falls through to
host-side resolveTemplateDir which only matches baked-in template
names. For a user-named workspace it matches nothing, so the handler
silently returns []fileEntry{}.

DeleteFile had the same gap — right-click delete (introduced in PR-C
of this issue) would silently no-op once #1 was fixed.

## Fix

1. Extracted shared EIC plumbing into `withEICTunnel` (closure-based,
   single SSOT for keypair → key push → tunnel → port-wait → cleanup).
   Refactored writeFileViaEIC + readFileViaEIC to use it. Added
   listFilesViaEIC + deleteFileViaEIC on the same scaffold. The
   `LogLevel=ERROR` shim from PR #2822 now lives in one
   `eicSSHSession.sshArgs()` helper instead of being duplicated per
   helper — the next time we need to tweak ssh options, one place.

2. Factored remote shell strings into pure functions
   (buildInstallShell / buildCatShell / buildRmShell / buildFindShell
   + parseFindOutput) so the wire shape can be pinned without booting
   a real EIC tunnel.

3. Refactored `resolveWorkspaceFilePath(runtime, root, relPath)` to
   honor `?root=`. New rule: `/configs` (or empty / unrecognized) →
   runtime managed-config dir via workspaceFilePathPrefix (preserves
   the v1 ReadFile/WriteFile behaviour where canvas's Config tab
   GETs/PUTs config.yaml without specifying a root and lands in the
   right per-runtime dir); `/home`, `/workspace`, `/plugins` →
   literal absolute path on the EC2 host. List/Read/Write/Delete now
   agree on what file a tree row points to — pre-fix List would say
   "/home contents" but Read/Write would route to /configs.

4. ListFiles + DeleteFile dispatch on instance_id != "" → EIC helper.
   Errors from the EIC path produce 500 (not silent fall-through to
   local-Docker, which would mask the failure as "0 files" — the
   exact user-visible symptom).

5. Added ?root= validation gate to WriteFile + DeleteFile so an
   out-of-allowlist root is rejected before the resolver runs.

## Test coverage

- TestResolveWorkspaceFilePath_RuntimeIndirection — pins the
  /configs → runtime prefix translation per-runtime (hermes,
  claude-code, langgraph, external, unknown). Catches the regression
  where a future edit accidentally drops the runtime indirection.

- TestResolveWorkspaceFilePath_LiteralRoots — pins /home,
  /workspace, /plugins as literal pass-through regardless of
  runtime. Catches the symmetric regression where the literal roots
  start getting rewritten to the runtime prefix (which would mean
  the FilesTab "/home" selector silently routes to /configs on
  hermes).

- TestResolveWorkspaceRootPath — directory-only translation used
  by listFilesViaEIC, same indirection rules.

- TestSSHArgs_HardenedFlags — pins the centralised ssh option set
  (LogLevel=ERROR + hardening). Catches drift in the
  one-place-where-ssh-flags-live.

- TestEicSSHSessionSingleSourceForSSHFlags — behaviour-based AST
  gate (per memory). Counts s.sshArgs() callers (must be ≥4 —
  list/read/write/delete) and asserts LogLevel=ERROR appears
  exactly once in the source. Fires if anyone copy-pastes a raw
  ssh args slice instead of going through the helper.

- TestBuildInstallShell / TestBuildCatShell / TestBuildRmShell /
  TestBuildFindShell — pure-function tests pinning the remote
  command shape. Catches regression like "rm -f silently becomes
  rm -rf" or "find loses node_modules pruning" without needing a
  real EC2.

- TestBuildFindShell_DepthForwarding — catches a regression where
  the helper hard-codes a depth instead of using the caller's value.

- TestParseFindOutput / TestParseFindOutput_EmptyInput — pin the
  TYPE|SIZE|REL parser. Empty-input case explicitly returns []
  not nil so the JSON wire shape stays a list.

- TestListFiles_EICDispatch_Success / Error — sqlmock-driven
  handler test. Verifies instance_id != "" routes to listFilesViaEIC
  and surfaces errors as 500 (does NOT silently fall through to
  local-Docker, which is the exact regression-mode of the original
  bug).

- TestListFiles_EICBranch_NotTakenForSelfHosted — back-compat
  guard: instance_id == "" must NOT enter the EIC branch (would
  break self-hosted operators).

- TestDeleteFile_EICDispatch_Success / Error — same shape for
  DeleteFile.

- TestListFiles_RootValidation / TestDeleteFile_RootValidation —
  ?root=/etc must 400 before any DB query or EIC call.

## Verification

- `go build ./...` clean
- `go test ./...` clean (full workspace-server suite)
- Will be live-verified against staging on hongming.moleculesai.app
  after merge: open Files tab → expect populated /home + /configs +
  /workspace listings (not "0 files"); right-click delete on
  /configs/old.yaml → expect file removed on the EC2 host.

## Three weakest spots (hostile self-review)

1. The LogLevel=ERROR drift gate counts source occurrences. A
   future refactor that intentionally moves the literal somewhere
   else (e.g. into a constant) would trigger a false positive. The
   gate's failure message points to the load-bearing constraint
   (must appear in sshArgs); operator can adjust.

2. `eicFileWriteTimeout` constant kept as an alias for back-compat
   with prior tests. Documented as intentional + safe to remove on
   the next pass.

3. The resolver tests pin the runtime → prefix map values
   (`/home/ubuntu/.hermes`, `/configs`, etc.). A future runtime
   addition that ships a new prefix needs the test updated. This
   is intentional — silent prefix changes orphan saved files, so a
   test failure on map edit IS the right signal.

## Follow-up (RFC #2312 subtask 2)

Long-term the right fix is to drop EIC entirely and HTTP-forward to
the workspace's own URL (RFC #2312). That's a substantially larger
refactor across 5 surfaces (chat upload, files, templates, plugins,
terminal) and out of scope for this bug-fix PR. Tracked separately
under that RFC.

Refs #2999.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-05 20:18:05 -07:00
..
artifacts chore: sync staging to main — 1188 commits, 5 conflicts resolved (#1743) 2026-04-23 18:30:18 +00:00
buildinfo feat(deploy): verify each tenant /buildinfo matches published SHA after redeploy 2026-04-30 10:55:08 -07:00
bundle fix(bundle): markFailed sets last_sample_error + AST gate 2026-05-04 21:08:08 -07:00
channels feat(channels): first-class Lark/Feishu support via schema-driven config 2026-04-24 11:51:15 -07:00
crypto
db fix(bundle): markFailed sets last_sample_error + AST gate 2026-05-04 21:08:08 -07:00
envx
events feat(events): typed EventType registry — single source of truth for WS event names (RFC #2945 PR-B) 2026-05-05 16:25:38 -07:00
handlers fix(workspace files API): EIC parity for ListFiles + DeleteFile (closes #2999 PR-A) 2026-05-05 20:18:05 -07:00
imagewatch feat(workspace-server): GHCR digest watcher closes runtime CD chain (#2114) 2026-04-26 13:36:26 -07:00
memory test(memory-v2-e2e): update expectChainQueryRoot for new name column 2026-05-05 19:10:18 -07:00
metrics feat(rfc): poll-mode chat upload — phase 3 GC sweep + observability 2026-05-05 05:00:13 -07:00
middleware fix(tenant-guard): allowlist /buildinfo so redeploy verifier can reach it 2026-04-30 12:54:51 -07:00
models refactor(models): consolidate per-runtime model defaults to SSOT (RFC #2873 iter 1) 2026-05-05 04:12:37 -07:00
orgtoken fix: F1085 rm scope concat + GH#756 ValidateToken terminal guard + CI test fixes 2026-04-24 07:16:54 +00:00
pendinguploads Merge pull request #2907 from Molecule-AI/feat/poll-mode-chat-upload-phase5a 2026-05-05 11:16:56 -07:00
plugins
provisioner feat(workspace-server): structured logging at provisioning boundaries 2026-05-05 12:30:11 -07:00
provlog feat(workspace-server): structured logging at provisioning boundaries 2026-05-05 12:30:11 -07:00
registry fix(orphan-sweeper): exclude runtime='external' from stale-token revoke 2026-05-03 00:49:37 -07:00
router feat(memory): redesign Memory tab for v2 plugin 2026-05-05 15:53:28 -07:00
scheduler feat(metrics): add molecule_phantom_busy_resets_total counter (#2865) 2026-05-05 04:45:24 -07:00
supervised
ws
wsauth perf(wsauth): in-process cache for platform_inbound_secret reads 2026-05-03 00:04:38 -07:00