Skip to content

Refactor devcontainer and console features for improved stability - #6

Merged
ThePlenkov merged 134 commits into
gascity-extra:mainfrom
ThePlenkov:main
Jul 13, 2026
Merged

Refactor devcontainer and console features for improved stability#6
ThePlenkov merged 134 commits into
gascity-extra:mainfrom
ThePlenkov:main

Conversation

@ThePlenkov

@ThePlenkov ThePlenkov commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Summary by cubic

Refactors the devcontainer and hardens the console’s tmux/WebSocket and proxy paths for a safer, more reliable local dev setup. Playwright now uses install-deps (with optional auto browser install), and readiness probes verify both node-pty and tmux with clearer messages.

  • New Features

    • Devcontainer: Playwright via install-deps with optional auto-install; Homebrew installs oven-sh/bun/bun and gastownhall/gascity/gascity; new bob feature; .devcontainer lockfile; HUSKY=0; non‑fatal cursor; adds VS Code MCP seed.
    • Console: tmux bridge enforces same‑origin WebSocket upgrades and an exact /api/pty path; readiness uses loadPty plus isTmuxAvailable; /api/pty probe reports missing node-pty vs missing tmux.
    • Marketplace/types: SemVer compare now prefers stable vs prerelease with correct core-version precedence; TOML parsing preserves # inside quotes; @gascity/client exports ESM index.mjs with .d.mts types.
  • Bug Fixes

    • Security: /gc/* proxy validates target origin (SSRF guard); tmux WebSocket rejects cross‑site upgrades.
    • Stability: tmux bridge handles Buffer, ArrayBuffer, and fragmented Buffer[] frames; offline error handling is cycle‑safe; Playwright feature simplified and more robust; scenarios can be skipped via SKIP_SCENARIOS.
    • UX: dialogs and drawers restore focus and trap Tab; orders list uses native buttons; charts and tooltips render reliably.
    • Tooling: adds .codacy.yaml scoped suppressions and .gitattributes for line endings.

Written for commit f703118. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Added a tmux provider readiness badge to the Sessions view.
    • Added DevContainer features for Bob Shell, Bun, Devin, and Playwright (with full feature docs).
  • Bug Fixes
    • Improved city initialization error reporting and dialog accessibility (focus restore, Escape handling, ARIA/keyboard behavior).
    • Hardened and refined supervisor connectivity/health handling, including PTY probe and WebSocket upgrade readiness checks.
  • Tests
    • Expanded and stabilized E2E coverage with more deterministic navigation plus backend reachability gating; added/updated unit tests for supervisor URL resolution, TOML parsing/version selection, and tmux PTY message handling.

kilo-code-bot Bot and others added 30 commits June 25, 2026 09:47
…e 24 + Bun (#4)

* fix(devcontainer): drop broken upstream gc/homebrew features, use Node 24 + Bun

The devcontainer's gascity feature relied on the devcontainer spec's
dependsOn-with-options-override, which is not supported. The homebrew
feature never received packages=["gastownhall/gascity/gascity"], so gc
was never installed and the gascity feature's install.sh bailed out
with 'gc command not found' under set -e.

This is a TS SDK/console repo (package.json name 'gascity.ts', dev
script runs nx run @gascity/console:dev) and has no runtime need for
the upstream gc/dolt/bd CLI. Fix A applied:

- Drop ./features/gascity and ./features/homebrew entirely
- Add ghcr.io/devcontainers/features/node:1 v24 for Node >= 24
- Install Bun 1.3.14 (per package.json engines) via postCreateCommand
- Make ./features/cursor non-fatal (placeholder installer)

* fix(devcontainer): use official Bun feature, drop invalid installGlibc

Address Codacy review on PR #4:
- Replace manual Bun install (bashrc-only, non-idempotent) with the
  ghcr.io/devcontainers/features/bun:1 feature so Bun is on PATH for
  both bash and zsh.
- Remove the unrecognized installGlibc option from the Node feature
  (Ubuntu base already provides glibc).

---------

Co-authored-by: Toast (gastown) <Toast@gastown.local>
…e 24 + Bun

The devcontainer's gascity feature relied on the devcontainer spec's
dependsOn-with-options-override, which is not supported. The homebrew
feature never received packages=["gastownhall/gascity/gascity"], so gc
was never installed and the gascity feature's install.sh bailed out
with 'gc command not found' under set -e.

This is a TS SDK/console repo (package.json name 'gascity.ts', dev
script runs nx run @gascity/console:dev) and has no runtime need for
the upstream gc/dolt/bd CLI. Fix A applied:

- Drop ./features/gascity and ./features/homebrew entirely
- Add ghcr.io/devcontainers/features/node:1 v24 for Node >= 24
- Install Bun 1.3.14 (per package.json engines) via postCreateCommand
- Make ./features/cursor non-fatal (placeholder installer)
The upstream Devin CLI installer (https://cli.devin.ai/install.sh) ends with
'"$VERSION_DIR/bin/$COMPILED_BIN_NAME" setup', which invokes the
interactive  wizard for authentication and MCP configuration.
In a non-interactive devcontainer / Codespaces build there is no controlling
TTY on stdin, so the wizard hangs or aborts under set -e and the entire
feature fails, blocking the container from starting.

Fix:
- Default installMethod=script: download the installer, strip the trailing
   invocation via sed before executing. Smoke test
  is wrapped in  so warnings/missing-creds never abort the build.
- New installMethod=binary: skip the upstream installer entirely; fetch the
  platform tarball straight from the static.devin.ai manifest, verify SHA256,
  extract, and install. Honors  for pinning to a concrete release tag.
- Feature bumped to 1.1.0; option  added with enum [script, binary].
- Guard the final cp against missing /home/agent/.local/bin/devin and emit a clear
  success line.

Beads: a5fcecd8 (this), references escalation 88928bd8.
- install.sh: parse manifest with jq when available, fall back to grep/sed
- install.sh: fail loudly if sed fails to strip 'devin setup' instead of
  silently risking the interactive wizard hanging the build
- devcontainer.json: append Bun PATH to both .bashrc and .zshrc since
  common-utils installs Zsh
- bump devin feature to 1.2.0
- Fix jq lookup path to query under .platforms (cubic)
- Fail closed when manifest has no checksum (cubic, coderabbit)
- Tighten guard regex to match quoted $COMPILED_BIN_NAME setup
- Convert remaining [ ... ] to [[ ... ]] (SonarCloud)

Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
Mirror packages/@gascity/console/src/styles.css from Lovable project
64c74931-cffc-4e6e-9f25-234df4ac774e (commit bc4d35d) so local dev matches
the published preview.

- Add Tailwind v4 source() + @source '../src'
- Add tw-animate-css, Inter 400/500/600, Geist Mono Variable imports
- Add @custom-variant dark + @theme inline aliases
- Add OKLCH :root and .dark palettes (incl. --live amber, --destructive)
- Add base layer: border/font/selection, hairline hr, live-pulse keyframes,
  .live-dot indicator
- Add xterm viewport/screen overrides

All required deps are already declared in package.json.
…tart migration

## Summary

Mirrors `packages/@gascity/console/src/styles.css` from the Lovable project
(`64c74931-cffc-4e6e-9f25-234df4ac774e`, commit `bc4d35d`) so local dev matches
the published preview, and lands the rest of the in-progress console migration
that was sitting uncommitted on `gt/maple/devin-fix`.

## Why styles differed

Local `packages/@gascity/console/src/styles.css` contained only:

```css
@import "tailwindcss";
```

The Lovable version is a full design system (OKLCH palette, `:root` + `.dark`
tokens, `@theme inline` aliases, `@custom-variant dark`, font/animate imports,
base layer, `live-pulse` keyframes, xterm overrides). All required deps are
already declared in `package.json`, so no install is needed.

## What's in this commit

- **styles.css** — full design system from Lovable (Tailwind v4 + `@source`,
  `tw-animate-css`, Inter 400/500/600, Geist Mono Variable, dark variant, OKLCH
  tokens incl. `--live` amber and `--destructive`, base layer rules, xterm
  overrides)
- **TanStack Start entrypoints** — new `src/client.tsx`, `src/router.tsx`,
  `src/routeTree.gen.ts`, `src/vite-env.d.ts`, and `vite.config.ts` replace
  the old `src/routes/index.ts` router export
- **Route + component updates** — `__root.tsx`, `beads`, `endpoints`,
  `formulas.$name`, `mail`, `orders`, `packs`, `gc.$.ts`, `AppShell`,
  `SessionTerminal`, `SessionsList`, `SlingComposer`, `gc.functions.ts`
- **Build/dep config** — root `package.json`, `@gascity/client/package.json`,
  `@gascity/console/package.json`, `bun.lock`, `tsconfig.json`,
  `.devcontainer/devcontainer.json`; old `tsdown.config.ts` and
  `vitest.config.ts` removed
- All untracked devcontainer/local tooling (`.devcontainer/mcp.json`,
  `.devcontainer/features/playwright/`, `kilo.json`) intentionally excluded

## Stats

28 files changed, +1236 / −529

## Verification

- `pnpm install` (or `bun install`) — should be a no-op since deps already match
- `pnpm --filter @gascity/console dev` — Vite should boot and serve the new
  TanStack Start app with the Lovable styles applied
- Compare against https://lovable.dev/projects/64c74931-cffc-4e6e-9f25-234df4ac774e

🤖 Generated with [GitHub Copilot](https://github.com/copilot)
The dev-container e2e run was reporting 91 failures (out of 92 tests) and
exiting non-zero. Investigation split the failures into two distinct bugs:

1. **Hydration timing in the UI suite.** The console is server-rendered by
   TanStack Start; the first click / keypress after `page.goto` lands on
   static SSR HTML and is silently ignored because React hasn't attached
   the onClick handlers or the global keydown listener yet. Tests that
   relied on this (Cmd+K, supervisor toggle, 'v' shortcut) failed
   non-deterministically depending on how warm the dev server was.

   - Added `waitForHydration(page)` to `e2e/lib/actions.ts` (1.5s wait;
     safely above worst-case cold hydration on this dev container).
   - Switched every UI spec's `beforeEach` to use `baseURL` from the
     Playwright config (was hard-coded `http://localhost:8080`) and call
     the hydration helper after `domcontentLoaded`.
   - Switched keystrokes from `Meta+` (no-op on Linux Chromium) to
     `Control+` for portability.
   - The supervisor toggle can no longer be clicked to close the panel
     (the `fixed inset-0 z-40` overlay intercepts pointer events), so
     tests now close via the global 'v' shortcut.

2. **Scenario suite never reached the backend.** The Playwright config
   promises scenarios are 'skipped if the backend isn't reachable', but
   that contract was never enforced — `e2e-workflow`, `task-processing`,
   and `task-workflow` all crashed with `selectOption: did not find some
   options` when `gc` wasn't running on 127.0.0.1:8372.

   - Added `isGcBackendReachable()` (Node `fetch` probe against
     `/health` with a 1.5s timeout) and `gcBackendURL()` to
     `e2e/lib/actions.ts`.
   - Added a `beforeAll` skip guard to every scenario spec.

Also:
- Added `configureGasCityClient()` at module load in
  `gc.functions.ts`. Server functions called `DefaultService.*` with an
  empty `OpenAPI.BASE`, which threw `ERR_INVALID_URL` from axios inside
  Node and flooded the dev-server stderr with stack traces. With a
  configured BASE the calls fail with `ECONNREFUSED` against the
  offline supervisor, which the existing per-handler try/catch degrades
  to empty arrays / `1.0.0` defaults.
- New `tmux-bridge.spec.ts` covers `/api/pty` (HTTP probe + WebSocket
  upgrade validation: hello frame, invalid session names, out-of-range
  cols).

Result: `bun run test:e2e` now reports 69 passed, 23 skipped, 0 failed
(exit 0). The 23 skips are the workflow scenarios that require a live
`gc` supervisor.
Wire the supervisor panel's start/stop/restart/logs controls to the
actual GC API and add a mock-driven e2e suite so the state machine is
covered without a live daemon.

Real (gc.functions.ts):
- gcCityStart now POSTs /v0/city with the anti-CSRF X-GC-Request header,
  then polls /v0/events until request.result.city.create (or
  request.failed) with the matching request_id arrives. Returns ok=true
  only when the backend confirms the city is up.
- gcCityStop mirrors start with POST /v0/city/{name}/unregister.
- gcSupervisorRestart cycles stop+start under the hood and notes in
  the action console that the gc daemon itself is a separate OS
  process that must be restarted by the operator's process manager.
- gcSupervisorLogs reads /v0/events?since=1h instead of returning a
  static stub.
- New gcCityStatus flattens the per-city rich status payload (agents,
  sessions, mail, work counts) so the popover can poll it cheaply on
  a 5s interval with lite=true.

LED state machine (AppShell.tsx):
- Five phases: down | up-stopped | up-running | starting | stopping.
- Phase label mapping makes the LED + label tell a coherent story:
  operational (green) when city is running, supervisor up - city
  stopped (amber) when only the daemon is reachable, down (red) when
  the backend is offline. The popover's start/stop/restart buttons
  enable based on phase so the operator can't fire a no-op.
- Popover shows 4 stat columns (agents, sessions, mail, beads) that
  populate from the city-status poll.

Mock e2e (new):
- e2e/mock-gc-supervisor.ts: tiny node:http server that implements
  just enough of the supervisor surface for the lifecycle spec
  (health, cities, POST /v0/city, POST /v0/city/default/unregister,
  city status, events tail, plus a test-only POST /__reset).
- e2e/with-mock-gc.sh: starts the mock in the background, waits for
  /health, then exec's Vite with GC_API_BASE_URL pointed at the mock.
- playwright.mock.config.ts: runs only e2e/mock/ via bun run test:e2e:mock.
- e2e/mock/supervisor-lifecycle.spec.ts: 3 tests covering the full
  start -> running -> stop -> stopped cycle, the restart cycle, and
  the anti-CSRF rejection invariant. All passing.

The default playwright.config.ts now excludes e2e/mock/ so the regular
bun run test:e2e flow (UI smoke + scenarios) is unaffected.

Result: bun run test:e2e:mock reports 3 passed, 0 failed. The default
test:e2e flow continues to skip scenarios without a live supervisor
(pre-existing behavior, unchanged).
Move the tmux bridge out of the TanStack Start route layer and into a
Vite middleware plugin so the WebSocket upgrade happens at the HTTP
server level (where it belongs) and the API file route becomes a
lightweight GET /api/pty feature-detection probe.

Infrastructure:
- vite/pty-websocket.ts: new tmuxWebSocketPlugin that attaches a
  ws.WebSocketServer to the dev HTTP server and handles the
  Upgrade: websocket for /api/pty. Lazy-imports tmux + node-pty so
  the dev server still starts cleanly when the host is missing them.
- src/server/tmux-pty.ts: standalone tmux attach helpers (parse,
  resolve tmux binary, encode/decode frames) reused by both the
  plugin and any future scripts.
- vite.config.ts: wire tmuxWebSocketPlugin() into the plugin chain
  and alias 'node-pty' to @homebridge/node-pty-prebuilt-multiarch
  so the dev container doesn't have to compile node-pty from source.
- vite-env.d.ts: re-export IPty / spawn from the prebuilt fork
  instead of redeclaring an EventEmitter-shaped IPty (the prior
  declaration re-exported DOM WebSocket, which broke on('message')).
- tsconfig.json: include vite/**/* so the plugin typechecks.
- bun.lock + tsconfig.tsbuildinfo deletes: add
  @homebridge/node-pty-prebuilt-multiarch to deps; drop the two
  tsbuildinfo artifacts that were tracked by accident in earlier
  commits (they're build outputs and should be .gitignored).

UI:
- routes/api/pty.ts: now a GET-only probe returning {available, reason}
  instead of a WebSocket handler. The probe lets the client degrade
  gracefully when tmux or node-pty isn't installed without first
  opening a WebSocket that will fail.
- SessionTerminal.tsx: feature-detects /api/pty on mount, surfaces
  unavailableReason to the user, and only opens the WebSocket when
  the probe reports available.
- SessionsList.tsx: pulls the probe into a useQuery so the session
  list can show a per-host tmux status hint.

No-op for tests: the existing tmux-bridge spec at e2e/tmux-bridge.spec.ts
already covers hello frames, invalid session names, and out-of-range
cols against the real bridge. The GET probe just makes the failure
mode observable before any upgrade attempt.
When the `gc` supervisor is offline (operator hasn't started it, or
it crashed), every `DefaultService.*` call rejects with an AxiosError
whose `code` is one of Node's network-failure codes. Without filtering,
dev-server stderr floods with 50-line axios stack traces on every page
load. Classify and silence them so the operator's terminal stays clean
while still logging genuinely unexpected errors.

- src/lib/gc-errors.ts: silentIfOffline(error) helper. Returns true
  for the expected offline codes (ECONNREFUSED, ECONNRESET,
  ENOTFOUND, ETIMEDOUT, EAI_AGAIN, EHOSTUNREACH, ENETUNREACH,
  EPIPE, ERR_CANCELED, ABORTED). Recurses into `cause` because axios
  nests the underlying network error under the top-level request
  error. Cycle-guarded via identity check.
- tests/unit/gc-errors.test.ts: 8 cases pinning down the classifier
  contract — every silent code is silenced, every non-offline error
  (5xx body, parse failure, TypeError) is NOT silenced, non-error
  inputs return false, cycle protection works.
- vitest.config.ts: enables vitest for the console package, scoped
  to tests/unit/**/* (e2e tests live under e2e/ and are picked up
  by Playwright instead).

Use: gcHealth / gcListAgents / etc. server functions should call
are suppressed when the operator simply hasn't started `gc` yet,
but real bugs (5xx response, JSON parse failure, code TypeError)
still surface in the log.
The mock-gc-supervisor.ts test fixture listens on the same default
port (8372) as a real `gc` daemon. Running it manually — without the
e2e wrapper — would silently shadow the operator's real supervisor
and produce fake /v0/city responses to a console that thinks it's
talking to production. Tighten the gates so the mock can only run
when the operator has explicitly opted into test mode.

- mock-gc-supervisor.ts:
  - Default port changed from 8372 to 8780 (a port no real `gc` would
    use). The e2e wrapper / Playwright config / spec all default to
    8780; nothing in production code paths touches 8780.
  - Add ALLOW_GC_MOCK=1 / Playwright-test-mode guard. The script prints
    a loud red REFUSED banner and exits 2 unless one of those opt-in
    signals is set, so "I forgot the env var" fails fast and visibly.
  - On startup print a loud MOCK banner (red box) that includes the
    port and the test-fixture warning, so a developer who accidentally
    points their browser at it sees immediately that this is fake.
  - Tag every response with X-Gc-Mock: 1 so DevTools network panels
    flag the source as a mock at a glance.
- e2e/with-mock-gc.sh: set ALLOW_GC_MOCK=1 when invoking the mock,
  switch default MOCK_GC_PORT to 8780.
- playwright.mock.config.ts: default MOCK_GC_PORT 8780 (matches the
  mock's own default).
- e2e/mock/supervisor-lifecycle.spec.ts: MOCK_GC_BASE_URL default
  matches the mock's 8780.

Net effect: the mock can no longer be invoked by accident as a dev
backend. The single canonical entry point is `bun run test:e2e:mock`
which sets the env, binds the port, and runs the spec.
…onsole

The supervisor panel used to only handle city lifecycle (start/stop
the city inside an already-running supervisor). Operators had to use
the shell to start `gc` before the panel could do anything. This
makes the panel self-sufficient for first-launch.

gc.functions.ts:
- New runGc(args, opts) helper mirrors runTmux: spawns the CLI with
  a minimal fixed environment, validates the binary path against an
  allow-list (GC_BIN env override), and surfaces a typed
  {ok, output, error} result.
- gcSupervisorStart spawns 'gc start', treats "already running"
  output as ok (idempotent), reports "gas city supervisor started"
  on success.
- gcSupervisorStop is the symmetric shutdown.
- gcSupervisorRestart now actually restarts the daemon (stop+start)
  instead of cycling the city. The previous behavior was misleading —
  the action console claimed a restart but the daemon kept running.

AppShell.tsx:
- Fix type errors from passing {data: {}} to gcSupervisorRestart which
  has no validator. The mutations that still use {data: {}} (city
  start/stop) keep that because their validators accept it.

mock-gc-supervisor.ts:
- Starts in supervisorUp=false (was supervisorUp=true) so e2e can
  exercise the bootstrap path.
- Adds POST /v0/supervisor/{start,stop,restart} endpoints that flip
  supervisorUp, clear city state on supervisor transitions, and
  emit supervisor.started/stopped events for the log query.
- /health and city surface endpoints now return 503 when
  supervisorUp=false. The anti-CSRF X-GC-Request check is performed
  BEFORE the supervisor-up check on POST endpoints so a missing
  header returns 403 (the wire-level invariant), not 503.
- Captures the state reference at request time in the asyncAccepted
  setTimeout so a __reset between request and timer fire doesn't
  leak the old request's completion event into the new state.
- Writes a /tmp/mock-gc-bin/gc shim on startup so the console's
  gcSupervisorStart server function (which spawns 'gc') drives the
  mock via HTTP instead of failing on ENOENT.

e2e/mock/supervisor-lifecycle.spec.ts:
- beforeEach brings the supervisor up so the existing
  start/stop/restart lifecycle tests still see a running backend.
- New "supervisor daemon start endpoint (bootstrap path)" test
  drives the full supervisor lifecycle via the daemon endpoint:
  reset → down → start → up → restart → up → stop → down. Polls
  /health so the panel's LED contract is verified, and confirms
  idempotency on stop.

Result: bun run test:e2e:mock reports 4 passed, 0 failed across 3
consecutive runs. The console's gcSupervisorStart server function
exists and is wired (though the UI panel still drives city start/stop
on its buttons; UI rewiring of the buttons to phase-conditional
targets is a follow-up).
The supervisor panel's start/stop buttons previously only knew how
to start/stop the city inside an already-running supervisor. With
gcSupervisorStart/Stop/Restart in place, the same buttons now do
double-duty depending on the current phase:

  start  phase=down       → spawn 'gc start'  (bring the daemon up)
  start  phase=up-stopped → POST /v0/city      (start the city)
  stop   phase=up-running → POST /v0/city/.../unregister (stop the city)
  stop   phase=up-stopped → spawn 'gc stop'   (bring the daemon down)
  restart phase=up-*      → spawn 'gc restart' (restart the daemon)

The button labels stay constant ("start"/"stop"/"restart"); the
current target is exposed via a data-action-kind attribute
(supervisor-start, city-start, city-stop, supervisor-stop,
supervisor-restart, or unavailable). This lets tests assert what the
click will do, and gives the tooltip room to describe the action
("start the gc supervisor daemon" vs "start the city").

AppShell.tsx:
- Capture the phase-conditional target in a ref at click time
  (captureStartIntent / captureStopIntent) so React StrictMode's
  intentional double-invoke of useMutation callbacks in dev doesn't
  drift: the first invoke fires supervisorStart() and the second
  re-invoke (after React re-renders with phase='starting') would
  otherwise fire cityStart(). With the ref, both invokes read the
  same captured target.
- useEffect now clears 'starting' once the system has reached the
  target state (either health.reachable OR city.running), and
  'stopping' once it's actually stopped (either !city.running OR
  transitions, so a supervisor-start (no city yet) would leave the
  panel stuck on 'starting…' indefinitely.

e2e/mock/supervisor-lifecycle.spec.ts:
- Existing tests now assert data-action-kind on the buttons and the
  phase-conditional action console labels ("$ gc city start" vs
  "$ gc city stop" — beforeEach brings the supervisor up).
- The restart test now asserts the daemon-level action (since the
  restart button actually restarts the daemon now).
- New 'UI bootstrap' test: reset the mock so the supervisor is
  down, open the popover, click start. Asserts the button is
  enabled with data-action-kind=supervisor-start, that the action
  console records '$ gc supervisor start / gc supervisor started',
  and that the panel settles on 'supervisor up · city stopped' with
  the start button now targeting the city. This is the user-visible
  proof that 'start the supervisor from the console' works.

Result: bun run test:e2e:mock reports 5 passed, 0 failed across 3
consecutive runs.
Two small UX wins surfaced by the operator flow:

1. Better error when the 'gc' binary isn't on PATH. The console's
   gcSupervisorStart server function spawns 'gc start' (via the same
   safeGcBin() allow-list pattern used for tmux). If the binary isn't
   installed the spawn returns exit=-1 with stderr 'spawn gc ENOENT'.
   We now special-case that combination and return a friendly message
   pointing the operator at GC_BIN=/absolute/path/to/gc as the fix,
   instead of dumping the raw ENOENT into the action console.

2. Copy buttons for the action console and supervisor log sections.
   Both sections now have a 'copy' button in their header (next to the
   existing 'clear' on the action console). Clicking briefly flips the
   label to 'copied!' for visual feedback, then back. The copy uses
   the navigator Clipboard API when available and falls back to a
   hidden textarea + execCommand when clipboard permissions are
   unavailable (headless Chromium, sandboxed iframes).

   A new test verifies both buttons: data-action-kind-style checks
   that the label flips to 'copied!' then back to 'copy' on click.

Result: bun run test:e2e:mock reports 6 passed, 0 failed across 3
consecutive runs.
Add two operator-facing controls to the supervisor popover so the
console can drive a gc supervisor that lives outside the project
root or behind a different host/port.

**City directory**

- New `resolveCityDir(override)` helper in gc.functions.ts with
  precedence: explicit override -> GC_CITY_DIR env -> console cwd.
  Validated against an allow-list (GC_CITY_ROOT, defaults to HOME)
  so a hostile client cannot make us `gc init /etc`.
- New `gcCityProbe` server fn reports whether the resolved dir is
  initialized (city.toml or .gc/ present) without spawning gc.
- New `gcCityInit` server fn wraps `gc init <path>` and creates
  the target directory if missing.
- Supervisor panel: input + persisted localStorage + live
  initialized/not-initialized badge. `start` is blocked and a
  yellow `init` button appears when the dir is uninitialized.
  `gcCityInitWithPacks` (used by the /cities page) now also
  delegates to the real init.

**Supervisor URL**

- New `resolveSupervisorUrl(override)` / `discoverSupervisorUrl`
  helpers with precedence: override -> GC_API_BASE_URL (console-
  side convention) -> ~/.gc/supervisor.toml -> upstream default
  http://127.0.0.1:9443.
- `parseSupervisorToml` is a tiny inline parser for the [supervisor]
  section (no new dep); `buildSupervisorUrlFromToml` mirrors the
  upstream Go CLI's normalisation (0.0.0.0 -> 127.0.0.1, :: -> ::1,
  strip IPv6 brackets).
- `withSupervisorUrl` re-points the OpenAPI client for the
  duration of a single server fn and restores it in finally, so
  per-request `apiUrl` overrides work without global state.
- `gcHealth`, `gcCityStatus`, and the new `gcSupervisorDiscover`
  server fn all honour the override; the discover fn also does a
  2s HEAD probe to /v0/health for the LED.
- `isSupervisorApiDisabled` reads the upstream `GC_NO_API` escape
  hatch (1/true/yes/on) and short-circuits with a clean 'api
  disabled' state instead of ECONNREFUSED spam.
- Supervisor panel: input + persisted localStorage + badge that
  distinguishes override/env/auto/default and reachable/offline/
  api-disabled.

**Deliberately NOT done**

- `GC_SUPERVISOR_URL`. Verified against gastownhall/gascity that
  the upstream Go CLI does not define it, so honouring it on the
  console would mislead operators. A guard-rail test pins this
  behaviour.

**Tests** (36/36 pass, up from 15)

- city-dir.test.ts: allow-list, ..-collapse, GC_CITY_DIR fallback,
  HOME-less fallback, reject /etc and /tmp, widened root.
- supervisor-url.test.ts: TOML parser, URL builder wildcard
  normalisation, override > env > default precedence,
  GC_API_BASE_URL is read, GC_SUPERVISOR_URL is NOT read
  (guard rail), trailing-slash trim, GC_NO_API escape hatch
  truthy values, end-to-end supervisor.toml shape.

**E2E helpers**

- `waitForSupervisorPanel` waits for the supervisor health query
  to round-trip; `with-mock-gc.sh` accepts any HTTP code from the
  probe (the mock returns 503 when supervisor is intentionally down)
  and points GC_BIN at the shim so supervisor start/stop server
  fns drive the mock instead of failing with ENOENT.
Three related fixes that together make the supervisor popover
actually work end-to-end on a fresh machine.

**1. Use `gc supervisor start`, not `gc start`**

`gc start` requires a bootstrapped city (city.toml in cwd) and
fails with `not in a city directory` when the console server's
cwd is `packages/@gascity/console` rather than the workspace. The
popover's job is to bring the supervisor up — that's
`gc supervisor start`, which has no such requirement. We were
unintentionally falling back to the wrong subcommand after a
`git reset --hard` had rolled this fix out of history.

`stop` and `restart` updated to match. Error messages relabelled
("gc supervisor start failed", "gc supervisor stop threw", ...)
so the action console matches the actual command. The
`not-in-city-dir` error code is removed (no longer reachable).

**2. Phase model: `up`/`down` only, no city-state inference**

The popover used to derive city lifecycle (operational vs city
stopped) and show extra controls (city start/stop, init button,
stats grid). None of that belongs here — the popover supervises
the supervisor, not cities. The phase machine collapses to four
states: `down | up | starting | stopping`. Buttons simplify to:
`start` when down, `stop` + `restart` when up.

The /cities page already handles city lifecycle (register, list,
start, stop, init) and is the right place to manage city state.

**3. Supervisor log gets follow / pause / clear / copy controls**

The `/v0/events` stream is the operator's primary feedback
channel when the supervisor is up. We expose:
  - `follow`  — auto-scroll to the newest line (default on)
  - `pause`   — freeze polling so the view doesn't jump
  - `clear`   — wipe the local buffer (next fetch repopulates)
  - `copy`    — clipboard

Placeholder texts (`(loading…)`, `(no supervisor events in the last hour)`)
render italic + dimmer so the operator can tell at a glance that
nothing meaningful has arrived yet.

**Also**

- Action console restored (was dropped in an earlier
  simplification). It's the only signal we get when `gc supervisor
  start` fails in tmux / a background service — auth errors,
  port-in-use, dolt identity probe, etc.
- The supervisor URL input + persisted localStorage was removed
  from the popover UI. The server still resolves the URL itself
  (env / supervisor.toml / default); the popover doesn't need to
  know about it.

**Tests**

All existing tests still pass (36/36). No new tests added — the
`gc supervisor start` choice is documented inline in
`startSupervisorImpl` and verified by running it manually.
Adds a /marketplace route that consumes the upstream
gastownhall/gascity-packs registry directly. The Browse tab
shows every pack across all configured registries with
install/uninstall actions. The Installed tab shows imported
packs, flags update-available packs, and exposes per-pack
Update and a bulk Update all action.

Key plumbing:
- gc pack registry add/list/remove/refresh server fns
  wrap the upstream gc CLI with normalised envelopes.
- gc import add/remove/upgrade server fns (with --version
  pin and uniform CLI failure classification).
- A deps-free TOML parser scoped to the upstream
  registry.toml shape, plus latestRelease() with proper
  numeric semver compare.
- gcListMarketplaceEntries joins installed packs against
  the catalog by name AND normalised source URL so that
  packs installed under a renamed binding still match.
- computePackUpdates reports up_to_date / update_available /
  not_in_catalog status per installed pack.
- runGc auto-discovers the city dir from the supervisor's
  /v0/cities endpoint and passes --city on every spawn, so
  the console no longer needs GC_CITY_DIR or a city cwd.

UI:
- /marketplace is a single page with Browse/Installed tabs.
- Browse shows registries strip (add/refresh/remove) +
  responsive grid of cards with source/install icons in the
  top-right corner.
- Installed shows per-pack status with Update / Uninstall
  buttons and a sticky Update-all floating action.
- /packs redirects to /marketplace for backward compat.
- AppShell uses h-screen + h-full overflow-hidden so the
  page never scrolls vertically past the viewport; only the
  inner grid scrolls.

Tests:
- 84 unit tests across registry-toml parsing, semver
  comparison, tag/tier inference, computePackUpdates join
  logic (name + source match, fork scenarios), registry
  feedback envelope parsing, pack name validation helpers,
  and gc-errors classification.

Refs the marketplace e2e suite (marketplace.spec.ts) and
updates navigation/palette e2e to the new /marketplace URL.
Adds a new devcontainer feature that installs the Kilo CLI plus its
libnotify-bin apt dependency. The kilo binary itself is installed by
declaring the existing homebrew feature as a hard dependency via
`dependsOn` and forwarding the Kilo-Org/tap/kilo package through its
`packages` option, so homebrew is pulled in automatically and no
duplication of the brew-install logic is needed.
## What changed

### Console code (src/lib/gc.functions.ts)
- Replaced require('node:path') with static import (was crashing
  server fns in the browser-bundled context)
- Added skipCity option to runGc for supervisor-level commands
- Widened BEAD_ID_RE to accept per-rig prefixes (e.g. BL-42, FE-1)
  instead of only gd-/bd- forms
- Updated parseSlingOutput to match upstream stdout markers
  (Created/Slung/Started workflow/Attached wisp)
- Fixed SUPERVISOR_URL_DEFAULT from 9443 to 8372 (matches upstream)
- Wired GC_* env vars through runGc spawn (GC_DOLT_SKIP, GC_HOME,
  GC_NO_API, GC_SUPERVISOR_LOG_TEE)
- Made gcCityInitWithPacks chain gc init -> gc start (was only gc
  init, leaving city unregistered)
- Added GC_RIG_DIR support for rig config injection
- startCityImpl now accepts dir parameter

### AppShell (src/components/AppShell.tsx)
- Fixed transition state stuck after start/stop/restart mutations
  (added setTransition(null) in all onSuccess handlers)

### Cities page (src/routes/cities.tsx)
- Init dialog surfaces r.error alongside r.output

### Mock GC supervisor (e2e/mock-gc-supervisor.ts)
- Added GET /v0/city/{name}/sessions endpoint
- Added supervisor sub-command handling (gc supervisor start|stop)

### Supervisor lifecycle spec (e2e/mock/supervisor-lifecycle.spec.ts)
- Removed assertions on unimplemented stat-agents/stat-sessions
  testids (Phase 1-2 fixed, Phase 3 pre-existing)
- Updated openSlingDrawer for hydration + hydration-aware click wait

### Playwright configs
- GC_BACKEND wired into webServer.env for real e2e
- Added GC_DOLT_SKIP, GC_CITY_ROOT, GC_RIG_DIR env vars
- Bumped mock webServer timeout to 90s

### New files
- e2e/rig/ - Devin CLI rig config (agent.toml + prompt.template.md)
- e2e/scenarios/sling-pickup.spec.ts - sling E2E spec
- tests/unit/gc-sling.test.ts - parseSlingOutput unit tests
- tests/unit/gc-close-bead.test.ts - bead-id validator tests

## Test evidence
- unit tests: 103/103 pass (was 98, +5 new)
- e2e:mock: 5 pass, 1 fail (restart test: pre-existing timing issue)
- typecheck: clean (0 errors)
- e2e real: sling-pickup spec blocked by devcontainer Dolt state
  (fresh gc init fails with 'dirty tables' migration error on the
  dolt backend after prior test runs left inconsistent state). This
  is an environment issue, not a code bug.
Applied across devcontainer, e2e harness, marketplace UI, and
supervisor plumbing after running the PR locally (Node 24 + Bun
1.3.14 + Playwright 1.61 on Linux):

P0
- features/kilo: dependsOn "./homebrew" → "./features/homebrew"
  (the resolved path didn't exist; sibling features use the
  "features/" prefix consistently).

P1
- e2e/rig/README.md: bootstrap snippet referenced non-existent
  files at the rig root; corrected to the actual layout under
  agents/devin-test/.
- e2e/with-mock-gc.sh: the EXIT trap was set BEFORE the exec, so
  bash dropped it across exec() and the mock-gc supervisor was
  reparented to init, leaving it orphaned when Playwright
  SIGTERMed the wrapper. Now: launch mock + vite as siblings,
  install the cleanup trap once we have both PIDs, and skip
  exec() entirely so bash stays as the parent of both processes.
- e2e/mock-gc-supervisor.ts: TMPDIR="" was treated as a real
  path by `??` (`'' ?? '/tmp'` is `''`), writing the gc shim
  to /mock-gc-bin while the bash wrapper looked under /tmp.
  Mirror the bash `${TMPDIR:-/tmp}` semantics explicitly.

P2
- packages/@gascity/console/package.json: ws and
  @homebridge/node-pty-prebuilt-multiarch are dev-only (Vite
  plugin with apply:"serve"; production route returns 403).
  Moved both into devDependencies.
- packages/@gascity/console/src/lib/gc.functions.ts +
  tests/unit/supervisor-url.test.ts: IPv6 host literals must
  be wrapped in brackets when followed by a port (RFC 3986
  §3.2.2); the previous output (e.g. `http://::1:9000`) failed
  strict URL parsers. Bracket-wrap on output and update the
  test expectations accordingly.
- README: removed stale line-number references (parseSlingOutput
  / BEAD_ID_RE / gcSling / gcCloseBead moved during the
  TanStack-Start migration).
- src/components/SlingComposer.tsx: `slingMut.data?.error`
  didn't catch `{ ok: false, error: "" }` envelopes; switch
  the failure branch to a positive `ok === false` check.
- src/lib/registry-feedback.ts: summariseAdd/summariseRemove
  now bail when the envelope carries `ok: false`, so a failed
  add/remove no longer surfaces as a misleading success
  summary.
- src/styles.css: dropped the redundant second `@source`
  directive and the duplicate shadcn theme block (kept the
  Lovable design system + the components layer that uses it).
- src/routes/marketplace.tsx: add-registry button now disables
  on whitespace-only input; uninstall mutation variables are
  threaded through so the InstalledCard button correctly
  reflects pending state per pack.
- e2e/tests/marketplace.spec.ts: removed the duplicate
  'can access marketplace page' assertion, replaced the
  no-op search check with a real filter assertion, and
  dropped the conditional `if (count > 0)` guard.
- e2e/tests/supervisor.spec.ts: switched to data-testid
  selectors that match the source (the accessible names are
  'start supervisor' etc., not 'start'); broadened the
  version-string matcher to also accept the unreachable
  fallback copy so the test doesn't require a live backend.
- .devcontainer/devcontainer.json: removed invalid
  installGlibc option from the Node feature (the base image
  already provides glibc and the option isn't supported on
  features/node:2.1.0).

P3
- e2e/mock-gc-supervisor.ts: city.work is now reset in the
  unregister handler so stale open/closed counts can't
  leak through /v0/city/{name}/status.
- vite/pty-websocket.ts: the close handler was registered on
  both httpServer and server.httpServer (same underlying
  node:http.Server in practice) — register on whichever is
  non-null so cleanup runs exactly once.
- packages/@gascity/client/package.json: add unrun to
  devDependencies (tsdown requires it for loading the
  config in ESM mode; without it `bun run build` errors
  out with 'Failed to import module unrun').

Verification
- bunx vitest run                    → 103 passed
- bun run build                      → built in 9s, 0 errors
- bunx tsc --noEmit                  → 0 errors
- bun run test:e2e:mock              → 6 passed
- bun run test:e2e                   → 69 passed, 24 skipped (workflow
  scenarios that need a live gc supervisor), 0 failed
- Move pkill -P "${VITE_PID}" inside conditional block to prevent stderr errors when VITE_PID is empty (gemini-code-assist suggestion)
- Fix uninstallVars comparison in marketplace.tsx - compare directly against string instead of treating as object with name property (codeant-ai finding)

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- Move manual module augmentation from auto-generated routeTree.gen.ts to separate router-types.d.ts file to prevent overwriting during route regeneration
- The @theme and :root CSS variable definitions are already present in styles.css (codacy comment appears to be outdated)

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- Fix MOCK_E2E_PORT vs E2E_PORT mismatch in playwright.mock.config.ts (use E2E_PORT to match wrapper script)
- Extract duplicate parsePort function to shared playwright-utils.ts to eliminate code duplication
- Update both playwright configs to import shared parsePort function

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- Fix eager port parsing in playwright.config.ts - only parse E2E_PORT when actually needed for BASE_URL construction
- Add data-testid attributes to BrowseCard and InstalledCard components for proper e2e test selection
- Update marketplace test to use correct data-testid selector pattern (pack-card- instead of pack-card)
- Enhance supervisor test assertion to include "supervisor up" steady-state text for live backend scenarios

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- Fix shell heredoc quoting error in rig/README.md - properly escape nested single quotes in sed command
- Remove redundant INT/TERM registration in with-mock-gc.sh trap - only EXIT needed since INT/TERM are handled separately later

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed

Not reviewed (too large): packages/@gascity/console/src/lib/gc.functions.ts (~3,454 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/@gascity/console/src/server/tmux-pty.ts
Comment thread .codacy.yaml
Comment thread packages/@gascity/console/e2e/scenarios/sling-pickup.spec.ts Outdated
Comment thread packages/@gascity/console/e2e/scenarios/sling-pickup.spec.ts Outdated
Comment thread .kilo/plans/1782605750670-ui-feature-catalog-e2e-sling-plan.md Outdated
Comment thread packages/@gascity/console/src/lib/registry-toml.ts Outdated
Comment thread packages/@gascity/console/src/routes/cities.tsx
Comment thread .devcontainer/devcontainer.json Outdated
Comment thread packages/@gascity/console/e2e/tests/beads.spec.ts Outdated
Comment thread packages/@gascity/console/src/routes/api/pty.ts Outdated
codescene-access[bot]

This comment was marked as outdated.

Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
codescene-access[bot]

This comment was marked as outdated.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 1 file (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/@gascity/console/src/routes/cities.tsx Outdated
devin-ai-integration Bot and others added 5 commits July 12, 2026 22:39
… semver

Treat '#' inside quoted TOML values as data, not an inline comment, and
compare release versions by semver core precedence before the
stable/prerelease tiebreak (a hyphen in build metadata is no longer read
as a prerelease). Adds regression tests.

Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
…d buffer decoding

Decode single Node Buffer frames from ws (text frames arrive as Buffers,
so keystrokes/resize were silently dropped); reject cross-origin
WebSocket upgrades; use WebSocket.OPEN instead of a per-socket property;
gate /api/pty and the Vite probe on loadPty() plus an executable tmux
check so readiness can't report a false positive/negative; drop the
self-referential ambient module declarations. Adds handleBrowserMessage tests.

Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
Hoist the supervisor-logs useQuery above its first use so 'log' is no
longer read in the temporal dead zone. Make the new-city dialog dismiss
on Escape, trap Tab focus within it, and restore focus on close. Split
the orders row's selectable region from its nested action buttons so
Space/Enter activates the focused button natively.

Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
…tart state

Anchor bead-id extraction to the 'slung. bead <id>' marker in visible
text and accept lowercase per-rig prefixes; scope the diagnostics
assertion to the sling dialog instead of full-page HTML; target the
beads close button by exact role name so it can't match the 'closed'
filter; capture the mock supervisor state before the delayed restart so
a concurrent __reset can't contaminate the next test.

Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
…c sling plan

Add the required '---' document marker so .codacy.yaml is honored and
document each narrowly-pinned exclusion; remove the duplicate Playwright
browser install from the devcontainer postCreate (the playwright feature
already installs chromium); reconcile the sling e2e plan with the shipped
--json gcSling/gcCloseBead implementation.

Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
codescene-access[bot]

This comment was marked as outdated.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 17 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread packages/@gascity/console/src/routes/cities.tsx
Comment thread packages/@gascity/console/vite/pty-websocket.ts Outdated
Comment thread packages/@gascity/console/src/routes/orders.tsx Outdated
…, HTTPS origin)

Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
codescene-access[bot]

This comment was marked as outdated.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
packages/@gascity/console/src/server/tmux-pty.ts (1)

121-138: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

isTmuxAvailable — consider passing an explicit env to spawnSync.

SonarCloud flags the inherited PATH variable as a security concern. While the docstring correctly notes this is dev-only and downstream callers validate tmuxBin against TMUX_BIN_RE, passing a minimal env (e.g., { PATH: process.env.PATH }) would silence the warning and reduce the surface if this function is ever reused outside the dev container.

♻️ Optional: restrict inherited environment
 export function isTmuxAvailable(tmuxBin = "tmux"): boolean {
   try {
-    const res = spawnSync(tmuxBin, ["-V"], { stdio: "ignore", timeout: 2000 });
+    const res = spawnSync(tmuxBin, ["-V"], {
+      stdio: "ignore",
+      timeout: 2000,
+      env: { PATH: process.env.PATH },
+    });
     return !res.error && res.status === 0;
   } catch {
     return false;
   }
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/`@gascity/console/src/server/tmux-pty.ts around lines 121 - 138,
Update isTmuxAvailable to pass an explicit minimal env object to spawnSync,
preserving the existing PATH value needed to locate tmux while avoiding
unrestricted environment inheritance. Keep the current timeout, stdio, status,
and error handling unchanged.

Source: Linters/SAST tools

packages/@gascity/console/src/routes/api/pty.ts (1)

89-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the nested ternary for readability.

Flagged by static analysis (SonarCloud).

♻️ Proposed fix
-        probe.message = probe.ok
-          ? "ready — open a WebSocket to attach a session"
-          : !probe.nodePty
-            ? "node-pty not installed. Run `bun add `@homebridge/node-pty-prebuilt-multiarch`` and restart the dev server."
-            : `tmux binary not found or not executable (${tmuxBin}). Install tmux and restart the dev server.`;
+        if (probe.ok) {
+          probe.message = "ready — open a WebSocket to attach a session";
+        } else if (!probe.nodePty) {
+          probe.message =
+            "node-pty not installed. Run `bun add `@homebridge/node-pty-prebuilt-multiarch`` and restart the dev server.";
+        } else {
+          probe.message = `tmux binary not found or not executable (${tmuxBin}). Install tmux and restart the dev server.`;
+        }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/`@gascity/console/src/routes/api/pty.ts around lines 89 - 93,
Refactor the nested ternary assigning probe.message in the PTY probe flow into a
clear conditional structure, such as an if/else chain, while preserving the
existing ready, missing node-pty, and missing tmux binary messages exactly.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/`@gascity/console/src/routes/api/pty.ts:
- Around line 57-71: Remove the unused websocket property from the probe object
and its type definition in the PTY route handler. Do not perform or report an
upgrade check there; preserve the existing tmux, nodePty, message, and other
probe behavior.

In `@packages/`@gascity/console/vite/pty-websocket.ts:
- Around line 129-135: Update the origin validation around expectedOrigin to
honor the deployment’s trusted forwarded HTTPS indication when Vite itself
reports HTTP, so browser HTTPS origins behind devcontainer/Codespaces proxies
are accepted. Preserve strict origin matching and socket destruction for missing
or untrusted origins; use the existing server/request configuration symbols
rather than broadly bypassing validation.

---

Nitpick comments:
In `@packages/`@gascity/console/src/routes/api/pty.ts:
- Around line 89-93: Refactor the nested ternary assigning probe.message in the
PTY probe flow into a clear conditional structure, such as an if/else chain,
while preserving the existing ready, missing node-pty, and missing tmux binary
messages exactly.

In `@packages/`@gascity/console/src/server/tmux-pty.ts:
- Around line 121-138: Update isTmuxAvailable to pass an explicit minimal env
object to spawnSync, preserving the existing PATH value needed to locate tmux
while avoiding unrestricted environment inheritance. Keep the current timeout,
stdio, status, and error handling unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 652b8caf-da9d-42f2-a44e-c8534182b98c

📥 Commits

Reviewing files that changed from the base of the PR and between 937abb6 and b59535e.

📒 Files selected for processing (17)
  • .codacy.yaml
  • .devcontainer/devcontainer.json
  • .kilo/plans/1782605750670-ui-feature-catalog-e2e-sling-plan.md
  • packages/@gascity/console/e2e/mock-gc-supervisor.ts
  • packages/@gascity/console/e2e/scenarios/diagnostics.spec.ts
  • packages/@gascity/console/e2e/scenarios/sling-pickup.spec.ts
  • packages/@gascity/console/e2e/tests/beads.spec.ts
  • packages/@gascity/console/src/components/AppShell.tsx
  • packages/@gascity/console/src/lib/registry-toml.ts
  • packages/@gascity/console/src/routes/api/pty.ts
  • packages/@gascity/console/src/routes/cities.tsx
  • packages/@gascity/console/src/routes/orders.tsx
  • packages/@gascity/console/src/server/tmux-pty.ts
  • packages/@gascity/console/src/vite-env.d.ts
  • packages/@gascity/console/tests/unit/registry-toml.test.ts
  • packages/@gascity/console/tests/unit/tmux-pty.test.ts
  • packages/@gascity/console/vite/pty-websocket.ts
🚧 Files skipped from review as they are similar to previous changes (6)
  • .codacy.yaml
  • packages/@gascity/console/e2e/tests/beads.spec.ts
  • packages/@gascity/console/e2e/mock-gc-supervisor.ts
  • .kilo/plans/1782605750670-ui-feature-catalog-e2e-sling-plan.md
  • packages/@gascity/console/e2e/scenarios/sling-pickup.spec.ts
  • packages/@gascity/console/src/routes/cities.tsx

Comment thread packages/@gascity/console/src/routes/api/pty.ts
Comment thread packages/@gascity/console/vite/pty-websocket.ts
…to for PTY origin

Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
codescene-access[bot]

This comment was marked as outdated.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/`@gascity/console/src/server/tmux-pty.ts:
- Around line 132-136: Update isTmuxAvailable and the /api/pty feature-detection
flow to avoid spawnSync blocking the event loop; use a cached availability
result or an asynchronous tmux version probe with an enforced timeout, while
preserving the existing unavailable behavior when tmux is missing or hangs.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e45822c8-ab7f-497f-8150-12a7836cbe8e

📥 Commits

Reviewing files that changed from the base of the PR and between b59535e and 2b36210.

📒 Files selected for processing (3)
  • packages/@gascity/console/src/routes/api/pty.ts
  • packages/@gascity/console/src/server/tmux-pty.ts
  • packages/@gascity/console/vite/pty-websocket.ts
💤 Files with no reviewable changes (1)
  • packages/@gascity/console/src/routes/api/pty.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/@gascity/console/vite/pty-websocket.ts

Comment thread packages/@gascity/console/src/server/tmux-pty.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread packages/@gascity/console/src/server/tmux-pty.ts
Caches the spawnSync tmux probe result per binary path so repeated
/api/pty probe requests don't re-spawn a synchronous process. The
first call still blocks for up to 2s, but subsequent requests return
instantly from the Map cache.

Addresses coderabbitai and cubic-dev-ai review threads on PR #6.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread packages/@gascity/console/src/server/tmux-pty.ts Outdated
Failed probe results are no longer cached so that installing tmux
mid-session is detected on the next probe request without requiring
a dev server restart.
@sonarqubecloud

Copy link
Copy Markdown

@ThePlenkov
ThePlenkov merged commit 3cbbd72 into gascity-extra:main Jul 13, 2026
8 checks passed
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

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