Skip to content

fix: throwaway repros silently used the live session #2600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
HackAttack wants to merge 1 commit into
base: master
Choose a base branch
from
+43 −25
Open

fix: throwaway repros silently used the live session #2600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 41 additions & 24 deletions .agents/skills/herdr-throwaway-repro/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@ description: Create and control a disposable named Herdr session from inside an

Use a disposable named Herdr session when a reproduction needs a real Herdr server, panes, PTYs, agents, or socket API without risking the user's main session.

The temporary TUI only keeps the disposable session attached and supplies terminal geometry. Drive the reproduction from the parent session through Herdr's CLI/API. Do not manually operate the nested TUI unless the bug specifically requires client input.
Run that session as a headless server and drive it from the current session through Herdr's CLI/API. A headless server spawns real PTYs with no client attached, so most reproductions need no nested TUI and no extra pane in the user's session.

## Non-negotiable safety

- Never run the reproduction in the default session.
- If the disposable session cannot be started or addressed, stop and report. Never continue the reproduction in the current session, and never edit the user's `config.toml` to work around it.
- Never stop, restart, delete, or kill the main Herdr server.
- Never use `pkill`, broad process matching, or guessed PIDs for cleanup.
- Create a unique session name. Never reuse or delete an unrelated named session.
- Create a new outer pane and close only that pane during cleanup.
- Create a parent-session pane only when the reproduction needs an attached client, and close only that pane during cleanup.
- Read workspace, tab, pane, terminal, and agent IDs from command output. Never construct them.
- Use `/var/tmp` for reproduction directories and potentially large artifacts.
- Do not approve destructive or unnecessary agent actions.
Expand All @@ -40,49 +41,65 @@ Inspect nested command help before using unfamiliar or potentially mutating comm

Record which Herdr binary and version the reproduction tests. If testing a checkout build, follow the repository's instructions for running that build instead of silently substituting the installed binary.

## Create the outer pane
## Start the disposable session

Create a sibling shell pane in the current tab without moving focus. Use an available Herdr layout tool when the harness provides one. Otherwise use the installed pane split command after checking its help.
Choose a short unique name such as `repro-<topic>-<timestamp>`, then prove it is unused before launching:

Use `/var/tmp` or a dedicated reproduction directory as the new pane's cwd. Save the returned outer pane ID. This is the only parent-session pane that cleanup may close.
```bash
herdr session list --json
```

## Start the disposable session
That lists stopped sessions as well as running ones. A running name is refused, but starting a server on the name of a stopped session silently restores that session's saved workspaces and panes, and cleanup would then delete someone else's session. Pick another name on any exact match.

Choose a short unique name such as `repro-<topic>-<timestamp>`.
Start it as a headless server. `herdr --session <name>` launches the TUI, and launching the TUI from inside a Herdr pane exits with `nested herdr is disabled by default` unless the user enabled `experimental.allow_nested`. The `server` command has no such gate.

Run the named session inside the new outer pane. Clear inherited session selection, socket overrides, and caller IDs so the nested runtime cannot accidentally address the parent session:
The server runs until it is stopped and never returns on its own, so start it with the harness's background primitive. The launching shell otherwise blocks here and never reaches the rest of this workflow. Clear inherited socket overrides, session selection, and caller IDs so the new server binds its own session paths instead of the parent's:

```bash
# To be run as a background job
env \
-u HERDR_SOCKET_PATH \
-u HERDR_CLIENT_SOCKET_PATH \
-u HERDR_SESSION \
-u HERDR_WORKSPACE_ID \
-u HERDR_TAB_ID \
-u HERDR_PANE_ID \
herdr --session <session-name>
herdr --session <session-name> server
Comment on lines 60 to +67

@coderabbitai coderabbitai Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Validate and quote the session name before shell interpolation.

The suggested repro-<topic>-<timestamp> format does not constrain <topic>. When the placeholder is replaced with whitespace, shell metacharacters, /, or .., the commands can split arguments, execute unintended commands, or escape /var/tmp. Require a name such as ^[A-Za-z0-9][A-Za-z0-9_-]*$, and quote every session-name expansion.

Proposed validation pattern 
+case "$session_name" in
+  ''|*[!A-Za-z0-9_-]*) exit 1 ;;
+esac
+
-  herdr --session <session-name> server
+  herdr --session "$session_name" server

```
Comment on lines 58 to 68

@greptile-apps greptile-apps Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Server launch still blocks

When an agent executes this startup block literally, herdr --session <session-name> server runs synchronously and occupies the shell until shutdown, so the readiness check, workspace creation, and remaining reproduction steps cannot execute. Referring to an unspecified harness background primitive does not provide an executable nonblocking launch.


Add reproduction-specific environment variables to this launch command when needed. Environment variables that configure the server must be present before the named server starts.

Do not continue until the named session's API is ready. Confirm readiness by addressing that session from the parent and listing its panes.
That log's first lines name the api socket, client socket, and session log. Do not continue until `herdr session list` shows the name as running; the same check catches a server that died with its launching shell.

A headless session starts empty. Create the first workspace with `herdr --session <session-name> workspace create --cwd <dir>`; the returned root pane is the reproduction's first shell. With no client attached the shared runtime size is 80x24.

### When the reproduction needs an attached client

Only bugs in client rendering, input, or attach behavior need a real TUI. That requires a nested launch, so give the nested process its own config file instead of changing the user's:

```bash
printf '[experimental]\nallow_nested = true\n' > /var/tmp/<session-name>-config.toml
```
Comment on lines +80 to +82

@coderabbitai coderabbitai Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Create the nested-client config with an exclusive temporary path.

printf ... > /var/tmp/<session-name>-config.toml can follow an existing symlink or overwrite an unrelated file. Cleanup can then remove that file. Use mktemp, store the returned path, pass that path through HERDR_CONFIG_PATH, and remove only that exact path.

Proposed temporary-file handling 
-printf '[experimental]\nallow_nested = true\n' > /var/tmp/<session-name>-config.toml
+config_path="$(mktemp /var/tmp/herdr-repro-config.XXXXXX)"
+printf '[experimental]\nallow_nested = true\n' >"$config_path"

- HERDR_CONFIG_PATH=/var/tmp/<session-name>-config.toml
+ HERDR_CONFIG_PATH="$config_path"

- Remove /var/tmp/<session-name>-config.toml
+ Remove "$config_path"

Also applies to: 166-167


Create a sibling shell pane in the current tab without moving focus, using an available Herdr layout tool or the installed pane split command after checking its help, with `/var/tmp` as its cwd. That split is the one command in this workflow that is meant to reach the user's session, so run it without `--session`; adding the flag would put the pane inside the disposable session, where no client can reach it. Everything that drives the disposable session still requires `--session <session-name>`.

Save the returned outer pane ID; this is the only parent-session pane that cleanup may close. Attach inside that pane using the launch environment above with `server` dropped and `HERDR_CONFIG_PATH=/var/tmp/<session-name>-config.toml` added.

Never set `experimental.allow_nested` in the user's `config.toml`.

## Address only the disposable session

Every control command issued from the parent must clear inherited socket overrides and explicitly select the temporary session:
Select the session with the `--session` flag on every control command:

```bash
env \
-u HERDR_SOCKET_PATH \
-u HERDR_CLIENT_SOCKET_PATH \
-u HERDR_WORKSPACE_ID \
-u HERDR_TAB_ID \
-u HERDR_PANE_ID \
HERDR_SESSION=<session-name> \
herdr pane list
herdr --session <session-name> pane list
```

Repeat this prefix for every command. Do not rely on shell state persisting between tool calls.
The flag marks the session explicit, so Herdr ignores the `HERDR_SOCKET_PATH` inherited from the surrounding pane. Naming a session that is not running then fails with `server_not_running` instead of answering from the user's session.

The `HERDR_SESSION` environment variable does not do this. Inside a Herdr pane `HERDR_SOCKET_PATH` already points at the user's server and takes precedence over that variable, so `HERDR_SESSION=<session-name> herdr pane list` reads and mutates the user's session and reports success. A bare `herdr pane list` does the same. Treat any command without `--session` as aimed at the user's session.

Repeat the flag on every server-scoped command. Do not rely on shell state persisting between tool calls.
Comment on lines 90 to +102

@coderabbitai coderabbitai Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- inherited target identifiers ---'
rg -n -C 8 \
  'HERDR_(WORKSPACE_ID|TAB_ID|PANE_ID)|--session' \
  src tests docs || true

printf '%s\n' '--- target resolution ---'
rg -n -C 10 \
  'resolve.*(workspace|tab|pane)|current.*(workspace|tab|pane)|default.*(workspace|tab|pane)' \
  src tests || true

Repository: herdrdev/herdr

Length of output: 50371

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- relevant files ---'
git ls-files | rg '(^|/)(SKILL\.md|.*(cli|session|target|env).*)$' | head -200

printf '%s\n' '--- exact environment-variable references ---'
rg -n -S \
  'HERDR_(WORKSPACE_ID|TAB_ID|PANE_ID|SESSION|SOCKET_PATH)|session' \
  --glob '!src/ghostty/**' \
  --glob '!target/**' \
  --glob '!*.lock' \
  .agents src tests docs 2>/dev/null | head -1000

printf '%s\n' '--- skill context ---'
cat -n .agents/skills/herdr-throwaway-repro/SKILL.md | sed -n '70,115p'

Repository: herdrdev/herdr

Length of output: 50371

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- session selection and CLI setup ---'
cat -n src/session.rs | sed -n '1,190p'
cat -n src/cli.rs | sed -n '1,180p'

printf '%s\n' '--- target parsing and CLI command handlers ---'
cat -n src/app/terminal_targets.rs | sed -n '1,260p'
cat -n src/cli/pane.rs | sed -n '1,240p'
cat -n src/cli/workspace.rs | sed -n '1,180p'
cat -n src/cli/tab.rs | sed -n '1,180p'

printf '%s\n' '--- target environment names in current source ---'
rg -n -S \
  'HERDR_(WORKSPACE_ID|TAB_ID|PANE_ID|ACTIVE_WORKSPACE_ID|ACTIVE_TAB_ID|ACTIVE_PANE_ID)' \
  src tests .agents \
  --glob '!src/ghostty/**' \
  --glob '!vendor/**' || true

Repository: herdrdev/herdr

Length of output: 50370

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- all CLI uses of inherited caller IDs ---'
rg -n -C 6 \
  'HERDR_(WORKSPACE_ID|TAB_ID|PANE_ID)|parse_optional_current|--current|caller_pane_id' \
  src/cli/pane.rs src/cli/workspace.rs src/cli/tab.rs src/cli/runtime.rs src/cli.rs

printf '%s\n' '--- server-side current-target handling ---'
rg -n -C 10 \
  'PaneCurrent|caller_pane_id|workspace_id.*caller|current.*pane|public_pane_id|public_workspace_id|public_tab_id' \
  src/api src/app src/server \
  --glob '!**/ghostty/**' | head -1200

printf '%s\n' '--- ID generation and environment injection ---'
cat -n src/integration/env.rs | sed -n '1,180p'
cat -n src/pane.rs | sed -n '110,165p'
rg -n -C 8 \
  'fn (public_pane_id|public_workspace_id|public_tab_id)|public_pane_id\(' \
  src/app src | head -500

Repository: herdrdev/herdr

Length of output: 50371

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- exact target parser implementations ---'
cat -n src/cli/pane.rs | sed -n '227,295p;544,590p;623,680p;900,990p'
cat -n src/cli/runtime.rs | sed -n '1,180p'

printf '%s\n' '--- workspace and public-ID construction ---'
rg -n -C 8 \
  'pub fn (public_(workspace|tab|pane)_id_for_number)|public_(workspace|tab|pane)_id_for_number|struct Workspace|fn test_new|WorkspaceId' \
  src/workspace.rs src/app/ids.rs src | head -800

printf '%s\n' '--- focused static behavioral verifier ---'
python3 - <<'PY'
from pathlib import Path

session = Path("src/session.rs").read_text()
pane = Path("src/cli/pane.rs").read_text()

assert 'apply_explicit_name(&session)?;' in session
assert 'std::env::remove_var("HERDR_PANE_ID")' not in session
assert 'std::env::remove_var("HERDR_WORKSPACE_ID")' not in session
assert 'std::env::remove_var("HERDR_TAB_ID")' not in session

assert 'std::env::var("HERDR_PANE_ID")' in pane
assert '--current' in pane
print("explicit --session selection does not clear inherited target IDs")
print("HERDR_PANE_ID remains an input to pane current-target parsing")
PY

Repository: herdrdev/herdr

Length of output: 50370

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- workspace ID generation and cross-session uniqueness ---'
cat -n src/workspace.rs | sed -n '90,170p;1260,1315p'
rg -n -C 8 \
  'generate_workspace_id|NEXT_WORKSPACE|Atomic.*workspace|workspace.*id' \
  src/workspace.rs src | head -500

printf '%s\n' '--- API fallback for omitted and explicit pane targets ---'
rg -n -C 12 \
  'PaneLayout|PaneSplit|PaneInputSet|PaneCurrent|params\.pane_id|pane_id\.as|unwrap_or.*focused|focused.*pane' \
  src/api/server.rs src/server/headless.rs src/app | head -1200

printf '%s\n' '--- focused behavioral source verifier ---'
python3 - <<'PY'
from pathlib import Path

session = Path("src/session.rs").read_text()
pane = Path("src/cli/pane.rs").read_text()
ids = Path("src/workspace.rs").read_text()

checks = {
    "explicit session does not clear caller IDs": all(
        f'remove_var("HERDR_{name}")' not in session
        for name in ("WORKSPACE_ID", "TAB_ID", "PANE_ID")
    ),
    "pane current reads HERDR_PANE_ID": 'std::env::var("HERDR_PANE_ID")' in pane
        and 'caller_pane_id = env_pane_id.map' in pane,
    "pane split current reads HERDR_PANE_ID": 'ok_or("--current requires HERDR_PANE_ID")' in pane,
    "public pane IDs include workspace IDs": 'format!("{workspace_id}:p' in ids,
}
for label, result in checks.items():
    print(f"{label}: {result}")
assert all(checks.values())
PY

Repository: herdrdev/herdr

Length of output: 50371

Clear inherited caller IDs for disposable-session commands that use current targets.

--session selects the socket but does not clear HERDR_PANE_ID. Current-target selectors can resolve a parent-session ID to a different disposable-session pane. Run these commands with env -u HERDR_WORKSPACE_ID -u HERDR_TAB_ID -u HERDR_PANE_ID, or use explicit IDs returned by the disposable session.


Read the disposable root pane ID from `pane list`. Confirm its cwd and foreground process before starting anything in it.

Expand All @@ -92,7 +109,7 @@ Named sessions isolate runtime state, sockets, panes, and persistence. They stil

Use pane commands for shells and ordinary processes:

- `pane run` to start a command at an available shell prompt.
- `pane run <pane-id> <command>...` to start a command at an available shell prompt. The command follows the pane ID directly; an inserted `--` is typed into the shell and fails.
- `pane wait-output` to wait for deterministic output.
- `pane read` to capture terminal contents.
- `pane send-text` for literal input.
Expand Down Expand Up @@ -146,9 +163,9 @@ Cleanup is part of the reproduction, including after failure.
2. Verify that harmless probe files or other test artifacts do not exist, or remove only artifacts created by this reproduction.
3. Stop the temporary named session with the installed session command.
4. Delete that same stopped session.
5. Confirm it no longer appears as running.
6. Wait for the outer pane to return to its shell.
7. Close only the outer pane created by this workflow.
5. Confirm it no longer appears in `session list`.
6. Remove the temporary config file when one was written.
7. When an outer pane was created, wait for it to return to its shell and close only that pane.

Never delete another named session because it looks stale. Never close the pane running the current agent or any pane not created for the reproduction.

Expand Down
1 change: 1 addition & 0 deletions docs/next/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
### Fixed
- OpenCode panes now track the root conversation selected in their own TUI for native restore without adopting activity from attached clients. (#2450)
- Server stop requests now bypass pane and API traffic, preventing busy sessions from blocking shutdown or admitting a client while shutdown is pending. (#2612)
- The herdr-throwaway-repro skill is now isolated from the main session as intended. (#2600)
- Fish `Ctrl+Alt` keybindings now work in panes after legacy Alt-prefixed control bytes are decoded with both modifiers. (#2514)
- `herdr config check` now reports unknown built-in theme names instead of silently accepting them. (#2452)
- macOS `herdr --remote` clients now keep the accepted bridge socket blocking, preventing an immediate disconnect after the protocol handshake. (#2478, thanks @mathijshenquet)
Expand Down
2 changes: 1 addition & 1 deletion skills/herdr/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,5 +191,5 @@ After that failed read, ask the agent to write its complete response as Markdown
- Parse IDs from JSON responses. Do not derive them from sidebar order or examples.
- Do not close workspaces, tabs, panes, or sessions you did not create unless the user explicitly asked.
- Never run `herdr server stop` from an active session unless the user explicitly intends to stop the server and its pane processes.
- Never kill the main Herdr process. Use named test sessions for experiments that need an isolated server.
- Never kill the main Herdr process. For an isolated server, run `herdr --session <name> server`; the plain `herdr --session <name>` form launches the TUI, which is blocked inside a Herdr pane by default.
- CLI server errors are JSON on stderr with exit status 1. CLI syntax errors exit with status 2.
Loading