shell integration. =terminal #71

New issue

Open

opened 2026-04-29 09:17:13 +00:00 by despiegk · 3 comments

despiegk commented 2026-04-29 09:17:13 +00:00

Owner

Copy link

issue 1:

only show tty enabled sessions

issue 2

check implementation with hero_router, some functionality can be reused
should be efficient, maybe we can reuse functionality from hero_router in hero_proc, so its more like a lib we an use, rather than duplication

not all functions are same, but the reusable ones could be put in lib on hero_router level

## issue 1: only show tty enabled sessions  ## issue 2 check implementation with hero_router, some functionality can be reused should be efficient, maybe we can reuse functionality from hero_router in hero_proc, so its more like a lib we an use, rather than duplication not all functions are same, but the reusable ones could be put in lib on hero_router level

despiegk added this to the ACTIVE project 2026-04-29 09:17:18 +00:00

despiegk commented 2026-05-01 04:22:22 +00:00

Author

Owner

Copy link

Issue #71 Implementation Spec — Shell Integration / Terminal

Objective

Two-part fix for the hero_proc admin dashboard's terminal feature:

• Part A (do now): Filter the "Select a running job" dropdown on the Terminal tab so it only shows jobs whose action has tty: true. Currently it shows every running job, including non-TTY jobs that fail to attach.
• Part B (proposal only): Document the duplication between hero_proc and hero_router around shell/PTY/terminal handling, and propose a concrete extraction into a shared library crate inside the hero_router workspace that hero_proc can depend on. No code is moved in this issue — output is a written follow-up plan.

Requirements

1. The Terminal tab dropdown in hero_proc UI must only list running jobs whose underlying action is TTY-enabled (action.tty == true). Non-TTY jobs are hidden from the list.
2. The Terminal action button shown in the Job-detail panel must continue to behave exactly as today (it already gates on actionSpec.tty).
3. The change should be a minimal, low-risk patch — UI-side filtering is preferred to avoid altering the JSON-RPC schema or the JobFilter shape.
4. Part B must be added as a design proposal in docs/ with no code moves; a follow-up issue will be opened to do the extraction.

Files to Modify (Part A only)

• crates/hero_proc_ui/templates/index.html — populateTerminalJobList IIFE around line 662; add an action-spec lookup and filter jobs to only those whose action has tty=true.

(No backend change required; cachedActions is already populated globally by the dashboard load path.)

Files to Create (Part B only)

• docs/proposal-shared-terminal-lib.md — design proposal describing what should be extracted from hero_router::server::terminal into a new shared library crate inside the hero_router workspace, and how hero_proc would consume it. (Marked clearly "DESIGN PROPOSAL — see follow-up issue.")

Implementation Plan

Step 1 — UI filter on the Terminal tab

Files: crates/hero_proc_ui/templates/index.html

Dependencies: none (server-side schema unchanged)

What to do:

In populateTerminalJobList (currently at index.html:662), after the existing jobs.filter(function (j) { return j.phase === 'running'; }) step and before the sort, add a second filter that intersects the running set with the cached action specs:

• Look up each job.name (the action name in JobSummary) in the global cachedActions array (already populated by dashboard.js).
• Keep only jobs whose matching action spec has tty === true.
• If the action is not found in cachedActions (race during startup), fall through to excluding the job — this matches the conservative behaviour of the Job-detail panel which only shows the Terminal button when the action is found AND has tty.

Also update the empty-option label from -- Select a running job -- to -- Select a running TTY job -- so the filter is visible to the user.

Rationale for UI-side over backend-side filter:

• JobSummary does not carry tty — the flag lives on ActionSpec. Adding it would require changes to JobSummary, to_summary, the SQL row reader, plus extending JobFilter with tty: Option<bool>. That is a much larger blast radius for a UI-only concern.
• cachedActions is already loaded by the dashboard on startup; the join is cheap.
• The Job-detail panel already uses exactly this cachedActions.find(... === a.name) pattern, so the filter is consistent with existing code.

Step 2 — Verify the dropdown still auto-attaches correctly

Files: none changed — manual smoke test only.

After Step 1, reload the Terminal tab and confirm:

• A non-TTY action's running job does NOT appear.
• A TTY action's running job DOES appear and can be attached.
• navigateToTerminalJob(id) still works because the caller already gates on hasTty.

Step 3 — Write the Part-B design proposal

Files: docs/proposal-shared-terminal-lib.md (new file)

Dependencies: Step 1 is independent and can ship first.

The document inventories actual duplication between hero_proc and hero_router and proposes a new crate hero_terminal_lib inside the hero_router workspace, exposing:

• session.rs — ShellType, TerminalSession, validate_name, encode_job_name, decode_job_name, shell_suffix, shell_from_suffix
• manager.rs — create_session / list_sessions / get_session / delete_session parameterised over a hero_proc_sdk client
• frame.rs — typed TerminalEvent enum for the wire format
• ansi/mod.rs — strip_local_terminal_queries, strip_da_plaintext_artifacts, pending_query_prefix_len plus their tests

What stays in hero_proc: PtyHandle, the portable_pty-based supervisor, the /api/jobs/{id}/pty WS endpoint, pty_handles map, scrollback buffer.

What stays in hero_router: pty_proxy and attached() registry remain as the WS-proxy concrete implementation, calling into the new crate for ansi::* and frame parsing.

Migration plan (for follow-up issue):

1. Create hero_terminal_lib, move name/encoding/strip helpers + tests, repoint hero_router use sites.
2. Move session CRUD into manager.rs, parameterised over an injected hero_proc_sdk client. The terminal.* RPC handlers in hero_router become 5-line shims.
3. Optional: unify wire protocol — move hero_proc to typed TerminalEvent frames, or accept both indefinitely.

Acceptance Criteria

• Loading the Terminal tab in hero_proc admin UI lists ONLY running jobs whose action has tty: true.
• A running job whose action has tty: false (or no tty field) is NOT present in the dropdown.
• The Terminal-tab dropdown placeholder text reads -- Select a running TTY job --.
• Clicking "Terminal" on the Job-detail panel for a TTY job continues to auto-attach correctly via navigateToTerminalJob.
• docs/proposal-shared-terminal-lib.md exists and contains the duplication inventory and extraction shape. The file's first paragraph is > DESIGN PROPOSAL — implementation tracked in follow-up issue.
• No changes to Rust code, schemas, or CI in this issue.

Notes

• No backend change. JobSummary and JobFilter stay as-is. Adding tty to either is left for a future cleanup if a backend filter is ever required.
• The UI filter relies on cachedActions being populated. If empty (user lands directly on #terminal/<id>), the conservative behaviour is to show no jobs — preferable to showing all of them, because clicking a non-TTY job would error on attach anyway.
• The Part-B proposal deliberately treats the PTY ownership layer (PtyHandle, scrollback, the WS endpoint) as not duplicated — only hero_proc owns these and hero_router proxies through them.
• The follow-up issue must verify that hero_proc's CI can resolve a workspace dependency on hero_terminal_lib from the hero_router workspace via the standard CODEROOT-patch route.

# Issue #71 Implementation Spec — Shell Integration / Terminal ## Objective Two-part fix for the hero_proc admin dashboard's terminal feature: - **Part A (do now):** Filter the "Select a running job" dropdown on the Terminal tab so it only shows jobs whose action has `tty: true`. Currently it shows every running job, including non-TTY jobs that fail to attach. - **Part B (proposal only):** Document the duplication between hero_proc and hero_router around shell/PTY/terminal handling, and propose a concrete extraction into a shared library crate inside the hero_router workspace that hero_proc can depend on. No code is moved in this issue — output is a written follow-up plan. ## Requirements 1. The Terminal tab dropdown in hero_proc UI must only list running jobs whose underlying action is TTY-enabled (`action.tty == true`). Non-TTY jobs are hidden from the list. 2. The Terminal action button shown in the Job-detail panel must continue to behave exactly as today (it already gates on `actionSpec.tty`). 3. The change should be a minimal, low-risk patch — UI-side filtering is preferred to avoid altering the JSON-RPC schema or the `JobFilter` shape. 4. Part B must be added as a design proposal in `docs/` with no code moves; a follow-up issue will be opened to do the extraction. ## Files to Modify (Part A only) - `crates/hero_proc_ui/templates/index.html` — `populateTerminalJobList` IIFE around line 662; add an action-spec lookup and filter `jobs` to only those whose action has `tty=true`. (No backend change required; `cachedActions` is already populated globally by the dashboard load path.) ## Files to Create (Part B only) - `docs/proposal-shared-terminal-lib.md` — design proposal describing what should be extracted from `hero_router::server::terminal` into a new shared library crate inside the hero_router workspace, and how hero_proc would consume it. (Marked clearly "DESIGN PROPOSAL — see follow-up issue.") ## Implementation Plan ### Step 1 — UI filter on the Terminal tab **Files:** `crates/hero_proc_ui/templates/index.html` **Dependencies:** none (server-side schema unchanged) **What to do:** In `populateTerminalJobList` (currently at `index.html:662`), after the existing `jobs.filter(function (j) { return j.phase === 'running'; })` step and before the sort, add a second filter that intersects the running set with the cached action specs: - Look up each `job.name` (the action name in `JobSummary`) in the global `cachedActions` array (already populated by `dashboard.js`). - Keep only jobs whose matching action spec has `tty === true`. - If the action is not found in `cachedActions` (race during startup), fall through to **excluding** the job — this matches the conservative behaviour of the Job-detail panel which only shows the Terminal button when the action is found AND has tty. Also update the empty-option label from `-- Select a running job --` to `-- Select a running TTY job --` so the filter is visible to the user. **Rationale for UI-side over backend-side filter:** - `JobSummary` does not carry `tty` — the flag lives on `ActionSpec`. Adding it would require changes to `JobSummary`, `to_summary`, the SQL row reader, plus extending `JobFilter` with `tty: Option<bool>`. That is a much larger blast radius for a UI-only concern. - `cachedActions` is already loaded by the dashboard on startup; the join is cheap. - The Job-detail panel already uses exactly this `cachedActions.find(... === a.name)` pattern, so the filter is consistent with existing code. ### Step 2 — Verify the dropdown still auto-attaches correctly **Files:** none changed — manual smoke test only. After Step 1, reload the Terminal tab and confirm: - A non-TTY action's running job does NOT appear. - A TTY action's running job DOES appear and can be attached. - `navigateToTerminalJob(id)` still works because the caller already gates on `hasTty`. ### Step 3 — Write the Part-B design proposal **Files:** `docs/proposal-shared-terminal-lib.md` (new file) **Dependencies:** Step 1 is independent and can ship first. The document inventories actual duplication between hero_proc and hero_router and proposes a new crate `hero_terminal_lib` inside the hero_router workspace, exposing: - `session.rs` — `ShellType`, `TerminalSession`, `validate_name`, `encode_job_name`, `decode_job_name`, `shell_suffix`, `shell_from_suffix` - `manager.rs` — `create_session` / `list_sessions` / `get_session` / `delete_session` parameterised over a `hero_proc_sdk` client - `frame.rs` — typed `TerminalEvent` enum for the wire format - `ansi/mod.rs` — `strip_local_terminal_queries`, `strip_da_plaintext_artifacts`, `pending_query_prefix_len` plus their tests **What stays in hero_proc:** `PtyHandle`, the `portable_pty`-based supervisor, the `/api/jobs/{id}/pty` WS endpoint, `pty_handles` map, scrollback buffer. **What stays in hero_router:** `pty_proxy` and `attached()` registry remain as the WS-proxy concrete implementation, calling into the new crate for `ansi::*` and frame parsing. **Migration plan (for follow-up issue):** 1. Create `hero_terminal_lib`, move name/encoding/strip helpers + tests, repoint `hero_router` use sites. 2. Move session CRUD into `manager.rs`, parameterised over an injected `hero_proc_sdk` client. The `terminal.*` RPC handlers in `hero_router` become 5-line shims. 3. Optional: unify wire protocol — move hero_proc to typed `TerminalEvent` frames, or accept both indefinitely. ## Acceptance Criteria - [ ] Loading the Terminal tab in hero_proc admin UI lists ONLY running jobs whose action has `tty: true`. - [ ] A running job whose action has `tty: false` (or no `tty` field) is NOT present in the dropdown. - [ ] The Terminal-tab dropdown placeholder text reads `-- Select a running TTY job --`. - [ ] Clicking "Terminal" on the Job-detail panel for a TTY job continues to auto-attach correctly via `navigateToTerminalJob`. - [ ] `docs/proposal-shared-terminal-lib.md` exists and contains the duplication inventory and extraction shape. The file's first paragraph is `> DESIGN PROPOSAL — implementation tracked in follow-up issue.` - [ ] No changes to Rust code, schemas, or CI in this issue. ## Notes - **No backend change.** `JobSummary` and `JobFilter` stay as-is. Adding `tty` to either is left for a future cleanup if a backend filter is ever required. - The UI filter relies on `cachedActions` being populated. If empty (user lands directly on `#terminal/<id>`), the conservative behaviour is to show no jobs — preferable to showing all of them, because clicking a non-TTY job would error on attach anyway. - The Part-B proposal deliberately treats the PTY ownership layer (`PtyHandle`, scrollback, the WS endpoint) as not duplicated — only hero_proc owns these and hero_router proxies through them. - The follow-up issue must verify that hero_proc's CI can resolve a workspace dependency on `hero_terminal_lib` from the hero_router workspace via the standard CODEROOT-patch route.

despiegk commented 2026-05-01 04:35:44 +00:00

Author

Owner

Copy link

Test Results

Command: cargo test --workspace --no-fail-fast (via make test, then re-run with --no-fail-fast to enumerate all failures)
Total: 416
Passed: 414
Failed: 2
Ignored: 52

Failures

---- harness::tests::test_harness_starts_and_stops (tests/integration/src/harness.rs:372)
harness should start: server startup timeout
  panic: 'harness should start: server startup timeout'
  (hero_proc_server failed to come up within timeout window — flaky integration harness, environmental)

---- ready::tests::declare_creates_marker (crates/hero_proc_sdk/src/ready.rs:105)
assertion failed: is_ready(svc)
  (pre-existing assertion failure in ready-marker SDK test)

Doc-test compilation errors (pre-existing, unrelated)

Five doc-test targets fail to compile because rustdoc cannot resolve hero_proc_sdk / HeroProcRPCAPIClient symbols inside doctests:

• -p hero_proc_cli --doc (E0463: can't find crate for hero_proc_sdk)
• -p hero_proc_integration_test --doc (28 errors: unresolved HeroProcRPCAPIClient, etc.)
• -p hero_proc_integration_tests --doc (E0463: can't find crate for hero_proc_sdk)
• -p hero_proc_integration_tests --lib (the harness timeout above)
• -p hero_proc_sdk --lib (the ready marker assertion above)

All five failures are pre-existing on development and unrelated to this issue's changes.

Notes: this issue's changes are in crates/hero_proc_ui/templates/index.html (JS only) and docs/proposal-shared-terminal-lib.md (markdown). The cargo suite is run as a regression sanity check.

## Test Results Command: `cargo test --workspace --no-fail-fast` (via `make test`, then re-run with `--no-fail-fast` to enumerate all failures) Total: 416 Passed: 414 Failed: 2 Ignored: 52 ### Failures ``` ---- harness::tests::test_harness_starts_and_stops (tests/integration/src/harness.rs:372) harness should start: server startup timeout panic: 'harness should start: server startup timeout' (hero_proc_server failed to come up within timeout window — flaky integration harness, environmental) ---- ready::tests::declare_creates_marker (crates/hero_proc_sdk/src/ready.rs:105) assertion failed: is_ready(svc) (pre-existing assertion failure in ready-marker SDK test) ``` ### Doc-test compilation errors (pre-existing, unrelated) Five doc-test targets fail to compile because rustdoc cannot resolve `hero_proc_sdk` / `HeroProcRPCAPIClient` symbols inside doctests: - `-p hero_proc_cli --doc` (E0463: can't find crate for `hero_proc_sdk`) - `-p hero_proc_integration_test --doc` (28 errors: unresolved `HeroProcRPCAPIClient`, etc.) - `-p hero_proc_integration_tests --doc` (E0463: can't find crate for `hero_proc_sdk`) - `-p hero_proc_integration_tests --lib` (the harness timeout above) - `-p hero_proc_sdk --lib` (the ready marker assertion above) All five failures are pre-existing on `development` and unrelated to this issue's changes. Notes: this issue's changes are in `crates/hero_proc_ui/templates/index.html` (JS only) and `docs/proposal-shared-terminal-lib.md` (markdown). The cargo suite is run as a regression sanity check.

despiegk commented 2026-05-01 04:37:26 +00:00

Author

Owner

Copy link

Implementation Summary

Both parts of the spec are in place. Changes are local to this branch and not yet pushed.

Files changed

File	Type	Lines	Description
crates/hero_proc_ui/templates/index.html	modified	+7 / -1	TTY filter on the Terminal-tab dropdown; placeholder label changed to -- Select a running TTY job --
docs/proposal-shared-terminal-lib.md	created	74 lines	Part-B design proposal — duplication inventory and hero_terminal_lib extraction shape, marked as DESIGN PROPOSAL

Part A — UI filter

populateTerminalJobList in index.html now intersects the running-jobs list with the global cachedActions array, keeping only jobs whose action spec has tty === true. The defensive guard falls back to an empty actions array if cachedActions is undefined or not an array — in that case the dropdown is empty rather than full of non-TTY jobs that would fail to attach. Jobs whose action name is missing or absent from cachedActions are excluded for the same reason.

Part B — Design proposal

docs/proposal-shared-terminal-lib.md documents the shell/terminal/PTY duplication between hero_proc and hero_router and proposes a new hero_terminal_lib crate inside the hero_router workspace. The PTY ownership layer (PtyHandle, portable_pty supervisor, the /api/jobs/{id}/pty WS endpoint) explicitly stays in hero_proc. The session-management and wire-framing layer above it (session naming, ANSI/CSI scrubbing, typed TerminalEvent envelope, session CRUD) is the extraction target. A migration plan and out-of-scope list are included so a follow-up issue can implement it cleanly.

Tests

cargo test --workspace --no-fail-fast was run as a regression sanity check. Result: 414 passed, 2 failed, 52 ignored. The 2 failures (harness::tests::test_harness_starts_and_stops, ready::tests::declare_creates_marker) plus 5 doc-test compile errors are all pre-existing on development and unrelated to this issue's changes (which touch only an HTML template and a new markdown file — no Rust source). Full output captured above.

Acceptance Criteria

• Loading the Terminal tab in hero_proc admin UI lists ONLY running jobs whose action has tty: true.
• A running job whose action has tty: false (or no tty field) is NOT present in the dropdown.
• The Terminal-tab dropdown placeholder text reads -- Select a running TTY job --.
• Clicking "Terminal" on the Job-detail panel for a TTY job continues to auto-attach correctly via navigateToTerminalJob (caller pre-gates on hasTty, so the job will pass the new filter).
• docs/proposal-shared-terminal-lib.md exists and the duplication inventory and extraction shape are present. First paragraph is the > DESIGN PROPOSAL blockquote.
• No changes to Rust code, schemas, or CI in this issue.

Follow-up

A separate issue should track the actual extraction of hero_terminal_lib per the migration plan in docs/proposal-shared-terminal-lib.md.

## Implementation Summary Both parts of the spec are in place. Changes are local to this branch and not yet pushed. ### Files changed | File | Type | Lines | Description | | --- | --- | --- | --- | | `crates/hero_proc_ui/templates/index.html` | modified | +7 / -1 | TTY filter on the Terminal-tab dropdown; placeholder label changed to `-- Select a running TTY job --` | | `docs/proposal-shared-terminal-lib.md` | created | 74 lines | Part-B design proposal — duplication inventory and `hero_terminal_lib` extraction shape, marked as DESIGN PROPOSAL | ### Part A — UI filter `populateTerminalJobList` in `index.html` now intersects the running-jobs list with the global `cachedActions` array, keeping only jobs whose action spec has `tty === true`. The defensive guard falls back to an empty actions array if `cachedActions` is undefined or not an array — in that case the dropdown is empty rather than full of non-TTY jobs that would fail to attach. Jobs whose action name is missing or absent from `cachedActions` are excluded for the same reason. ### Part B — Design proposal `docs/proposal-shared-terminal-lib.md` documents the shell/terminal/PTY duplication between `hero_proc` and `hero_router` and proposes a new `hero_terminal_lib` crate inside the `hero_router` workspace. The PTY ownership layer (`PtyHandle`, `portable_pty` supervisor, the `/api/jobs/{id}/pty` WS endpoint) explicitly stays in `hero_proc`. The session-management and wire-framing layer above it (session naming, ANSI/CSI scrubbing, typed `TerminalEvent` envelope, session CRUD) is the extraction target. A migration plan and out-of-scope list are included so a follow-up issue can implement it cleanly. ### Tests `cargo test --workspace --no-fail-fast` was run as a regression sanity check. Result: 414 passed, 2 failed, 52 ignored. The 2 failures (`harness::tests::test_harness_starts_and_stops`, `ready::tests::declare_creates_marker`) plus 5 doc-test compile errors are all pre-existing on `development` and unrelated to this issue's changes (which touch only an HTML template and a new markdown file — no Rust source). Full output captured above. ### Acceptance Criteria - [x] Loading the Terminal tab in hero_proc admin UI lists ONLY running jobs whose action has `tty: true`. - [x] A running job whose action has `tty: false` (or no `tty` field) is NOT present in the dropdown. - [x] The Terminal-tab dropdown placeholder text reads `-- Select a running TTY job --`. - [x] Clicking "Terminal" on the Job-detail panel for a TTY job continues to auto-attach correctly via `navigateToTerminalJob` (caller pre-gates on `hasTty`, so the job will pass the new filter). - [x] `docs/proposal-shared-terminal-lib.md` exists and the duplication inventory and extraction shape are present. First paragraph is the `> DESIGN PROPOSAL` blockquote. - [x] No changes to Rust code, schemas, or CI in this issue. ### Follow-up A separate issue should track the actual extraction of `hero_terminal_lib` per the migration plan in `docs/proposal-shared-terminal-lib.md`.

despiegk referenced this issue 2026-05-01 05:00:02 +00:00

Extract hero_terminal_lib crate (PTY wire format, ANSI scrubbers, terminal envelope) #88

despiegk referenced this issue from a commit 2026-05-01 07:02:32 +00:00

feat(ui): filter terminal sessions to TTY-enabled only

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

development

main

lab-latest

v0.5.0-rc2

v0.5.0-rc1

v0.4.4

v0.4.1

v0.1.1

Labels

Clear labels   

prio_critical

  

prio_low

  

type_bug

  

type_contact

  

type_issue

  

type_lead

  

type_question

  

type_story

  

type_task

No labels

prio_critical

prio_low

type_bug

type_contact

type_issue

type_lead

type_question

type_story

type_task

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

ACTIVE

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

lhumina_code/hero_proc#71

No description provided.