Skip to content

fix(workspace files API): GET ReadFile via SSH-EIC for SaaS workspaces (fixes 'No config.yaml found' on Config tab) - #2785

Merged
HongmingWang-Rabbit merged 1 commit into
stagingfrom
fix/readfile-eic-symmetry
May 4, 2026
Merged

HongmingWang-Rabbit merged 1 commit into
stagingfrom
fix/readfile-eic-symmetry

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

User-visible bug

Canvas Config tab returns 404 `No config.yaml found` for every SaaS workspace. Visible after PR #2781 added the explicit "no config" UX state — the underlying 404 was always there.

Root cause

`WriteFile` (templates.go:436) had an `instance_id != ""` branch that dispatched to `writeFileViaEIC` (SSH through EC2 Instance Connect). `ReadFile` (templates.go:362) skipped that branch and always tried local-Docker container + host-side template-dir fallback. Neither path sees what `WriteFile` writes onto the workspace EC2.

Net effect: every `PUT /workspaces//files/config.yaml` (200) followed by `GET /workspaces//files/config.yaml` returns 404. Save & Restart still works (the workspace container reads /configs directly via bind-mount), but Canvas can't render the Config tab.

This is the asymmetry I diagnosed earlier as task #302 / dropped from the synth-E2E gate in PR #2783. Now closing the loop at the source.

Fix

  1. New `readFileViaEIC` helper in `template_files_eic.go` mirrors `writeFileViaEIC` end-to-end (ephemeral keypair → EIC tunnel → SSH `sudo -n cat `). Returns `os.ErrNotExist` on missing file so the handler maps it cleanly to HTTP 404.
  2. `ReadFile` dispatch now mirrors `WriteFile`'s: when `instance_id` is set, dispatch through `readFileViaEIC`; otherwise fall through to local-Docker / template-dir path.
  3. `ReadFile`'s DB query expanded to also select `instance_id` + `runtime` (was just `name`).
  4. Three sqlmock-based tests updated to match the new column shape. The local-Docker fallback path stays green via `instance_id=""` rows.

Verification

  • `go build ./...` clean
  • Full handlers test suite green
  • Will be exercised against staging after merge: open Canvas Config tab on `hongming.moleculesai.app` → expect populated form, not 404

Follow-up (separate PR)

Synth-E2E 7c gate's GET-back marker assertion was dropped in #2783 because of this asymmetry. Now that the asymmetry is closed, restore the assertion so future regressions in the round-trip are caught by the canary.

🤖 Generated with Claude Code

Pre-fix WriteFile (templates.go:436) had an `instance_id != ""` branch
that dispatched to writeFileViaEIC (SSH through EC2 Instance Connect),
but ReadFile (templates.go:362) skipped that branch entirely. ReadFile
always tried `findContainer` (which only works for local-Docker
workspaces, not SaaS EC2-per-workspace ones) and fell through to
`resolveTemplateDir` (which returns the seed template, not the
persisted workspace state).

Net effect on production: every Canvas Config tab open against a
SaaS workspace returned 404 "No config.yaml found" because GET
couldn't see what PUT had written. Visible to users after PR #2781
("show-misconfigured-state") surfaced the 404 as an error UX.

Caught by the synth-E2E 7c gate's GET-back assertion, but
misdiagnosed as a "test bug" and the GET assertion was dropped in
PR #2783 (rather than fixed at the source). This PR closes the loop:

1. New `readFileViaEIC` helper in template_files_eic.go that mirrors
   writeFileViaEIC's SSH-via-EIC dance and runs `sudo -n cat <path>`.
   Returns os.ErrNotExist on missing file (cat exits 1 with empty
   stdout under `2>/dev/null`) so the handler maps it cleanly to 404.

2. ReadFile dispatch now mirrors WriteFile's: when `instance_id` is
   non-empty, use readFileViaEIC; otherwise fall through to the
   local-Docker / template-dir path.

3. ReadFile's DB query expanded to also select instance_id + runtime
   (was just name). Three sqlmock-based tests updated to match the
   new column shape; the existing local-Docker fallback path stays
   green by passing instance_id="" in the mock rows.

Follow-up (separate PR): the synth-E2E 7c gate should restore the
GET-back marker assertion now that the read/write paths are unified.
That'll also catch any future Files API regression in the round-trip.
This PR doesn't touch the gate to keep the scope tight.

Verification:
- go build ./... clean
- full handlers test suite green (0.4s for ReadFile subset; 5.8s
  full)
- The 3 ReadFile sqlmock tests still cover the local-Docker fallback
  (instance_id=""); SaaS EIC dispatch is covered by the upcoming
  re-enabled synth-E2E 7c GET assertion (deferred to follow-up)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant