Skip to content

fix(terminals): enable the web terminal in the ephemeral-pod topology (daemon-host zellij gate) - #2083

Open
garciajrx wants to merge 2 commits into
mainfrom
feat/web-terminal-ephemeral-support
Open

fix(terminals): enable the web terminal in the ephemeral-pod topology (daemon-host zellij gate)#2083
garciajrx wants to merge 2 commits into
mainfrom
feat/web-terminal-ephemeral-support

Conversation

@garciajrx

@garciajrx garciajrx commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Problem

The web terminal shows "Zellij isn't installed on the daemon host" and is disabled on Cloud Cells, even though the daemon isn't where the shell runs.

TerminalsService probes which zellij on the daemon host and sets zellijAvailable from it. But the terminal spawns an executor with command: 'zellij.attach' — so under executor_command_template (ephemeral/templated execution) the PTY runs in the executor image, in a separate pod. The daemon-host probe is a leftover from the local_process model where the executor ran as a subprocess on the daemon host. The agor-daemon image has no zellij, so the gate always fails in the pod topology.

Change

terminals.ts — when execution is offloaded (execution.executor_command_template set), treat zellij as available (it's in the executor image); otherwise keep the daemon-host probe for local_process:

const executionOffloaded = Boolean(loadConfigSync().execution?.executor_command_template);
this.zellijAvailable = executionOffloaded || isZellijAvailable();

Pairs with

preset-io/agor-cloud — adds zellij to Dockerfile.executor (the image where the PTY actually runs). Both are needed together.

Not covered (follow-up)

The session-adoption logic (pgrep -f 'zellij attach …') also pgreps the daemon host, so reconnecting to a surviving session after a daemon restart still assumes daemon/executor co-location and won't find pod-based executors. Basic spawn -> PTY streaming works (the executor connects back to the daemon service); adoption/reconnect-after-restart needs a separate rework.

…ephemeral mode

The terminal spawns an executor with `zellij.attach`, so under
executor_command_template (ephemeral/templated execution) the PTY runs in the
executor image, not on the daemon host. The availability probe ran `which zellij`
on the daemon host and disabled terminals whenever the daemon image lacked zellij
— which is always in the Cloud pod topology. Treat zellij as available when
execution is offloaded; keep the daemon-host probe only for local_process.

Pairs with the executor image adding zellij (preset-io/agor-cloud). Not covered:
the pgrep-based session adoption still assumes daemon/executor co-location, so
reconnecting to a surviving session after a daemon restart needs a follow-up.
…ranch

The service constructor now reads loadConfigSync() to detect executor
offload, but terminals.test.ts did not mock it — under vitest an unmocked
named export is a hard error, so 14 of 16 tests failed at construction.

Mock loadConfigSync and add two tests: host-absent + not offloaded rejects
creation; host-absent + offloaded treats Zellij as available and spawns.
@garciajrx

Copy link
Copy Markdown
Contributor Author

Adversarial review flagged a ship-blocker: the constructor's new loadConfigSync() call wasn't mocked in terminals.test.ts, and under vitest an unmocked named export is a hard error — so 14 of 16 tests failed at new TerminalsService(...). Confirmed locally (PR branch: 14 failed | 2 passed; main: 16 passed).

Fixed in c8799c6:

  • Mock loadConfigSync in the @agor/core/config factory (restores the suite → 16/16).
  • Add two topology tests (the offload gate was previously untested): host Zellij absent + not offloaded ⇒ create() rejects with 'Zellij is not installed'; host absent + execution.executor_command_template set ⇒ Zellij treated as available and the executor is spawned. Suite now 18/18.

Rollout note from the review (L3): deploy the executor image with Zellij (agor-cloud #206) before the daemon. If the daemon rolls first, offloaded Cells will pass the guard and only surface a generic terminal:error after the ~10s readiness window instead of the old instant 'not installed' message. No hang — the error path exists — just a slower, less precise failure during that window.

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