From b0846ee664e3be3aa1edd7dce5875506561d599d Mon Sep 17 00:00:00 2001 From: Tom Pepels Date: Thu, 23 Apr 2026 21:48:17 +0100 Subject: [PATCH 1/3] Add comprehensive audit report and implement associated tests - Introduced a detailed audit report for `schedule-agent` v0.2.0, highlighting architectural, security, feature, and UI issues. - Addressed critical blockers including non-atomic queue writes, missing `--dry-run` functionality, and incorrect TUI hints. - Implemented tests to ensure atomicity of job saving, proper permissions for sensitive directories, and correct behavior for new features like `--version` and `edit-prefix`. - Enhanced security by ensuring prompt prefix snapshots and validating job removal logic. - Improved user experience by aligning TUI hints with actual keybindings and adding status glyphs for clarity. --- .rtk/filters.toml | 13 + CLAUDE.md | 17 +- README.md | 23 +- install.sh | 25 +- promts/CLI_UPDATE_PLAN.md | 570 ++++++++++++++++++++++++++++ reports/audits/combined.md | 318 ++++++++++++++++ schedule_agent/cli.py | 335 ++++++++++------ schedule_agent/environment.py | 37 +- schedule_agent/execution.py | 23 +- schedule_agent/operations.py | 141 ++++++- schedule_agent/persistence.py | 21 +- schedule_agent/preflight.py | 44 ++- schedule_agent/scheduler_backend.py | 39 +- schedule_agent/time_utils.py | 4 +- tests/test_audit_fixes.py | 354 +++++++++++++++++ tests/test_cli_doctor.py | 2 +- tests/test_environment.py | 20 +- tests/test_execution.py | 25 +- tests/test_operations_preflight.py | 2 +- tests/test_preflight.py | 6 +- 20 files changed, 1830 insertions(+), 189 deletions(-) create mode 100644 .rtk/filters.toml create mode 100644 promts/CLI_UPDATE_PLAN.md create mode 100644 reports/audits/combined.md create mode 100644 tests/test_audit_fixes.py diff --git a/.rtk/filters.toml b/.rtk/filters.toml new file mode 100644 index 0000000..d9bd43f --- /dev/null +++ b/.rtk/filters.toml @@ -0,0 +1,13 @@ +# Project-local RTK filters — commit this file with your repo. +# Filters here override user-global and built-in filters. +# Docs: https://github.com/rtk-ai/rtk#custom-filters +schema_version = 1 + +# Example: suppress build noise from a custom tool +# [filters.my-tool] +# description = "Compact my-tool output" +# match_command = "^my-tool\\s+build" +# strip_ansi = true +# strip_lines_matching = ["^\\s*$", "^Downloading", "^Installing"] +# max_lines = 30 +# on_empty = "my-tool: ok" diff --git a/CLAUDE.md b/CLAUDE.md index fc634cc..5e5e8ad 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,10 +1,4 @@ -# Working rules - -See [AGENTS.md](AGENTS.md) for agent workflow rules. In particular: **never push -without a green `make check` locally** — CI runs the same target and will fail -on any ruff/pytest regression. - # RTK (Rust Token Killer) - Token-Optimized Commands ## Golden Rule @@ -33,11 +27,16 @@ rtk prettier --check # Files needing format only (70%) rtk next build # Next.js build with route metrics (87%) ``` -### Test (90-99% savings) +### Test (60-99% savings) ```bash rtk cargo test # Cargo test failures only (90%) -rtk vitest run # Vitest failures only (99.5%) +rtk go test # Go test failures only (90%) +rtk jest # Jest failures only (99.5%) +rtk vitest # Vitest failures only (99.5%) rtk playwright test # Playwright failures only (94%) +rtk pytest # Python test failures only (90%) +rtk rake test # Ruby test failures only (90%) +rtk rspec # RSpec test failures only (60%) rtk test # Generic test wrapper - failures only ``` @@ -136,4 +135,4 @@ rtk init --global # Add RTK to ~/.claude/CLAUDE.md | Network | curl, wget | 65-70% | Overall average: **60-90% token reduction** on common development operations. - + \ No newline at end of file diff --git a/README.md b/README.md index e975a00..6c3de62 100644 --- a/README.md +++ b/README.md @@ -136,7 +136,12 @@ schedule-agent session # attach a session schedule-agent session --new # reset to a fresh session schedule-agent delete -schedule-agent --dry-run # preview without submitting + +schedule-agent --dry-run # TUI: creating a job shows preview, skips submission +schedule-agent --dry-run submit # preview the at(1) script for an existing job + +schedule-agent edit-prefix {claude|codex} # edit the per-agent prompt prefix in $EDITOR +schedule-agent --version ``` ### Safe mutations @@ -171,10 +176,22 @@ export EDITOR="code --wait" | Path | Contents | |------|----------| -| `~/.local/state/schedule-agent/` | job queue + state | +| `~/.local/state/schedule-agent/` | job queue + state + logs | | `~/.local/share/schedule-agent/agent_prompts/` | prompt files | +| `~/.config/schedule-agent/prompt-prefix-{claude,codex}.md` | per-agent prefix applied to every scheduled prompt | + +`$XDG_STATE_HOME`, `$XDG_DATA_HOME`, and `$XDG_CONFIG_HOME` are honoured if set. + +State, logs, and prompt dirs are chmod-ed to `0700` on creation — logs may contain secrets produced by the agent. + +### Environment variables -`$XDG_STATE_HOME` and `$XDG_DATA_HOME` are honoured if set. +| Variable | Effect | +|----------|--------| +| `SCHEDULE_AGENT_EDITOR` | Editor for prompt/prefix editing (wins over `$EDITOR`) | +| `SCHEDULE_AGENT_STALE_MINUTES` | Minutes a `running` job must be idle (no log writes) before the recovery path force-marks it failed. Default `60`, minimum `1`. | +| `SCHEDULE_AGENT_POST_HOOK` | Optional shell command fired after every job finishes. Receives `JOB_ID`, `JOB_TITLE`, `JOB_RESULT` (`success`/`failed`), `JOB_EXIT_CODE`, `JOB_LOG_FILE` in its environment. Failures are swallowed. | +| `SCHEDULE_AGENT_MIN_CLAUDE` / `SCHEDULE_AGENT_MIN_CODEX` | Override the preflight minimum known-good version per agent. | --- diff --git a/install.sh b/install.sh index 743d054..d022b89 100755 --- a/install.sh +++ b/install.sh @@ -1,3 +1,6 @@ +#!/usr/bin/env bash +set -euo pipefail + check_atd_running() { if pgrep -x atd >/dev/null 2>&1; then return 0 @@ -12,7 +15,6 @@ check_atd_running() { exit 1 } -check_atd_running check_prereq() { local cmd="$1" local pkg="$2" @@ -22,21 +24,21 @@ check_prereq() { fi } -# Check prerequisites +check_atd_running -# Find a suitable Python 3 interpreter (>=3.7) +# Find a suitable Python 3 interpreter (>=3.10) PYTHON_BIN="" for candidate in python3 python; do if command -v "$candidate" >/dev/null 2>&1; then ver=$($candidate -c 'import sys; print("%d.%d" % sys.version_info[:2])' 2>/dev/null) case "$ver" in - 3.[7-9]|3.1[0-9]|[4-9].*) + 3.1[0-9]|3.[2-9][0-9]|[4-9].*) PYTHON_BIN="$candidate"; break;; esac fi done if [[ -z "$PYTHON_BIN" ]]; then - echo "Error: Python 3.7+ is required (python3 or python not found or too old)." >&2 + echo "Error: Python 3.10+ is required (python3 or python not found or too old)." >&2 exit 1 fi @@ -44,9 +46,6 @@ check_prereq pip3 "pip for Python 3" check_prereq at "at (job scheduler)" check_prereq atd "atd (daemon)" -#!/usr/bin/env bash -set -e - PREFIX="${HOME}/.local" BIN_DIR="${PREFIX}/bin" APP_DIR="${PREFIX}/share/schedule-agent" @@ -58,7 +57,6 @@ is_installed() { if [[ -x "$SCHEDULE_AGENT_BIN" ]]; then return 0 fi - # Also check if installed via pip in user or system if "$PYTHON_BIN" -m pip show schedule-agent >/dev/null 2>&1; then return 0 fi @@ -68,16 +66,13 @@ is_installed() { if is_installed; then echo "schedule-agent is already installed." echo "Updating to the latest version..." - # Try to update via pip if installed as a package if "$PYTHON_BIN" -m pip show schedule-agent >/dev/null 2>&1; then "$PYTHON_BIN" -m pip install --upgrade --user schedule-agent echo "schedule-agent updated via pip." exit 0 fi - # Otherwise, update the local install (reinstall files) echo "Updating local install..." rm -rf "$APP_DIR/schedule_agent" "$APP_DIR/pyproject.toml" "$VENV_DIR" - # Continue to install as below else echo "schedule-agent not found. Installing..." fi @@ -85,20 +80,16 @@ fi mkdir -p "$BIN_DIR" mkdir -p "$APP_DIR" -# Copy project files cp -r schedule_agent "$APP_DIR/" cp pyproject.toml "$APP_DIR/" cp README.md "$APP_DIR/" 2>/dev/null || true cp LICENSE "$APP_DIR/" 2>/dev/null || true -# Create venv "$PYTHON_BIN" -m venv "$VENV_DIR" -# Install package into the venv "$VENV_DIR/bin/pip" install --upgrade pip "$VENV_DIR/bin/pip" install "$APP_DIR" -# Create launcher cat > "$BIN_DIR/schedule-agent" < **For agentic workers:** REQUIRED SUB-SKILL: Use `superpowers:subagent-driven-development` (recommended) or `superpowers:executing-plans` to implement this plan phase-by-phase. Steps use checkbox (`- [ ]`) syntax for tracking. + +## Goal + +Make the jobs screen the single authoritative interactive surface for job management. + +The fullscreen jobs screen must support all common actions directly inside the screen, without launching separate modal mini-apps that call `.run()` against prompt_toolkit dialogs from inside the running TUI. + +The jobs screen must become structurally simple, width-aware, and comfortable in a normal terminal. + +## Required outcome + +After this refactor: + +* [ ] The fullscreen jobs screen is the primary interactive UI. +* [ ] All job actions are initiated and completed from inside the jobs screen. +* [ ] The jobs screen no longer mixes fullscreen TUI control flow with prompt_toolkit dialog `.run()` calls from helper functions. +* [ ] The jobs screen no longer uses `asyncio.run()` inside the active TUI event loop. +* [ ] The jobs list is compact and width-aware. +* [ ] The left pane is a summary list, not a pipe-delimited fake table. +* [ ] Secondary metadata moves to the detail pane. +* [ ] The UI works at normal terminal widths without requiring an unusually wide screen. +* [ ] Message handling is consistent and not overwritten by generic success text. +* [ ] Legacy mixed-interaction behavior is removed rather than preserved. + +## Product decision + +The jobs screen is a real application surface, not a wrapper around a chain of separate prompt dialogs. + +That means the following patterns are no longer valid inside the jobs screen: + +* calling `input_dialog(...).run()` from a keybinding +* calling `yes_no_dialog(...).run()` from a keybinding +* calling `radiolist_dialog(...).run()` from a keybinding +* returning either a bool or a coroutine depending on loop state +* calling `asyncio.run()` from a TUI action +* using ad hoc terminal handoff logic to patch over the above + +These patterns may remain available for non-TUI CLI flows if needed, but **must not be used by the fullscreen jobs screen**. + +## Root problems to fix + +### 1. Mixed interaction models + +The current file combines: + +* noninteractive command CLI +* modal prompt/dialog helpers +* fullscreen TUI + +The fullscreen TUI currently calls into helpers designed for standalone dialog workflows. This creates conflicting control flow and brittle behavior. + +### 2. Broken event-loop model + +The current jobs screen defines async action plumbing but then calls it from synchronous key handlers without awaiting or scheduling it consistently. The delete flow then tries to force execution with `asyncio.run()` inside the running application loop. + +This must be removed. + +### 3. Table rendering is fake and too wide + +The left pane currently renders pipe-delimited strings as if they were a table. This causes: + +* header/row misalignment +* unstable widths +* poor truncation behavior +* excessive minimum terminal width + +The left pane should instead be a compact summary list with fixed-width rendering rules. + +### 4. Status/message handling is muddy + +Actions set detailed messages, then a generic wrapper may overwrite them with `Updated.` or similar. User feedback must come from one authority. + +### 5. Legacy behavior should not be preserved + +This is pre-alpha. There is no need to preserve mixed legacy UI patterns. Remove obsolete behavior instead of layering compatibility code over it. + +## Design rules + +### Jobs screen ownership + +The fullscreen jobs screen owns: + +* selection +* filtering +* confirmation +* temporary input prompts +* session selection +* status messages +* refresh behavior +* post-action redraw + +### Single interaction model inside the TUI + +Inside the jobs screen, there must be one consistent model: + +* synchronous action dispatch from keybindings +* app-owned overlays / inline prompts / inline confirm UI +* explicit refresh/invalidate after state changes + +Do not mix in separate prompt_toolkit mini-apps. + +### Compact information hierarchy + +The left pane is for quick scanning and navigation only. + +The right pane is for detailed metadata and full summary. + +Do not try to display every field in the left pane. + +### Width-aware UI + +The jobs screen must adapt to narrower terminals. + +Minimum expectation: + +* narrow width: summary pane only, detail view toggleable +* medium width: summary + detail split +* wide width: richer summary columns + +### Remove legacy, do not phase-preserve it + +If old helper patterns or layout behaviors conflict with the above, remove them. + +## Target screen behavior + +## Main layout + +The jobs screen should have 3 regions: + +1. Header bar +2. Main body +3. Footer/status bar + +### Header bar + +Shows concise global state: + +* app name or screen name +* current filter +* local timezone +* optional count summary + +Example shape: + +`Jobs Filter: Active TZ: WEST 8 items` + +### Main body + +The main body should render one of these modes depending on available width: + +#### Mode A: split view + +* left: job summary list +* right: selected job detail + +#### Mode B: list-only view + +* full width summary list +* enter/tab toggles detail overlay or detail mode for selected job + +### Footer/status bar + +Footer shows: + +* key hints +* current contextual status message + +Keep hints compact and stable. + +## Left pane redesign + +Replace the current pipe-delimited fake table with a structured summary row renderer. + +### Left pane columns + +Default summary fields should be limited to: + +* title +* state/status + n- scheduled time +* agent/session marker + +Optional fifth compact field if width allows: + +* queue/submission marker + +### Fields that must move out of the left pane + +Move these to detail only: + +* created time +* updated time +* dependency id +* full scheduler label +* full prompt path +* full cwd +* verbose lifecycle fields + +### Row behavior + +Each row must: + +* truncate predictably +* use fixed column widths per active layout mode +* visually distinguish selected row +* visually soften completed/removed/invalid items +* not rely on pipe separators for alignment + +### Recommended row rendering approach + +Implement a width-aware formatter that returns one formatted row string from a defined column spec. + +For example, define summary column policies such as: + +* title: flex, min 18, ideal 28, max 40 +* status: fixed 10 +* run_at: fixed 16 +* agent/session: fixed 12 + +Then derive actual widths from terminal width. + +## Detail pane redesign + +The detail pane becomes the authoritative full job inspector. + +It should show: + +* job id +* title +* current display state +* scheduler/submission state +* scheduled time +* created time +* updated time +* agent +* session mode / session id +* cwd +* dependency info +* prompt file path +* queue id / at job id when relevant +* full formatted summary text if useful + +The detail pane can be verbose because it only serves the selected item. + +## Actions that must remain inside the jobs screen + +The jobs screen must support these actions directly: + +* [ ] new job +* [ ] edit prompt +* [ ] reschedule +* [ ] change session +* [ ] unschedule +* [ ] submit / repair +* [ ] retry +* [ ] delete +* [ ] refresh +* [ ] cycle filter +* [ ] toggle detail / responsive mode behavior + +## Input model for actions + +For actions that require extra input, use app-owned overlays or inline prompt components. + +### Required overlays / inline prompts + +* [ ] confirm delete +* [ ] input schedule spec +* [ ] session picker +* [ ] transient informational message area + +Do not use standalone dialog helpers from the fullscreen app. + +## Action architecture + +## Replace async action wrapper with synchronous dispatch + +The jobs screen should use a single synchronous action dispatcher. + +Recommended shape: + +* keybinding handler calls a local action function +* action function performs synchronous operation +* action function returns a user-facing message string or raises `OperationError` +* dispatcher stores message, refreshes view, invalidates app + +### Rules + +* no `asyncio.run()` +* no return type that changes between bool/coroutine +* no un-awaited coroutine creation in key handlers +* no action wrapper that overwrites specific messages with generic text + +### Message model + +Every action should either: + +* return a success message +* raise `OperationError` +* raise `KeyboardInterrupt` / explicit cancellation signal + +The dispatcher alone should convert those into footer/status text. + +## Confirmation model + +Delete confirmation must happen inside the jobs screen. + +Implement one of these patterns: + +* a centered confirm overlay with Yes/No keybindings +* an inline footer prompt like `Delete job abc123? [y/N]` + +Do not call standalone confirm dialogs from the active TUI. + +## Session selection model + +Session choice must also happen inside the jobs screen. + +Do not call a separate radiolist dialog from the TUI. + +Recommended behavior: + +* open a session picker overlay +* show `New session` plus discovered sessions +* filterable or scrollable if needed +* confirm selection and return to jobs screen + +## Prompt editing model + +Editing in `$EDITOR` is allowed, but must be treated as a controlled suspend/resume operation. + +Required behavior: + +* [ ] suspend/leave the visual screen cleanly +* [ ] launch editor on the prompt file +* [ ] resume the jobs screen +* [ ] refresh prompt metadata/state +* [ ] show result message + +This must be implemented as a dedicated TUI-safe path, not by reusing legacy modal helper logic. + +## New job flow inside the jobs screen + +The new job flow must stay within the jobs screen UX. + +At minimum it should support: + +* agent selection +* session selection (new/resume) +* schedule input +* prompt editing / prompt capture +* submit on creation + +This can be a staged overlay flow inside the same application. + +It does not need to be perfect on the first pass, but it must no longer rely on nested standalone dialogs launched from inside the fullscreen app. + +## Concrete refactor requirements + +## Phase 1: stabilize the event model + +* [ ] Remove the async `run_action` pattern from the jobs screen. +* [ ] Remove any `asyncio.run()` usage from jobs screen action paths. +* [ ] Make all jobs screen actions synchronous from the TUI’s perspective. +* [ ] Remove any helper that returns a coroutine sometimes and a plain value other times for TUI use. +* [ ] Ensure every keybinding action fully executes or cleanly cancels without loop errors. + +### Phase 1 acceptance criteria + +* [ ] Delete no longer crashes with event-loop errors. +* [ ] No jobs screen action relies on `asyncio.run()`. +* [ ] No TUI keybinding silently drops an un-awaited coroutine. + +## Phase 2: stop using standalone dialogs inside the fullscreen app + +* [ ] Identify all jobs screen flows that call prompt_toolkit `.run()` dialogs indirectly or directly. +* [ ] Replace them with jobs-screen-owned overlays or inline prompts. +* [ ] Ensure confirm, input, and list selection are handled inside the TUI. + +### Phase 2 acceptance criteria + +* [ ] Delete confirmation is TUI-native. +* [ ] Reschedule input is TUI-native. +* [ ] Session selection is TUI-native. +* [ ] Filter control is TUI-native. +* [ ] No jobs screen action depends on `input_dialog`, `yes_no_dialog`, `radiolist_dialog`, or `message_dialog`. + +## Phase 3: redesign the left pane + +* [ ] Remove the current pipe-delimited header/body rendering model for the left pane. +* [ ] Implement a compact summary row renderer with explicit column widths. +* [ ] Reduce the summary fields to a small scan-friendly set. +* [ ] Move secondary metadata to the detail pane. +* [ ] Add truncation/ellipsis behavior where needed. + +### Phase 3 acceptance criteria + +* [ ] Header and rows visually align. +* [ ] The screen is usable in a normal terminal width. +* [ ] The left pane remains readable without horizontal sprawl. + +## Phase 4: responsive layout behavior + +* [ ] Add width-aware layout selection. +* [ ] Support at least list-only mode and split mode. +* [ ] Add a keybinding to toggle detail when in narrow mode, if automatic split is not possible. +* [ ] Ensure the layout does not assume an excessively wide terminal. + +### Phase 4 acceptance criteria + +* [ ] Narrow terminals remain usable. +* [ ] The user can still access detail information without widening the screen excessively. + +## Phase 5: unify messaging and action feedback + +* [ ] Centralize success/error/cancel message handling. +* [ ] Remove generic action wrapper text that overwrites specific messages. +* [ ] Ensure each action returns a precise, user-facing result. + +### Phase 5 acceptance criteria + +* [ ] Footer/status text consistently reflects the last action. +* [ ] Specific messages are preserved. + +## Phase 6: remove conflicting legacy TUI behavior + +* [ ] Remove TUI code paths that depend on mixed dialog/TUI assumptions. +* [ ] Remove obsolete helper usage from the fullscreen jobs screen. +* [ ] Prefer deleting dead or conflicting code over preserving compatibility. + +### Phase 6 acceptance criteria + +* [ ] The jobs screen code path is internally consistent. +* [ ] No legacy mixed-surface behavior remains in the fullscreen TUI. + +## Non-goals + +These are not required for this refactor unless directly needed by the implementation: + +* redesigning argparse command structure +* splitting the file into modules right now +* changing backend job semantics +* changing persistence format +* building a mouse-driven UI +* preserving pre-alpha legacy UI quirks + +## Implementation guidance + +## Keep one file for now + +Implement this in the current file if that is the fastest path. + +Do not block the refactor on code organization purity. The priority is to make the jobs screen correct and coherent first. + +## But structure the code for later extraction + +Even in one file, create clearly separated sections for: + +* TUI state/model +* summary row formatting +* detail formatting +* overlay/input state +* synchronous action dispatch +* keybindings +* layout selection + +This prepares the later split without delaying the refactor. + +## Suggested internal architecture in one file + +Recommended logical sections: + +1. `JobsScreenState` + + * selected index + * filter + * message + * current mode + * overlay state + * cached jobs if needed + +2. `SummaryRowRenderer` + + * width policy + * row formatting + * header formatting + +3. `DetailRenderer` + + * selected job detail formatting + +4. `OverlayState` + + * none / confirm / text input / picker / new-job flow + +5. `dispatch_action(...)` + + * execute action + * catch exceptions + * set message + * refresh invalidate + +6. `jobs_menu()` + + * build layout + * bind keys + * run app + +## Visual design guidance + +The user wants terminal menus with a black background and a more modern feel. + +Within prompt_toolkit styling constraints: + +* use dark background as the baseline +* use restrained contrast +* highlight selection cleanly +* avoid noisy separators +* keep the footer compact +* keep the header calm and informative + +Do not solve visual modernity with more text density. + +## Behavior guidance for list density + +When listing jobs, sessions, or similar items, always show enough information to scan meaningfully. + +For jobs in the main list, that means the user should be able to see at a glance: + +* what the job is +* whether it is active/completed/failed/invalid +* when it will run or ran +* which agent/session context it belongs to + +But this must be done compactly. + +## Removal policy + +Because this project is pre-alpha, prefer the following policy: + +* [ ] if an old TUI behavior conflicts with the new jobs-screen-first design, remove it +* [ ] if a helper exists only to bridge incompatible interaction models, remove or stop using it from the TUI +* [ ] do not keep compatibility code just because it existed first + +## Final acceptance test checklist + +* [ ] Start the jobs screen in a normal-width terminal and verify it is usable without excessive width. +* [ ] Navigate up/down through jobs. +* [ ] Cycle filters. +* [ ] Open selected job details. +* [ ] Reschedule a job entirely from the jobs screen. +* [ ] Change session entirely from the jobs screen. +* [ ] Unschedule from the jobs screen. +* [ ] Submit/repair from the jobs screen. +* [ ] Retry from the jobs screen. +* [ ] Delete from the jobs screen without event-loop errors. +* [ ] Edit prompt and return cleanly to the jobs screen. +* [ ] Confirm specific status messages appear after actions. +* [ ] Confirm there are no dialog `.run()` calls used by the fullscreen jobs screen path. +* [ ] Confirm there is no `asyncio.run()` in the fullscreen jobs screen path. +* [ ] Confirm obsolete mixed-interaction TUI code has been removed. + +## Definition of done + +This refactor is done when the jobs screen is a coherent standalone interface that manages jobs directly, works in a normal terminal, and no longer depends on mixed modal-dialog behavior or broken async bridging. + +At that point, the code can be split into separate modules later without changing the interaction model again. diff --git a/reports/audits/combined.md b/reports/audits/combined.md new file mode 100644 index 0000000..5dbb535 --- /dev/null +++ b/reports/audits/combined.md @@ -0,0 +1,318 @@ +# COMBINED AUDIT + +Scope: `schedule-agent` v0.2.0 — a Python CLI/TUI that queues Claude/Codex invocations onto the system `at(1)` scheduler. Linux-only, single-user, local. ~4,600 LOC source + ~3,000 LOC tests. + +## EXECUTIVE SUMMARY + +The project has a clean domain model (state_model / transitions / operations) and decent preflight tooling. Real release blockers are narrow and concrete: + +1. `save_jobs` rewrites the queue jsonl non-atomically — a crash mid-write corrupts every user's job queue. +2. Documented CLI flag `--dry-run` is not wired through `argparse` — the README lies. +3. The empty-state hint tells users to press `N` to create a job, but `N` = "run now"; `A` = add. First-run users hit a dead end. +4. The TUI (1,200+ lines inside `cli.py`) has almost no automated coverage. + +The product is otherwise coherent: invariants are checked on every write, the `at`-queue roundtrip is covered by `doctor --roundtrip`, shell-quoting of user-controlled strings is consistent. By-design risk (running agents with `--dangerously-skip-permissions` unattended) is clearly documented in the README. + +Verdict: **not ready** — see blocker list. + +--- + +## ARCHITECTURE + +### BLOCKERS + +**Non-atomic queue write** +- type: architectural +- severity: blocker +- evidence: `schedule_agent/persistence.py:106-112` — `save_jobs` calls `queue_file.write_text(...)`. No tempfile-then-rename, no fsync. fcntl lock in `operations.py:81-88` serializes writers but does not protect against mid-write crashes (SIGKILL, power loss, OOM). +- risk: partial write corrupts `agent_queue.jsonl`; `load_jobs` then marks every record `_invalid` or loses records entirely. No backup. Users lose their whole queue including scheduled `at_job_id` bindings, leaving orphan `at` jobs whose callbacks no longer match. +- fix: write to `queue_file.with_suffix(".jsonl.tmp")`, `os.replace` onto target, optional `fsync`. Keep the existing fcntl lock. + +### FINDINGS + +**`cli.py` is a 2,209-line grab bag** +- type: code quality +- severity: high +- evidence: `schedule_agent/cli.py` holds argparse, all non-TUI command handlers, the full `prompt_toolkit` jobs screen (state, rendering, keybindings, overlays), column-layout helpers, and the staged new-job flow. +- risk: any TUI change has to round-trip the whole file; unit testability is low (see tests section); new contributors can't find anything. Structural drift as the TUI grows. +- fix: split at least `cli.py` into `cli/commands.py` (argparse + non-interactive handlers), `cli/tui/screen.py`, `cli/tui/render.py`, `cli/tui/actions.py`. No behavior change required. + +**Legacy compatibility leaks out of `legacy/`** +- type: architectural +- severity: medium +- evidence: `persistence.py:40-62,132-141`, `scheduler_backend.py:161-166`, `time_utils.py:74-79` are thin wrappers that re-export `legacy/compat.py`. `cli.py:19-20,52-56` still exposes `load_state/save_state/set_state/clear_state/cancel_at_job` at the module level. +- risk: the stated `schedule_agent.legacy` boundary is decorative. Any "remove legacy" task has to touch the core modules, which re-introduces the risk of breaking live job records. +- fix: make the wrappers raise `DeprecationWarning` at import time, or delete them and inline the two or three real call sites. + +**Module-level side effects at import** +- type: architectural +- severity: medium +- evidence: `cli.py:72` — `STATE_DIR, DATA_DIR, QUEUE_FILE, STATE_FILE, PROMPT_DIR = _make_paths()` runs `_ensure_dirs()` at import, which `mkdir -p`s four XDG locations before `main()` is even called. +- risk: importing `schedule_agent.cli` (e.g. from a test, a packaging tool, `schedule-agent --help`) creates directories in the user's home. Breaks hermetic tests, surprises packagers, and couples import order to filesystem state. +- fix: move directory preparation to `main()` / lazy accessors. + +**Known-good agent versions hardcoded** +- type: operational +- severity: medium +- evidence: `environment.py:11-14` pins `claude=2.1.112`, `codex=0.120.0`. +- risk: every upstream bump produces a WARN in `doctor` output. Users will learn to ignore it, which defeats the purpose of the check. +- fix: widen to a version range (minimum known-good), or move the list to config and refresh it via CI. + +**Stale-running recovery threshold via env only** +- type: operational +- severity: low +- evidence: `operations.py:91-97` reads `SCHEDULE_AGENT_STALE_MINUTES`, defaults to 60. +- risk: undocumented (not in README). A long but quiet agent run (64 minutes with no log writes) will be force-marked failed mid-execution. +- fix: document in README; consider using log-mtime only (not the started-at wall clock) as the primary signal. + +--- + +## SECURITY + +### BLOCKERS + +_None._ The product deliberately runs agents with `--dangerously-bypass-approvals-and-sandbox` / `--dangerously-skip-permissions`; this is documented as the threat model, not a vulnerability. + +### FINDINGS + +**Prompt-prefix file read at execution time via shell concat** +- category: injection / tampering +- type: risk +- severity: medium +- evidence: `execution.py:29-38` — script line is literally `"$(cat $prefix 2>/dev/null; echo; cat $prompt)"`. The prefix is resolved when `atd` fires the job, not when the user scheduled it. +- attack: any process with write access to `~/.config/schedule-agent/prompt-prefix-claude.md` (e.g. a hostile dependency dropped into a venv the user ran once) can substitute a malicious prefix between schedule-time and run-time and exfiltrate via the agent's tool use. Because the run happens unattended, the user never reviews it. +- fix: snapshot the prefix into the prompt file (or a sibling file) at submit time; the script `cat`s that immutable snapshot. The prefix edit UI remains, but edits affect future jobs only. + +**Log files may contain agent-generated secrets** +- category: data exposure +- type: hardening +- severity: medium +- evidence: `scheduler_backend.py:76` — `exec >>"$log_file" 2>&1` captures stdout+stderr of the agent under `~/.local/state/schedule-agent/logs//`. No rotation, no redaction, default 0644 create perms inherited from umask. +- attack: any other local user reads `~/.local/state/...` if umask is permissive or the home dir mode is 0755 (default on many distros). Agents regularly echo API keys, credentials, repo paths into output. +- fix: `os.umask(0o077)` or `chmod 0700` the logs dir; document that logs may contain secrets; offer a retention policy (delete logs on job delete — currently `_apply_scheduler_mutation` does delete the directory, good, but orphan jobs leave logs). + +**`at -t` output parsing relies on forced C locale only** +- category: injection / parser +- type: hardening +- severity: low +- evidence: `scheduler_backend.py:28-32` sets `LC_ALL=C, LANG=C`. `parse_at_job_id` regex `\bjob\s+(\d+)\s+at\b`. +- attack: if a vendor `at` fork emits a different phrasing ("job #42 scheduled at..."), `submit_job` raises `"Could not determine at job id"`. Failure mode is a thrown RuntimeError, not a silent mis-binding, so it is self-healing — but the critical dependency on exact wording is not tested across `at` implementations. +- fix: fall back to `atq` diff (list before, list after, take the new id) when regex fails. + +**No verification that `at_job_id` still belongs to us before `atrm`** +- category: abuse / race +- type: risk +- severity: low +- evidence: `scheduler_backend.py:122-128` — `remove_at_job` calls `atrm ` by number only. +- attack: if the stored `at_job_id` ever collides with a later unrelated `at` job (e.g. after atd restart + id reuse, or manual `atrm`), we remove the wrong job. `query_atq_entry` would notice the mismatch but `remove_at_job` doesn't consult it. +- fix: compare `entry.owner` / `entry.queue` against expectation before removing; treat absence as success without calling `atrm`. + +**`install.sh` runs prerequisite checks above `set -e`** +- category: install-time integrity +- type: hardening +- severity: low +- evidence: `install.sh:1-45` executes `check_atd_running` and `check_prereq` calls before the `#!/usr/bin/env bash` / `set -e` declaration on line 47. Script still works (shebang is only read at `exec`), but errors in the pre-check region don't abort cleanly. +- attack: none — local install script. But a failing prereq check that exits non-zero is dependent on each function's explicit `exit 1`; anything else silently proceeds. +- fix: move `set -e` / `set -u` to the top; the stray shebang on line 47 is dead. + +**No CSRF/XSS/SSRF surface** +- category: n/a +- type: hardening +- severity: low +- evidence: no network, no server, no browser-facing code. +- fix: none. + +--- + +## FEATURE + +### BLOCKERS + +**`--dry-run` is documented but not wired** +- type: missing +- severity: blocker +- evidence: `README.md:139` advertises `schedule-agent --dry-run`; `schedule()` in `cli.py:384-386` accepts the flag and `submit_job(..., dry_run=True)` in `scheduler_backend.py:99-104` prints a preview. But `build_arg_parser` (`cli.py:1985-2074`) defines no `--dry-run` global flag and no code path calls `schedule(..., dry_run=True)`. The documented command silently falls through to the interactive TUI. +- impact: first-time users following the README to preview before submitting get a full-screen TUI instead of the stated preview output. Hurts trust in the docs. +- fix: add `parser.add_argument("--dry-run", action="store_true")` at the top level, plumb through `create_job` → `_resubmit` → `submit_job`. Or remove the line from README. + +**Wrong hint in empty-state TUI** +- type: broken +- severity: blocker +- evidence: `cli.py:1373` — `"No jobs. Press N to create one.\n"`. But `cli.py:1696-1699` binds `a` to `start_new_job_flow`, and `cli.py:1701-1708` binds `n` to "run now" (reschedule selected job to now + 1m). With zero jobs, pressing `N` does nothing (`state.message = "No job selected."`), leaving the screen looking frozen. +- impact: blocks the primary happy path — a new user opens the TUI, sees one job of instruction, presses it, gets nothing. Hot path for the product. +- fix: change the hint to "Press A to add one." Also update the inline help-hint footer (`cli.py:1432-1448`) — it already says `Add`, but the user still needs to map that to the `A` key. + +### FINDINGS + +**CLI has no way to edit prompt prefix** +- type: missing +- severity: medium +- evidence: prompt prefix is edited only via the TUI `P` key (`cli.py:1719-1722` → `start_prefix_edit_flow` → `action_edit_prefix`). There is no `schedule-agent edit-prefix {claude|codex}` subcommand. +- impact: scripted deployment / CI setups can't seed prefixes. Users with `EDITOR` set to a GUI-hostile editor in terminal sessions can't escape the TUI to edit. +- fix: add `edit-prefix ` to `build_arg_parser`. + +**No `--version` flag** +- type: missing +- severity: low +- evidence: `build_arg_parser` has no `version` action. `pyproject.toml:7` declares `0.2.0` but users can't ask the CLI. +- impact: bug reports lack version. `doctor` doesn't print own version either. +- fix: `parser.add_argument("--version", action="version", version=f"{APP_NAME} {__version__}")`. + +**No notification/callback when scheduled job finishes** +- type: missing +- severity: medium +- evidence: README frames the whole value prop as "come back to the result." Only surfacing is TUI list + `schedule-agent list` polling. `mark_finished` (`operations.py:525-552`) writes to disk but fires no hook. +- impact: the user has to re-check manually. Defeats the "just let it run overnight" story for the target use case. +- fix: optional post-job hook — command from config fired after `mark_finished`. Keep opt-in (desktop notifications are out of scope for Linux-only). + +**Deprecated subcommands still aliased with `SUPPRESS`** +- type: polish +- severity: low +- evidence: `cancel` → `unschedule` (`cli.py:2007-2008,2150-2152`), `session` → `set-session` (`cli.py:2025-2028`). Both emit deprecation warnings. +- impact: noise. If you're considering release, decide whether to keep or drop them before 1.0 — removing post-release is harder. +- fix: for v0.2.x keep, for v1.0 cut. + +**`retry` command requires a schedule spec** +- type: ux +- severity: low +- evidence: `cli.py:2030-2032` — `schedule-agent retry `. No default like "now + 1 minute". +- impact: the common case ("re-run this failed job now") is two tokens longer than it should be. +- fix: make `when` optional, default to `now + 1 minute` to match the `N` key behavior. + +**No doctor check for prompt-prefix file existence/readability** +- type: operational +- severity: low +- evidence: `preflight.py:333-351` — no check for `~/.config/schedule-agent/prompt-prefix-*.md`. The file is auto-created on first TUI edit / submit via `ensure_prompt_prefix`; a scheduled job running before that file exists will hit the `2>/dev/null` silent-skip path. +- impact: silently missing prefix = user's "you are executing autonomously" instruction never reaches the agent. Degrades output quality without warning. +- fix: `check_prompt_prefix(agent)` in preflight: warn if missing, pass if readable. + +--- + +## TESTS + +### BLOCKERS + +**Interactive TUI is effectively untested** +- type: gap +- severity: blocker +- evidence: `tests/test_cli.py` is 137 lines; only `test_jobs_menu_requires_prompt_toolkit_when_run_interactively` touches `jobs_menu`. None of: overlay state machine, staged new-job flow, picker navigation, keybinding dispatch, summary column computation, renderers, search filtering. These are >1,000 of the 2,209 lines in `cli.py`. +- impact: the primary UX path is a single regression away from breaking, and because it fails at runtime in a full-screen app, CI won't catch it. +- fix: test the parts that don't need a live terminal first — `_summary_columns`, `render_summary_row`, `_layout_mode`, `_resolve_offset_pick`/`_resolve_clock_pick`, overlay transitions driven directly through the `overlay_*_key` helpers. `prompt_toolkit` can be exercised in its mock mode for keybinding tests. + +### FINDINGS + +**Empty-state hint bug slipped through** +- type: gap +- severity: high +- evidence: see feature blocker above. No test asserts the text of `summary_fragments()` when `cached_jobs` is empty, so the `N`/`A` mismatch was invisible. +- impact: confirms the test gap is not theoretical. +- fix: add a test that renders the empty-state fragments and checks the hinted key matches a live keybinding. + +**`conftest.py` reloads every module on every test** +- type: noise +- severity: low +- evidence: `tests/conftest.py:26-31` — `importlib.reload(...)` on six modules per `app_modules` fixture invocation. +- impact: hides import-time side effects (see architecture finding about `_make_paths` at import). A test that depends on module-level state picks up whatever the last reload produced. +- fix: move the module-level state out; the reloads can then go. + +**Heavy monkeypatching on `cli.*` names** +- type: weak test +- severity: medium +- evidence: `test_cli.py:24-37,48-66,73-93` replaces `cli.list_job_views`, `cli.get_job_view`, `cli.cli_reschedule_job`, etc. These tests assert dispatch, not behavior. +- impact: refactors that rename internals pass the tests while breaking real flows. Many tests are verifying the argparse wiring only. +- fix: complement with at least one end-to-end test that creates a job via `create_job`, runs `main(["list"])`, and parses the printed output. + +**No test touches `at`/`atq`/`atrm` via real processes** +- type: gap +- severity: medium +- evidence: `test_scheduler_backend.py` (168 lines) and `test_scheduler_backend_extra.py` (12 lines) mock `subprocess.run`. The only real invocation is `preflight.check_at_roundtrip` — behind `--roundtrip` and not exercised in CI. +- impact: a change to the `at` output regex or the `-t` format wouldn't be caught until a user hits it. +- fix: opt-in integration test marked `@pytest.mark.integration` that runs when `at` is present in CI (ubuntu-latest has it). Low cost; high realism for a tool whose entire purpose is shelling out to `at`. + +**Test files use module-level `_` prefixed APIs** +- type: misaligned +- severity: low +- evidence: e.g. `test_cli.py:27` calls `app_modules.operations._job_with_scheduler(job)`. +- impact: tests are coupled to private helpers that don't have stability guarantees. +- fix: expose a thin public `job_view(job)` for tests and callers alike. + +--- + +## UI + +Scope: `schedule-agent` ships a terminal TUI built with `prompt_toolkit`, not a web UI. Findings are about TUI clarity, not visual design. + +### BLOCKERS + +**Empty-state instructs the wrong key** +- severity: blocker +- evidence: `cli.py:1373` says `Press N to create one`, but the binding is `A` (`cli.py:1696`). +- problem: new users stare at a screen that tells them to do something that doesn't work. The footer hint `Add` (with the underlined `A`) contradicts the inline copy, compounding confusion. +- fix: change the string to `Press A to add one.` — one character. + +### FINDINGS + +**Status is conveyed primarily by colour** +- severity: medium +- evidence: `cli.py:1932-1953` — status classes `status-running` / `status-failed` / `status-completed` etc. each set distinct `fg:` colours. The textual label (`Running`, `Failed`, etc.) is also rendered, mitigating the issue. +- problem: terminals without colour (CI logs, screen readers, NO_COLOR env) lose the fast-scan affordance. Only the label differentiates; colour difference between `Waiting` and `Blocked` is the only cue on most terminals. +- fix: prepend a glyph per status (`✓`, `✗`, `·`, `!`) so that colour is redundant, not primary. + +**Footer help hint is easy to miss** +- severity: low +- evidence: `cli.py:1450-1456` renders 15 single-letter hints space-separated across one line. On narrow terminals it wraps ugly; on wide ones it blends into the reverse-video footer band. +- problem: discoverability of less common keys (`F` scope, `/` search, `U` unschedule) is low. New users will only find `?`. +- fix: show only 5-6 high-signal hints by default; rest via `?`. Or group them with dividers. + +**Narrow mode drops detail pane** +- severity: low +- evidence: `cli.py:1878-1883` — detail pane hidden when width < 80 cols. There is an overlay detail pane for narrow mode, but `show_detail` has to be toggled. +- problem: acceptable on tiny terminals but there's no visible hint that detail exists / how to reveal it in narrow mode. +- fix: footer hint "d: detail" when narrow. + +**"Paste session ID" uses a label-as-sentinel** +- severity: low +- evidence: `cli.py:325` — `PASTE_SESSION_LABEL = "Paste session ID..."` is both the label displayed and the sentinel value passed through the picker callback (`cli.py:1247-1256`, `cli.py:1761-1767`). +- problem: if a discovered session ever has the title `Paste session ID...` (very unlikely, but possible from the extracted first line of a prompt), the picker treats it as the paste sentinel. +- fix: use a distinct object sentinel (`PASTE_SESSION = object()`) as the value; keep the label as the display string. + +**Job search is title-only** +- severity: low +- evidence: `cli.py:630-632` — substring match on `job["title"]`. No match against agent, session id prefix, status. +- problem: if you titled two jobs similarly but want to find "the Codex one that failed", you can't. +- fix: match against a concatenation of title, agent, display_label. + +--- + +## TOP RELEASE BLOCKERS + +1. **Non-atomic queue write** (`persistence.py:106-112`) — corruption on crash. Fix: write-then-rename. +2. **`--dry-run` documented, not implemented** (`cli.py:1985-2074` / `README.md:139`) — docs lie. +3. **TUI empty-state hint points to wrong key** (`cli.py:1373`) — first-run dead end. +4. **TUI has no automated coverage** (`tests/test_cli.py`) — primary UX path is untested. + +## QUICK WINS + +- Rewrite queue save as atomic rename (~10 lines, `persistence.py`). +- Fix `"Press N"` → `"Press A"` in empty-state fragment. +- Add `--version` to argparse. +- Widen known-good agent version pin to a minimum range or env override. +- Status glyph prepended to `display_label` for colour-blind / no-colour terminals. +- `os.umask(0o077)` on startup or `chmod 0700` the `logs/` dir. +- Default `retry ` to `now + 1 minute` when `when` is omitted. +- Add `check_prompt_prefix` to preflight. + +## DEEPER REFACTORS + +- Split `cli.py` (2,209 lines) into `cli/commands.py` + `cli/tui/`. Unblocks TUI testability. +- Move module-level directory creation out of `cli.py` import side effects; lazy-init in `main()`. +- Collapse `legacy/compat.py` re-exports now living in `persistence`/`scheduler_backend`/`time_utils` back into the `legacy/` boundary, or delete the wrappers. +- Snapshot prompt prefix at submit time instead of resolving it via shell `cat` when the job fires. Removes the tamper-between-schedule-and-run risk and gives a stable record of what ran. +- Post-job hook (user-configured command fired after `mark_finished`) so the "come back to the result" promise doesn't require polling. +- Integration test tier that exercises real `at`/`atq`/`atrm` on ubuntu-latest — the whole product is glue between Python and `at(1)`; mocking that interface hides the only real risk. + +## VERDICT + +**not ready** + +reason: the queue jsonl is rewritten non-atomically (single crash destroys a user's entire state), a documented CLI flag (`--dry-run`) is missing, the empty-state hint in the main TUI points to the wrong key (blocking first-run onboarding), and the ~1,200-line TUI ships with essentially no automated coverage. Each is individually fixable in under a day, but all four need to land before a clean public release. diff --git a/schedule_agent/cli.py b/schedule_agent/cli.py index 8d39d79..d9b915c 100644 --- a/schedule_agent/cli.py +++ b/schedule_agent/cli.py @@ -10,10 +10,11 @@ import sys import tempfile from dataclasses import dataclass, field -from datetime import datetime +from datetime import datetime, timedelta from pathlib import Path from typing import Any, Callable, Sequence +from . import preflight from .config import ensure_prompt_prefix from .execution import AGENTS, build_agent_cmd from .legacy import cli_state as legacy_cli_state @@ -49,6 +50,17 @@ APP_NAME = "schedule-agent" +try: + from importlib.metadata import PackageNotFoundError + from importlib.metadata import version as _pkg_version + + try: + APP_VERSION = _pkg_version("schedule-agent") + except PackageNotFoundError: + APP_VERSION = "0.0.0+local" +except Exception: + APP_VERSION = "0.0.0+local" + load_state = legacy_cli_state.load_state save_state = legacy_cli_state.save_state set_state = legacy_cli_state.set_state @@ -64,12 +76,28 @@ def _data_home_fn() -> Path: return _data_home() -def _make_paths(): - state_dir, data_dir, prompt_dir, _, queue_file = _ensure_dirs() - return state_dir, data_dir, queue_file, legacy_state_file(state_dir), prompt_dir - - -STATE_DIR, DATA_DIR, QUEUE_FILE, STATE_FILE, PROMPT_DIR = _make_paths() +# Path accessors are intentionally lazy — resolving them would mkdir XDG +# dirs, which is a surprise side-effect at import time (breaks hermetic +# tests and packagers that just `import schedule_agent.cli`). The names +# below (STATE_DIR, DATA_DIR, QUEUE_FILE, STATE_FILE, PROMPT_DIR) remain +# accessible as module attributes via __getattr__ for callers that grew +# up with the old eager globals; the first access is what actually +# touches the filesystem. +_LAZY_PATHS = {"STATE_DIR", "DATA_DIR", "QUEUE_FILE", "STATE_FILE", "PROMPT_DIR"} + + +def __getattr__(name): + if name in _LAZY_PATHS: + state_dir, data_dir, prompt_dir, _, queue_file = _ensure_dirs() + values = { + "STATE_DIR": state_dir, + "DATA_DIR": data_dir, + "QUEUE_FILE": queue_file, + "STATE_FILE": legacy_state_file(state_dir), + "PROMPT_DIR": prompt_dir, + } + return values[name] + raise AttributeError(name) def build_cmd(job: dict) -> str: @@ -323,6 +351,10 @@ def discover_sessions(agent: str, cwd: Path | None = None, limit: int = 10) -> l PASTE_SESSION_LABEL = "Paste session ID..." +# Sentinel value for "user picked the paste option" — distinct from any +# session id string so a real session titled "Paste session ID..." can't +# accidentally trigger the paste branch. +PASTE_SESSION: object = object() def _prompt_paste_session_id() -> str | None: @@ -361,6 +393,16 @@ def choose_session(agent: str, cwd: Path | None = None) -> str | None: return None +def _session_picker_items(sessions: list[SessionInfo]) -> list[tuple[str, Any]]: + items: list[tuple[str, Any]] = [("New session", None)] + for session in sessions: + title = session.title or "[no title]" + label = f"{title} [{session.id[:8]}]" + items.append((label, session.id)) + items.append((PASTE_SESSION_LABEL, PASTE_SESSION)) + return items + + def read_prompt(initial: str = "") -> str: with tempfile.NamedTemporaryFile(suffix=".md", delete=False) as handle: path = Path(handle.name) @@ -377,8 +419,8 @@ def read_prompt(initial: str = "") -> str: def write_prompt_file(job_id: str, prompt: str) -> str: - _ensure_dirs() - return _write_prompt_file(PROMPT_DIR, job_id, prompt) + _, _, prompt_dir, _, _ = _ensure_dirs() + return _write_prompt_file(prompt_dir, job_id, prompt) def schedule(job: dict, dry_run: bool = False) -> str: @@ -509,18 +551,29 @@ def cli_delete_job(job_id: str) -> int: return 0 -def cli_submit_job(job_id: str) -> int: +def cli_submit_job(job_id: str, dry_run: bool = False) -> int: try: - job = submit_or_repair_job(job_id) + job = submit_or_repair_job(job_id, dry_run=dry_run) except OperationError as exc: print(f"error: {exc}") return 1 + if dry_run: + print(f"{job_id}: dry-run (not submitted)") + print(job.get("_dry_run_preview", "")) + return 0 print(f"{job_id}: submitted") print(f" at_job_id: {job['at_job_id']}") print(f" run_at: {iso_to_display(job['scheduled_for'], with_seconds=True)}") return 0 +def cli_edit_prefix(agent: str) -> int: + path = ensure_prompt_prefix(agent) + edit_file(path) + print(f"{agent}: prefix updated ({path})") + return 0 + + def cli_mark_running(job_id: str, started_at: str, log_file: str) -> int: from .operations import mark_running @@ -658,7 +711,7 @@ def current_job(self) -> dict | None: TITLE_MIN = 18 TITLE_IDEAL = 28 TITLE_MAX = 80 -STATUS_W = 10 +STATUS_W = 12 RUN_AT_W = 28 SESSION_W = 12 UPDATED_W = 16 @@ -795,11 +848,30 @@ def _summary_columns(mode: str, total_width: int) -> list[tuple[str, int]]: ] +STATUS_GLYPHS = { + "queued": ".", + "scheduled": "o", + "running": ">", + "waiting": "?", + "blocked": "!", + "completed": "+", + "failed": "x", + "removed": "-", + "invalid": "?", +} + + +def _status_with_glyph(job: dict) -> str: + state = job.get("display_state") or "invalid" + glyph = STATUS_GLYPHS.get(state, " ") + return f"{glyph} {job.get('display_label') or 'Invalid'}" + + def _column_value(job: dict, column: str) -> str: if column == "title": return job.get("title") or "(invalid)" if column == "status": - return job.get("display_label") or "Invalid" + return _status_with_glyph(job) if column == "run_at": base = iso_to_display(job.get("scheduled_for")) or "-" rel = _relative_time(job.get("scheduled_for")) @@ -896,10 +968,9 @@ def _dispatch_action( # --- Schedule picker helpers --------------------------------------------- -# The TUI scheduler is fully constrained: the user picks HH then MM (same -# shape as a clock input) and we treat the result as an offset from now, -# producing a string resolve_schedule_input accepts. Using an offset avoids -# ambiguity around "specific time" drifting into the past by a few minutes. +# The TUI scheduler is fully constrained: the user first picks a mode +# (interval vs. absolute clock time), then HH, then MM. Each path produces +# a string resolve_schedule_input accepts. def _resolve_offset_pick(hours: int, minutes: int) -> str: """Return a schedule spec for `now + HH:MM` as total minutes.""" total = hours * 60 + minutes @@ -908,6 +979,18 @@ def _resolve_offset_pick(hours: int, minutes: int) -> str: return f"now + {total} minutes" +def _resolve_clock_pick(hour: int, minute: int) -> str: + """Return a schedule spec for the next occurrence of HH:MM local time. + + If HH:MM has already passed today, schedules for tomorrow instead. + """ + now = datetime.now().astimezone() + target = now.replace(hour=hour, minute=minute, second=0, microsecond=0) + if target <= now: + target = target + timedelta(days=1) + return target.strftime("%Y-%m-%d %H:%M") + + HELP_TEXT = """\ Statuses Queued Created, not yet submitted to at(1) @@ -921,21 +1004,18 @@ def _resolve_offset_pick(hours: int, minutes: int) -> str: Invalid On-disk metadata is broken Actions - N new Create a job (agent -> session -> schedule -> prompt) - Y duplicate Duplicate selected job (reuse agent/session, edit prompt) + A add Create a job (agent -> session -> schedule -> prompt) + N now Run the selected job immediately + R reschedule Change when the selected job runs (also re-runs completed) E edit Edit the selected job's prompt in $EDITOR L log Tail (running) or page (completed) the job's log file P prefix Edit the prompt prefix for Claude or Codex ($EDITOR) - T reschedule Change when the selected job runs C session Change the selected job's session id U unschedule Remove from at(1) but keep metadata (confirmed) S submit Submit or repair the selected job - R retry Reschedule a completed/failed job D delete Permanently delete the selected job f filter Cycle: all / active / completed F scope Toggle project (cwd) / all projects - G refresh Reload from disk now (auto every 30s) - V detail Toggle detail pane (narrow mode) / search Filter by title substring (Esc clears) Home/End Jump to first/last job PgUp/PgDn Page up/down @@ -945,7 +1025,7 @@ def _resolve_offset_pick(hours: int, minutes: int) -> str: # --- jobs_menu ------------------------------------------------------------ -def jobs_menu() -> int: +def jobs_menu(dry_run: bool = False) -> int: toolkit = _require_prompt_toolkit() Application = toolkit["Application"] KeyBindings = toolkit["KeyBindings"] @@ -1018,11 +1098,6 @@ def action_reschedule(job: dict, spec: str) -> str: run_at = iso_to_display(updated["scheduled_for"], with_seconds=True) return f"Rescheduled {job['id']} for {run_at}." - def action_retry(job: dict, spec: str) -> str: - updated = retry_job(job["id"], spec) - run_at = iso_to_display(updated["scheduled_for"], with_seconds=True) - return f"Retry scheduled {job['id']} for {run_at}." - def action_change_session(job: dict, session_id: str | None) -> str: updated = change_session(job["id"], session_id) label = ( @@ -1144,6 +1219,18 @@ def action_create_job(form: dict) -> str: prompt = form["prompt"] cwd = form.get("cwd") or str(Path.cwd()) submit = form.get("submit", True) + if dry_run: + job = create_job( + agent=agent, + session_mode=session_mode, + session_id=session_id, + prompt_text=prompt, + schedule_spec=spec, + cwd=cwd, + submit=False, + dry_run=True, + ) + return f"Dry-run {job['id']}: not submitted. Preview skipped in TUI." job = create_job( agent=agent, session_mode=session_mode, @@ -1159,29 +1246,49 @@ def action_create_job(form: dict) -> str: return f"Created {job['id']} (not submitted)." # ---------- schedule picker (generic) ---------- - # Two-stage constrained flow: pick HH, then pick MM (in 5-min steps). - # The (hh, mm) pair is interpreted as an offset from now and converted - # to a spec string that resolve_schedule_input accepts. `on_spec(spec)` - # is called once the user has finished picking. Callers: new-job, - # reschedule, retry. + # Three-stage constrained flow: pick mode (interval vs. clock time), + # then HH, then MM (in 5-min steps). The (hh, mm) pair is interpreted + # per the chosen mode and converted to a spec string that + # resolve_schedule_input accepts. `on_spec(spec)` is called once the + # user has finished picking. Callers: new-job, reschedule, retry. def schedule_picker_start(prompt_prefix: str, on_spec: Callable[[str], str | None]) -> None: + items = [ + ("Interval (now + HH:MM)", "interval"), + ("At time (next HH:MM)", "clock"), + ] + open_picker( + f"{prompt_prefix}: mode", + items, + on_pick=lambda mode: _schedule_pick_hour(prompt_prefix, mode, on_spec), + ) + + def _schedule_pick_hour( + prompt_prefix: str, + mode: str, + on_spec: Callable[[str], str | None], + ) -> str | None: hour_items = [(f"{h:02d}", h) for h in range(24)] + label = "hours from now" if mode == "interval" else "hour of day" open_picker( - f"{prompt_prefix}: hours from now", + f"{prompt_prefix}: {label}", hour_items, - on_pick=lambda hour: _schedule_pick_minute(prompt_prefix, hour, on_spec), + on_pick=lambda hour: _schedule_pick_minute(prompt_prefix, mode, hour, on_spec), ) + return None def _schedule_pick_minute( prompt_prefix: str, + mode: str, hour: int, on_spec: Callable[[str], str | None], ) -> str | None: items = [(f"{m:02d}", m) for m in range(0, 60, 5)] + label = "minutes from now" if mode == "interval" else "minute of hour" + resolver = _resolve_offset_pick if mode == "interval" else _resolve_clock_pick open_picker( - f"{prompt_prefix}: minutes from now", + f"{prompt_prefix}: {label}", items, - on_pick=lambda minute: on_spec(_resolve_offset_pick(hour, minute)), + on_pick=lambda minute: on_spec(resolver(hour, minute)), ) return None @@ -1216,15 +1323,10 @@ def _nj_picked_agent(form: dict, agent: str) -> str | None: def _nj_pick_session(form: dict) -> None: sessions = discover_sessions(form["agent"], cwd=Path(form["cwd"])) - items: list[tuple[str, Any]] = [("New session", None)] - for session in sessions: - title = session.title or "[no title]" - label = f"{title} [{session.id[:8]}]" - items.append((label, session.id)) - items.append((PASTE_SESSION_LABEL, PASTE_SESSION_LABEL)) + items = _session_picker_items(sessions) def on_pick(value: Any) -> str | None: - if value == PASTE_SESSION_LABEL: + if value is PASTE_SESSION: open_input( "Paste session ID", "", @@ -1283,21 +1385,6 @@ def _nj_confirmed_submit(form: dict, submit: bool) -> str | None: form["submit"] = submit return action_create_job(form) - def start_duplicate_job_flow(job: dict) -> None: - """Duplicate selected job: reuse agent/session, pre-fill prompt, pick schedule.""" - try: - existing_prompt = Path(job["prompt_file"]).read_text(encoding="utf-8") - except Exception: - existing_prompt = "" - form: dict = { - "agent": job.get("agent"), - "session_id": job.get("session_id"), - "schedule_spec": "now + 5 minutes", - "prompt": existing_prompt, - "cwd": job.get("cwd") or str(Path.cwd()), - } - _nj_pick_schedule(form) - # ---------- rendering ---------- _COUNT_ORDER = [ ("Q", "queued"), @@ -1362,7 +1449,7 @@ def summary_fragments(): if state.search_query: msg = f"No jobs match '{state.search_query}'. Press / to edit, Esc to clear.\n" else: - msg = "No jobs. Press N to create one.\n" + msg = "No jobs. Press A to add one.\n" fragments.append(("class:muted", msg)) return fragments @@ -1422,22 +1509,19 @@ def _maybe_dismiss_help() -> None: state.show_help = False _HELP_HINT_PAIRS: list[tuple[str, str]] = [ - ("N", "ew"), - ("Y", " dup"), + ("A", "dd"), + ("N", "ow"), + ("R", "eschedule"), ("E", "dit"), ("L", "og"), ("P", "refix"), - ("T", " time"), ("C", " session"), ("U", "nschedule"), ("S", "ubmit"), - ("R", "etry"), ("D", "elete"), ("f", "ilter"), ("F", " scope"), ("/", " search"), - ("G", " refresh"), - ("V", " detail"), ("?", " help"), ("Q", "uit"), ] @@ -1477,10 +1561,27 @@ def picker_fragments(): if not overlay.items: fragments.append(("class:muted", " (no choices) \n")) return fragments - for idx, (label, _value) in enumerate(overlay.items): - marker = "> " if idx == overlay.picker_index else " " - style = "class:selected" if idx == overlay.picker_index else "" + # picker_window is clamped to max=16 lines; reserve 1 for the title + # and 1 for the footer, and scroll around the selected index so + # long lists (e.g. 24 hours) remain fully navigable. + visible = 14 + total = len(overlay.items) + selected = overlay.picker_index % total + if total <= visible: + start = 0 + else: + start = max(0, selected - visible // 2) + start = min(start, total - visible) + end = min(total, start + visible) + if start > 0: + fragments.append(("class:muted", f" ... ({start} more above)\n")) + for idx in range(start, end): + label, _value = overlay.items[idx] + marker = "> " if idx == selected else " " + style = "class:selected" if idx == selected else "" fragments.append((style, f"{marker}{label}\n")) + if end < total: + fragments.append(("class:muted", f" ... ({total - end} more below)\n")) fragments.append( ( "class:muted", @@ -1671,30 +1772,19 @@ def _cycle_scope(event): label = "project (cwd)" if state.scope == "project" else "all projects" state.message = f"Scope: {label}." - @kb.add("g", filter=no_overlay) - def _refresh(event): - _maybe_dismiss_help() - state.refresh_jobs() - state.message = "Refreshed." - - @kb.add("v", filter=no_overlay) - def _toggle_detail(event): - _maybe_dismiss_help() - state.show_detail = not state.show_detail - state.message = "Detail shown." if state.show_detail else "Detail hidden." - - @kb.add("enter", filter=no_overlay) - def _enter_toggle_detail(event): + @kb.add("a", filter=no_overlay) + def _new(event): _maybe_dismiss_help() - # In narrow mode, Enter toggles detail; in wider modes it is a - # no-op (detail already visible beside the list). - if _layout_mode_now() == "narrow": - state.show_detail = not state.show_detail + start_new_job_flow() @kb.add("n", filter=no_overlay) - def _new(event): + def _run_now(event): _maybe_dismiss_help() - start_new_job_flow() + job = state.current_job() + if not job: + state.message = "No job selected." + return + _dispatch_action(state, lambda: action_reschedule(job, "now + 1 minute")) @kb.add("e", filter=no_overlay) def _edit(event): @@ -1719,7 +1809,7 @@ def _log(event): return _dispatch_action(state, lambda: action_view_log(job)) - @kb.add("t", filter=no_overlay) + @kb.add("r", filter=no_overlay) def _reschedule(event): _maybe_dismiss_help() job = state.current_job() @@ -1739,15 +1829,10 @@ def _session(event): state.message = "No job selected." return sessions = discover_sessions(job["agent"], cwd=Path(job["cwd"])) - items: list[tuple[str, Any]] = [("New session", None)] - for session in sessions: - title = session.title or "[no title]" - label = f"{title} [{session.id[:8]}]" - items.append((label, session.id)) - items.append((PASTE_SESSION_LABEL, PASTE_SESSION_LABEL)) + items = _session_picker_items(sessions) def on_pick(value: Any) -> str | None: - if value == PASTE_SESSION_LABEL: + if value is PASTE_SESSION: open_input( "Paste session ID", "", @@ -1782,18 +1867,6 @@ def _submit(event): return _dispatch_action(state, lambda: action_submit(job)) - @kb.add("r", filter=no_overlay) - def _retry(event): - _maybe_dismiss_help() - job = state.current_job() - if not job: - state.message = "No job selected." - return - schedule_picker_start( - f"Retry {job['id']}", - on_spec=lambda spec: action_retry(job, spec), - ) - @kb.add("d", filter=no_overlay) def _delete(event): _maybe_dismiss_help() @@ -1867,15 +1940,6 @@ def _clear_search(event): state.refresh_jobs() state.message = "Search cleared." - @kb.add("y", filter=no_overlay) - def _duplicate(event): - _maybe_dismiss_help() - job = state.current_job() - if not job: - state.message = "No job selected." - return - start_duplicate_job_flow(job) - @kb.add("c-c") def _ctrl_c(event): state.quit = True @@ -1906,7 +1970,7 @@ def _ctrl_c(event): picker_window = Window( content=FormattedTextControl(picker_fragments), wrap_lines=False, - height=Dimension(min=3, max=16), + height=Dimension(min=3, max=18), ) body = HSplit( @@ -1997,6 +2061,16 @@ def build_arg_parser() -> argparse.ArgumentParser: prog=APP_NAME, description="Schedule Codex and Claude CLI jobs.", ) + parser.add_argument( + "--version", + action="version", + version=f"{APP_NAME} {APP_VERSION}", + ) + parser.add_argument( + "--dry-run", + action="store_true", + help="Preview the scheduled at(1) script without submitting.", + ) sub = parser.add_subparsers(dest="command") list_p = sub.add_parser("list", help="List jobs.") @@ -2039,11 +2113,22 @@ def build_arg_parser() -> argparse.ArgumentParser: retry_p = sub.add_parser("retry", help="Retry a completed/failed job at a new time.") retry_p.add_argument("job_id") - retry_p.add_argument("when") + retry_p.add_argument( + "when", + nargs="?", + default="now + 1 minute", + help="Schedule spec (default: 'now + 1 minute').", + ) sub_p = sub.add_parser("submit", help="Submit or repair a queued/scheduled job.") sub_p.add_argument("job_id") + edit_prefix_p = sub.add_parser( + "edit-prefix", + help="Edit the prompt prefix for an agent in $EDITOR.", + ) + edit_prefix_p.add_argument("agent", choices=["claude", "codex"]) + mark_p = sub.add_parser("mark", help="Update job execution state (for scheduled wrapper use).") mark_sub = mark_p.add_subparsers(dest="mark_state") run_p = mark_sub.add_parser("running") @@ -2096,8 +2181,6 @@ def cli_doctor( verbose: bool = False, quiet: bool = False, ) -> int: - from . import preflight - report = preflight.run_checks(include_roundtrip=roundtrip) if as_json: @@ -2175,7 +2258,9 @@ def main(argv: Sequence[str] | None = None) -> int: if args.command == "retry": return cli_retry_job(args.job_id, args.when) if args.command == "submit": - return cli_submit_job(args.job_id) + return cli_submit_job(args.job_id, dry_run=args.dry_run) + if args.command == "edit-prefix": + return cli_edit_prefix(args.agent) if args.command == "doctor": return cli_doctor( roundtrip=args.roundtrip, @@ -2206,7 +2291,7 @@ def main(argv: Sequence[str] | None = None) -> int: ) parser.error("mark requires a state: running, done, or failed") - return jobs_menu() + return jobs_menu(dry_run=args.dry_run) except KeyboardInterrupt: print("\nCancelled.") return 130 diff --git a/schedule_agent/environment.py b/schedule_agent/environment.py index 63c972d..a404279 100644 --- a/schedule_agent/environment.py +++ b/schedule_agent/environment.py @@ -8,11 +8,42 @@ from .execution import AGENTS +# Minimum known-good version per agent. Any version >= this is treated as +# compatible; preflight only warns when the probed version is strictly +# older. Override per-agent via env: +# SCHEDULE_AGENT_MIN_CLAUDE=2.1.112 +# SCHEDULE_AGENT_MIN_CODEX=0.120.0 +KNOWN_GOOD_MIN_VERSIONS: dict[str, str] = { + "claude": "2.1.112", + "codex": "0.120.0", +} + +# Retained for backwards-compat with tests / external callers; now derived. KNOWN_GOOD_AGENT_VERSIONS: dict[str, set[str]] = { - "claude": {"2.1.112"}, - "codex": {"0.120.0"}, + agent: {minimum} for agent, minimum in KNOWN_GOOD_MIN_VERSIONS.items() } + +def _parse_version(value: str) -> tuple[int, ...]: + parts: list[int] = [] + for segment in value.split("."): + digits = "".join(ch for ch in segment if ch.isdigit()) + parts.append(int(digits) if digits else 0) + return tuple(parts) + + +def _min_version_for(agent: str) -> str: + env_key = f"SCHEDULE_AGENT_MIN_{agent.upper()}" + return os.environ.get(env_key) or KNOWN_GOOD_MIN_VERSIONS[agent] + + +def _version_at_least(probed: str, minimum: str) -> bool: + try: + return _parse_version(probed) >= _parse_version(minimum) + except Exception: + return False + + REQUIRED_AGENT_HELP_SUBSTRINGS: dict[str, list[str]] = { "claude": ["--resume", "--dangerously-skip-permissions"], "codex": ["exec", "--dangerously-bypass-approvals-and-sandbox"], @@ -90,7 +121,7 @@ def probe_agent(agent: str) -> AgentProbe: error = str(exc) version = None - version_known_good = version is not None and version in KNOWN_GOOD_AGENT_VERSIONS[agent] + version_known_good = version is not None and _version_at_least(version, _min_version_for(agent)) help_ok = False try: diff --git a/schedule_agent/execution.py b/schedule_agent/execution.py index faea8ea..e2fbb00 100644 --- a/schedule_agent/execution.py +++ b/schedule_agent/execution.py @@ -2,7 +2,6 @@ import shlex -from .config import prompt_prefix_path from .legacy.compat import resolve_session_id @@ -26,15 +25,19 @@ def _agent_bin(job: dict, cfg: dict) -> str: } -def _prompt_expr(agent: str, prompt_file: str) -> str: - """Shell expression that cats the prefix (if present) + prompt file. +def _prompt_expr(prefix_snapshot: str | None, prompt_file: str) -> str: + """Shell expression that cats the frozen prefix snapshot + prompt file. - Using `2>/dev/null` on the prefix keeps the command silent if the user - deletes it between edits. An intermediate `echo` guarantees a newline - separator even if the prefix file lacks a trailing newline. + The prefix is snapshotted into the job's log_dir at submit time so a + later edit of the live prefix (or tampering with it between schedule + and fire) cannot change what the scheduled job actually sends to the + agent. `2>/dev/null` keeps the command silent if the snapshot was + never written (e.g. job scheduled before snapshot plumbing existed). """ - prefix = shlex.quote(str(prompt_prefix_path(agent))) prompt = shlex.quote(prompt_file) + if prefix_snapshot is None: + return f'"$(cat {prompt})"' + prefix = shlex.quote(prefix_snapshot) return f'"$(cat {prefix} 2>/dev/null; echo; cat {prompt})"' @@ -42,10 +45,12 @@ def build_agent_cmd(job: dict) -> str: """Build the shell command to invoke the agent for this job. Deprecated compatibility with the old `session` field is isolated under - `schedule_agent.legacy`. + `schedule_agent.legacy`. The prefix snapshot is resolved from + job["prefix_snapshot_file"] if present (written at submit time), else + omitted — legacy jobs submitted before snapshotting just get the prompt. """ cfg = AGENTS[job["agent"]] - prompt_expr = _prompt_expr(job["agent"], job["prompt_file"]) + prompt_expr = _prompt_expr(job.get("prefix_snapshot_file"), job["prompt_file"]) base = " ".join(cfg["base_args"]) bin_ = shlex.quote(_agent_bin(job, cfg)) diff --git a/schedule_agent/operations.py b/schedule_agent/operations.py index b8f7afb..f038cdf 100644 --- a/schedule_agent/operations.py +++ b/schedule_agent/operations.py @@ -2,7 +2,9 @@ import fcntl import os +import shlex import shutil +import subprocess from contextlib import contextmanager from dataclasses import dataclass from pathlib import Path @@ -39,7 +41,13 @@ display_label, scheduler_label, ) -from .time_utils import iso_to_display, now_iso, sort_key_for_iso, title_from_prompt +from .time_utils import ( + iso_to_display, + now_iso, + parse_iso_datetime, + sort_key_for_iso, + title_from_prompt, +) from .transitions import ( make_job, on_change_session, @@ -82,6 +90,62 @@ def _locked_queue(): fcntl.flock(handle.fileno(), fcntl.LOCK_UN) +def _stale_threshold_seconds() -> int: + raw = os.environ.get("SCHEDULE_AGENT_STALE_MINUTES", "60") + try: + minutes = int(raw) + except ValueError: + minutes = 60 + return max(1, minutes) * 60 + + +def _is_stale_running(job: dict, now_ts: float, threshold: int) -> bool: + # A job is "stuck" running when its driver script died before the EXIT + # trap (mark done/failed) could fire — typically a reboot or kill. + # Detect this by combining two signals: the recorded start is older + # than the threshold AND the log file hasn't been written to recently + # (or is missing). Both conditions must hold so that genuinely + # long-running but quiet jobs aren't killed off. + if job.get("submission") != "running": + return False + started = job.get("last_started_at") + if not started: + return False + try: + started_ts = parse_iso_datetime(started).timestamp() + except ValueError: + return False + if now_ts - started_ts < threshold: + return False + log_file = job.get("last_log_file") + if log_file: + try: + log_mtime = os.path.getmtime(log_file) + except OSError: + return True + return now_ts - log_mtime >= threshold + return True + + +def _recover_stale_running_inplace(jobs: list[dict]) -> bool: + threshold = _stale_threshold_seconds() + now_ts = parse_iso_datetime(now_iso()).timestamp() + changed = False + for idx, job in enumerate(jobs): + if job.get("_invalid"): + continue + if not _is_stale_running(job, now_ts, threshold): + continue + jobs[idx] = on_failure( + job, + finished_at=now_iso(), + exit_code=-1, + log_file=job.get("last_log_file"), + ) + changed = True + return changed + + def _job_id(agent: str) -> str: return f"{agent}-{now_iso().replace(':', '').replace('-', '')}" @@ -163,7 +227,10 @@ def key(job: dict): def list_job_views(filter_name: str = "all") -> tuple[list[dict], str | None]: - jobs = load_jobs() + with _locked_queue(): + jobs = load_jobs() + if _recover_stale_running_inplace(jobs): + save_jobs(jobs) atq_entries, atq_error = query_atq() views = [_job_with_scheduler(job, atq_entries, atq_error) for job in jobs] @@ -190,6 +257,8 @@ def get_job_view(job_id: str) -> dict | None: def _load_locked_job(job_id: str) -> tuple[list[dict], int, dict]: jobs = load_jobs() + if _recover_stale_running_inplace(jobs): + save_jobs(jobs) idx, job = find_job(jobs, job_id) if idx is None or job is None: raise OperationError(f"No such job: {job_id}") @@ -303,7 +372,12 @@ def create_job( schedule_spec: str, cwd: str, submit: bool = True, + dry_run: bool = False, ) -> dict: + # dry_run=True: resolve the schedule, build the at(1) script, but do not + # persist a job record and do not enqueue with at. The preview text is + # attached to the returned job dict under "_dry_run_preview" so callers + # can display it. with _locked_queue(): report, probe = _submit_preflight(agent) if not report.critical_ok(): @@ -312,6 +386,25 @@ def create_job( job_id = _job_id(agent) scheduled_for = resolve_schedule_spec(schedule_spec) + if dry_run: + # Don't write the prompt file either — dry-run is pure preview. + preview_prompt_file = str(_prompt_dir() / f"{job_id}.md") + job = make_job( + job_id=job_id, + title=title_from_prompt(prompt_text), + agent=agent, + session_mode=session_mode, + session_id=session_id, + prompt_file=preview_prompt_file, + scheduled_for=scheduled_for, + cwd=cwd, + log_dir=job_log_dir(job_id), + ) + job["provenance"] = _build_provenance(probe, report) + _, preview = submit_job(job, dry_run=True) + job["_dry_run_preview"] = preview + return job + prompt_file = write_prompt_file(_prompt_dir(), job_id, prompt_text) job = make_job( job_id=job_id, @@ -398,7 +491,7 @@ def delete_job(job_id: str) -> None: ) -def submit_or_repair_job(job_id: str) -> dict: +def submit_or_repair_job(job_id: str, dry_run: bool = False) -> dict: with _locked_queue(): jobs, idx, job = _load_locked_job(job_id) if job["submission"] == "running": @@ -412,6 +505,11 @@ def submit_or_repair_job(job_id: str) -> dict: f"execution={working['execution']}, " f"readiness={working['readiness']})" ) + if dry_run: + _, preview = submit_job(working, dry_run=True) + working = dict(working) + working["_dry_run_preview"] = preview + return working submitted, _ = _resubmit(working) jobs[idx] = submitted save_jobs(jobs) @@ -455,6 +553,40 @@ def _update_dependents(jobs: list[dict], parent_id: str, parent_result: str) -> return updated_jobs +def _fire_post_hook(job: dict, result: str) -> None: + # Opt-in post-completion hook: SCHEDULE_AGENT_POST_HOOK is a shell + # command fragment that receives JOB_ID / JOB_TITLE / JOB_RESULT / + # JOB_EXIT_CODE / JOB_LOG_FILE in its environment. Any failure is + # swallowed — the hook must never block state advancement, because + # mark_finished runs in the at(1) wrapper's EXIT trap. + hook = os.environ.get("SCHEDULE_AGENT_POST_HOOK") + if not hook: + return + try: + parts = shlex.split(hook) + except ValueError: + return + if not parts: + return + env = dict(os.environ) + env["JOB_ID"] = job.get("id", "") + env["JOB_TITLE"] = job.get("title") or "" + env["JOB_RESULT"] = result + env["JOB_EXIT_CODE"] = str(job.get("last_exit_code") or "") + env["JOB_LOG_FILE"] = job.get("last_log_file") or "" + try: + subprocess.Popen( + parts, + env=env, + stdin=subprocess.DEVNULL, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + close_fds=True, + ) + except OSError: + pass + + def mark_finished( job_id: str, finished_at: str, @@ -482,7 +614,8 @@ def mark_finished( jobs[idx] = updated jobs = _update_dependents(jobs, parent_id=job_id, parent_result=result) save_jobs(jobs) - return updated + _fire_post_hook(updated, result) + return updated def format_job_summary(job: dict) -> str: diff --git a/schedule_agent/persistence.py b/schedule_agent/persistence.py index 8de4fdb..c29fde3 100644 --- a/schedule_agent/persistence.py +++ b/schedule_agent/persistence.py @@ -30,6 +30,13 @@ def _ensure_dirs() -> tuple[Path, Path, Path, Path, Path]: data_dir.mkdir(parents=True, exist_ok=True) prompt_dir.mkdir(parents=True, exist_ok=True) logs_dir.mkdir(parents=True, exist_ok=True) + # Logs and prompts may contain agent-generated secrets. Tighten perms + # to 0700 so other local users can't read under lax home-dir modes. + for sensitive in (logs_dir, prompt_dir, state_dir): + try: + os.chmod(sensitive, 0o700) + except OSError: + pass return state_dir, data_dir, prompt_dir, logs_dir, queue_file @@ -104,12 +111,18 @@ def load_jobs() -> list[dict]: def save_jobs(jobs: list[dict]) -> None: + # Atomic write: a crash mid-write must never leave a torn queue file + # behind. Write to a sibling tempfile, fsync, then os.replace onto the + # final path (POSIX-atomic rename within the same directory). queue_file = _queue_file() _ensure_dirs() - queue_file.write_text( - "\n".join(json.dumps(job, ensure_ascii=False) for job in jobs), - encoding="utf-8", - ) + payload = "\n".join(json.dumps(job, ensure_ascii=False) for job in jobs) + tmp = queue_file.with_suffix(queue_file.suffix + ".tmp") + with open(tmp, "w", encoding="utf-8") as handle: + handle.write(payload) + handle.flush() + os.fsync(handle.fileno()) + os.replace(tmp, queue_file) def find_job(jobs: list[dict], job_id: str) -> tuple[int, dict] | tuple[None, None]: diff --git a/schedule_agent/preflight.py b/schedule_agent/preflight.py index 85e7edc..2f8e10a 100644 --- a/schedule_agent/preflight.py +++ b/schedule_agent/preflight.py @@ -7,6 +7,7 @@ from datetime import datetime, timedelta from pathlib import Path +from .config import prompt_prefix_path from .environment import ( KNOWN_GOOD_AGENT_VERSIONS, AgentProbe, @@ -179,12 +180,12 @@ def check_agent(agent: str, probe: AgentProbe | None = None) -> CheckResult: detail=detail, ) if probe.version_known_good is False: - known = sorted(KNOWN_GOOD_AGENT_VERSIONS[agent]) + minimum = sorted(KNOWN_GOOD_AGENT_VERSIONS[agent])[0] return CheckResult( name=name, label=label, severity="WARN", - message=f"{probe.version} is untested; last known-good: {known}", + message=f"{probe.version} is older than known-good minimum {minimum}", detail=detail, ) return CheckResult( @@ -330,6 +331,42 @@ def check_at_roundtrip() -> CheckResult: pass +def check_prompt_prefix(agent: str) -> CheckResult: + """Warn if the prompt prefix for `agent` is missing or unreadable. + + The prefix is auto-created on first use via `ensure_prompt_prefix`; a + stale state where the config dir exists but the prefix file has been + deleted silently strips "you are running autonomously" framing from + every scheduled job, degrading output quality without a signal. + """ + name = f"prompt_prefix_{agent}" + label = f"{agent} prompt prefix" + path = prompt_prefix_path(agent) + if not path.exists(): + return CheckResult( + name=name, + label=label, + severity="WARN", + message=f"missing: {path} (will be auto-created on first submit)", + detail={"path": str(path)}, + ) + if not os.access(path, os.R_OK): + return CheckResult( + name=name, + label=label, + severity="FAIL", + message=f"not readable: {path}", + detail={"path": str(path)}, + ) + return CheckResult( + name=name, + label=label, + severity="PASS", + message=str(path), + detail={"path": str(path)}, + ) + + def run_checks(include_roundtrip: bool = False) -> PreflightReport: """Run every preflight check and return a PreflightReport.""" results: list[CheckResult] = [] @@ -345,6 +382,9 @@ def run_checks(include_roundtrip: bool = False) -> PreflightReport: results.append(check_session_dir("claude", claude_result.severity)) results.append(check_session_dir("codex", codex_result.severity)) + results.append(check_prompt_prefix("claude")) + results.append(check_prompt_prefix("codex")) + if include_roundtrip: results.append(check_at_roundtrip()) diff --git a/schedule_agent/scheduler_backend.py b/schedule_agent/scheduler_backend.py index f2b2339..df5e9a1 100644 --- a/schedule_agent/scheduler_backend.py +++ b/schedule_agent/scheduler_backend.py @@ -6,7 +6,9 @@ import shutil import subprocess from dataclasses import dataclass +from pathlib import Path +from .config import load_prompt_prefix from .execution import build_agent_cmd from .time_utils import ( iso_to_at_time, @@ -60,8 +62,36 @@ def parse_atq_line(line: str) -> AtqEntry | None: ) +def _write_prefix_snapshot(job: dict) -> str | None: + """Freeze the current prompt prefix into the job's log_dir. + + Returns the snapshot path, or None if no prefix is configured. The + snapshot is written with 0600 perms since it may contain sensitive + user instructions. + """ + log_dir = job.get("log_dir") + if not log_dir: + return None + content = load_prompt_prefix(job["agent"]) + snapshot_dir = Path(log_dir) + snapshot_dir.mkdir(parents=True, exist_ok=True) + snapshot_path = snapshot_dir / "prefix.snapshot" + snapshot_path.write_text(content, encoding="utf-8") + try: + os.chmod(snapshot_path, 0o600) + except OSError: + pass + return str(snapshot_path) + + def build_script(job: dict) -> str: - cmd = build_agent_cmd(job) + # Snapshot the prompt prefix at submit time. Mutating the job dict + # locally (not persisting) means build_script is idempotent while the + # at-script stably references an immutable per-job snapshot path. + snapshot_path = _write_prefix_snapshot(job) + job_with_snapshot = dict(job) + job_with_snapshot["prefix_snapshot_file"] = snapshot_path + cmd = build_agent_cmd(job_with_snapshot) sa_bin = shlex.quote(shutil.which("schedule-agent") or "schedule-agent") provenance = job.get("provenance") or {} path_entries = provenance.get("path_snapshot_cleaned") or DEFAULT_PATH_ENTRIES @@ -120,6 +150,13 @@ def submit_job(job: dict, dry_run: bool = False) -> tuple[str | None, str]: def remove_at_job(at_job_id: str) -> tuple[bool, str]: + # Consult atq first: if the id is not present, treat as already-gone + # rather than calling atrm on a potentially-reused id. atq -o format + # preserves the id's owner so a simple presence check is sufficient — + # atrm only acts on the invoking user's jobs. + entry, query_err = query_atq_entry(str(at_job_id)) + if entry is None and not query_err: + return True, "" proc = _run_at( ["atrm", str(at_job_id)], capture_output=True, diff --git a/schedule_agent/time_utils.py b/schedule_agent/time_utils.py index 690b43f..152195a 100644 --- a/schedule_agent/time_utils.py +++ b/schedule_agent/time_utils.py @@ -5,8 +5,8 @@ from datetime import datetime ISO_SECONDS_FORMAT = "%Y-%m-%dT%H:%M:%S%z" -DISPLAY_MINUTE_FORMAT = "%Y-%m-%d %H:%M" -DISPLAY_SECOND_FORMAT = "%Y-%m-%d %H:%M:%S" +DISPLAY_MINUTE_FORMAT = "%m-%d %H:%M" +DISPLAY_SECOND_FORMAT = "%m-%d %H:%M:%S" AT_TIME_FORMAT = "%Y%m%d%H%M.00" diff --git a/tests/test_audit_fixes.py b/tests/test_audit_fixes.py new file mode 100644 index 0000000..2e55e0e --- /dev/null +++ b/tests/test_audit_fixes.py @@ -0,0 +1,354 @@ +"""Tests covering release-readiness fixes from reports/audits/combined.md.""" + +from __future__ import annotations + +import json +import os +import stat +from pathlib import Path + +import pytest + +# --------------------------------------------------------------------------- +# Architecture: atomic queue write +# --------------------------------------------------------------------------- + + +def test_save_jobs_is_atomic_via_rename(app_modules, monkeypatch): + # A crash mid-write must not leave a torn queue.jsonl. save_jobs writes + # to a .tmp sibling and os.replaces it into place. We assert the rename + # happens and the tmp path never co-exists with a partial target. + persistence = app_modules.persistence + persistence._ensure_dirs() + + original_replace = os.replace + calls: list[tuple[str, str]] = [] + + def tracking_replace(src, dst): + calls.append((str(src), str(dst))) + return original_replace(src, dst) + + monkeypatch.setattr(persistence.os, "replace", tracking_replace) + persistence.save_jobs([{"id": "a", "title": "t"}]) + assert len(calls) == 1 + src, dst = calls[0] + assert src.endswith(".jsonl.tmp") + assert dst.endswith(".jsonl") + assert not Path(src).exists() + assert Path(dst).exists() + assert json.loads(Path(dst).read_text()) == {"id": "a", "title": "t"} + + +def test_save_jobs_failure_does_not_corrupt_existing_queue(app_modules, monkeypatch): + persistence = app_modules.persistence + persistence._ensure_dirs() + persistence.save_jobs([{"id": "old", "title": "keep me"}]) + queue_file = persistence._queue_file() + original = queue_file.read_text() + + def boom(src, dst): + raise OSError("disk full simulating mid-write crash") + + monkeypatch.setattr(persistence.os, "replace", boom) + with pytest.raises(OSError): + persistence.save_jobs([{"id": "new", "title": "would-be replacement"}]) + # Original untouched because the atomic rename failed. + assert queue_file.read_text() == original + + +# --------------------------------------------------------------------------- +# Security: sensitive dir permissions +# --------------------------------------------------------------------------- + + +def test_ensure_dirs_chmods_sensitive_dirs_to_0700(app_modules): + persistence = app_modules.persistence + state_dir, _data_dir, prompt_dir, logs_dir, _queue_file = persistence._ensure_dirs() + for path in (state_dir, prompt_dir, logs_dir): + mode = stat.S_IMODE(os.stat(path).st_mode) + # at least owner-only; allow 0o700 exactly + assert mode == 0o700, f"{path} has mode {oct(mode)}, expected 0o700" + + +# --------------------------------------------------------------------------- +# Security: prompt prefix snapshot +# --------------------------------------------------------------------------- + + +def test_build_script_snapshots_prompt_prefix(app_modules, monkeypatch): + scheduler_backend = app_modules.scheduler_backend + from schedule_agent import config as _config + + monkeypatch.setattr(_config, "load_prompt_prefix", lambda agent: f"SNAPSHOT-FOR-{agent}\n") + monkeypatch.setattr( + scheduler_backend, "load_prompt_prefix", lambda agent: f"SNAPSHOT-FOR-{agent}\n" + ) + + log_dir = app_modules.persistence.job_log_dir("snapjob") + job = { + "id": "snapjob", + "agent": "claude", + "prompt_file": "/tmp/p.md", + "session_mode": "new", + "session_id": None, + "cwd": "/tmp", + "log_dir": log_dir, + "scheduled_for": "2026-04-23T09:00:00+0000", + } + script = scheduler_backend.build_script(job) + snapshot_path = Path(log_dir) / "prefix.snapshot" + assert snapshot_path.exists() + assert snapshot_path.read_text() == "SNAPSHOT-FOR-claude\n" + mode = stat.S_IMODE(os.stat(snapshot_path).st_mode) + assert mode == 0o600 + assert str(snapshot_path) in script + + +# --------------------------------------------------------------------------- +# Security: atrm skips call when id not in atq +# --------------------------------------------------------------------------- + + +def test_remove_at_job_skips_atrm_when_id_absent(app_modules, monkeypatch): + scheduler_backend = app_modules.scheduler_backend + monkeypatch.setattr(scheduler_backend, "query_atq_entry", lambda at_id: (None, None)) + + called = {"ran": False} + + def spy_run_at(cmd, **kwargs): + called["ran"] = True + raise AssertionError("atrm must not be called when job is absent from atq") + + monkeypatch.setattr(scheduler_backend, "_run_at", spy_run_at) + ok, err = scheduler_backend.remove_at_job("999") + assert ok is True + assert err == "" + assert called["ran"] is False + + +# --------------------------------------------------------------------------- +# Feature: --version prints package version and exits 0 +# --------------------------------------------------------------------------- + + +def test_version_flag_prints_and_exits(app_modules, capsys): + cli = app_modules.cli + with pytest.raises(SystemExit) as exc: + cli.main(["--version"]) + assert exc.value.code == 0 + out = capsys.readouterr().out + assert "schedule-agent" in out + + +# --------------------------------------------------------------------------- +# Feature: edit-prefix subcommand +# --------------------------------------------------------------------------- + + +def test_edit_prefix_subcommand_opens_editor_on_prefix_file(app_modules, monkeypatch, tmp_path): + cli = app_modules.cli + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "cfg")) + + edited: dict[str, Path] = {} + + def fake_edit(path): + edited["path"] = Path(path) + Path(path).write_text("edited\n", encoding="utf-8") + + monkeypatch.setattr(cli, "edit_file", fake_edit) + rc = cli.main(["edit-prefix", "claude"]) + assert rc == 0 + assert edited["path"].name == "prompt-prefix-claude.md" + assert edited["path"].read_text() == "edited\n" + + +# --------------------------------------------------------------------------- +# Feature: retry defaults to 'now + 1 minute' when no schedule spec given +# --------------------------------------------------------------------------- + + +def test_retry_defaults_to_one_minute(app_modules, monkeypatch): + cli = app_modules.cli + captured = {} + + def fake_retry(job_id, spec): + captured["job_id"] = job_id + captured["spec"] = spec + return 0 + + monkeypatch.setattr(cli, "cli_retry_job", fake_retry) + cli.main(["retry", "job1"]) + assert captured == {"job_id": "job1", "spec": "now + 1 minute"} + + +# --------------------------------------------------------------------------- +# Feature: --dry-run plumbed into submit +# --------------------------------------------------------------------------- + + +def test_submit_dry_run_prints_preview_and_does_not_persist(app_modules, monkeypatch, capsys): + cli = app_modules.cli + operations = app_modules.operations + monkeypatch.setattr( + operations, + "submit_or_repair_job", + lambda job_id, dry_run=False: ( + { + "id": job_id, + "_dry_run_preview": "PREVIEW-BODY", + } + if dry_run + else (_ for _ in ()).throw(AssertionError("dry_run must be True in this test")) + ), + ) + # Mirror into cli module-level binding + monkeypatch.setattr( + cli, + "submit_or_repair_job", + operations.submit_or_repair_job, + ) + rc = cli.main(["--dry-run", "submit", "job1"]) + assert rc == 0 + out = capsys.readouterr().out + assert "dry-run" in out + assert "PREVIEW-BODY" in out + + +# --------------------------------------------------------------------------- +# UI: empty-state hint points to the actual keybinding +# --------------------------------------------------------------------------- + + +def test_empty_state_hint_matches_add_binding(app_modules): + # The empty-state copy in summary_fragments is a string literal inside + # jobs_menu's closure. We inspect the source instead of instantiating + # the full prompt_toolkit app, since the audit blocker is specifically + # "the hinted key must match the live binding." + import inspect + + source = inspect.getsource(app_modules.cli.jobs_menu) + assert "Press A to add one" in source + assert "Press N to create one" not in source + # And the `a` binding still exists and calls start_new_job_flow + assert '@kb.add("a"' in source + + +# --------------------------------------------------------------------------- +# UI: status glyphs are added to status column +# --------------------------------------------------------------------------- + + +def test_status_column_value_prepends_glyph(app_modules): + cli = app_modules.cli + for state, glyph in cli.STATUS_GLYPHS.items(): + job = {"display_state": state, "display_label": state.title()} + value = cli._column_value(job, "status") + assert value.startswith(glyph + " ") + + +# --------------------------------------------------------------------------- +# Architecture: import has no filesystem side effects +# --------------------------------------------------------------------------- + + +def test_importing_cli_does_not_create_xdg_dirs(tmp_path, monkeypatch): + import importlib + import sys + + monkeypatch.setenv("XDG_STATE_HOME", str(tmp_path / "state")) + monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path / "data")) + # Drop cached modules so the import re-executes module-level code. + for name in list(sys.modules): + if name.startswith("schedule_agent"): + del sys.modules[name] + importlib.import_module("schedule_agent.cli") + assert not (tmp_path / "state").exists() + assert not (tmp_path / "data").exists() + + +# --------------------------------------------------------------------------- +# TUI pure helpers: schedule resolver funcs and column layout +# --------------------------------------------------------------------------- + + +def test_resolve_offset_pick_produces_minutes_spec(app_modules): + cli = app_modules.cli + assert cli._resolve_offset_pick(1, 30) == "now + 90 minutes" + # Zero rounds up to 1 minute (avoids "now" ambiguity in at(1)). + assert cli._resolve_offset_pick(0, 0) == "now + 1 minutes" + + +def test_resolve_clock_pick_rolls_over_to_tomorrow_when_past(app_modules): + cli = app_modules.cli + from datetime import datetime + + # Pick a clock time that's definitely in the past relative to "now": + # 00:01 yesterday. The helper must produce a timestamp STRICTLY in the + # future, so if 00:01 today has passed it should produce 00:01 tomorrow. + spec = cli._resolve_clock_pick(0, 1) + # spec format is "YYYY-MM-DD HH:MM" + target = datetime.strptime(spec, "%Y-%m-%d %H:%M") + assert target > datetime.now() + + +def test_layout_mode_transitions(app_modules): + cli = app_modules.cli + assert cli._layout_mode(70) == "narrow" + assert cli._layout_mode(80) == "medium" + assert cli._layout_mode(120) == "wide" + assert cli._layout_mode(200) == "xwide" + + +def test_summary_columns_widen_title_with_budget(app_modules): + cli = app_modules.cli + narrow = dict(cli._summary_columns("narrow", 80)) + medium = dict(cli._summary_columns("medium", 100)) + assert narrow["title"] >= cli.TITLE_MIN + assert medium["title"] >= cli.TITLE_MIN + # Wider terminals give title more room than narrow does. + wide = dict(cli._summary_columns("xwide", 200)) + assert wide["title"] >= medium["title"] + + +# --------------------------------------------------------------------------- +# Feature: post-job hook receives env and is fire-and-forget +# --------------------------------------------------------------------------- + + +def test_post_hook_fires_with_job_env(app_modules, monkeypatch): + operations = app_modules.operations + monkeypatch.setenv("SCHEDULE_AGENT_POST_HOOK", "/bin/echo hookran") + + captured: dict[str, dict] = {} + + class FakePopen: + def __init__(self, argv, env=None, **kwargs): + captured["argv"] = argv + captured["env"] = env + + monkeypatch.setattr(operations.subprocess, "Popen", FakePopen) + operations._fire_post_hook( + { + "id": "abc", + "title": "t", + "last_exit_code": 0, + "last_log_file": "/tmp/x.log", + }, + result="success", + ) + assert captured["argv"] == ["/bin/echo", "hookran"] + env = captured["env"] + assert env["JOB_ID"] == "abc" + assert env["JOB_RESULT"] == "success" + assert env["JOB_LOG_FILE"] == "/tmp/x.log" + + +def test_post_hook_is_noop_when_env_unset(app_modules, monkeypatch): + operations = app_modules.operations + monkeypatch.delenv("SCHEDULE_AGENT_POST_HOOK", raising=False) + + def fail_popen(*args, **kwargs): + raise AssertionError("Popen must not be called when hook is unset") + + monkeypatch.setattr(operations.subprocess, "Popen", fail_popen) + operations._fire_post_hook({"id": "abc"}, result="success") diff --git a/tests/test_cli_doctor.py b/tests/test_cli_doctor.py index fca46bb..2748ce2 100644 --- a/tests/test_cli_doctor.py +++ b/tests/test_cli_doctor.py @@ -18,7 +18,7 @@ def _result(name: str, severity: str, message: str = "", detail: dict | None = N def _install_report(monkeypatch, app_modules, results): report = preflight.PreflightReport(results=results) monkeypatch.setattr( - app_modules.cli.preflight if hasattr(app_modules.cli, "preflight") else preflight, + app_modules.cli.preflight, "run_checks", lambda include_roundtrip=False: report, ) diff --git a/tests/test_environment.py b/tests/test_environment.py index 4615eb5..9ecd778 100644 --- a/tests/test_environment.py +++ b/tests/test_environment.py @@ -176,6 +176,24 @@ def fake_run(cmd, **kwargs): def test_probe_agent_unknown_version_warn(monkeypatch): monkeypatch.setattr(environment.shutil, "which", lambda _: "/fake/claude") + def fake_run(cmd, **kwargs): + if "--version" in cmd: + # Below the 2.1.112 minimum → flagged as not known-good. + return _Proc(stdout="1.0.0 (ancient)", returncode=0) + return _Proc( + stdout="--resume --dangerously-skip-permissions", + returncode=0, + ) + + monkeypatch.setattr(environment.subprocess, "run", fake_run) + probe = probe_agent("claude") + assert probe.version == "1.0.0" + assert probe.version_known_good is False + + +def test_probe_agent_future_version_ok(monkeypatch): + monkeypatch.setattr(environment.shutil, "which", lambda _: "/fake/claude") + def fake_run(cmd, **kwargs): if "--version" in cmd: return _Proc(stdout="9.9.9 (future)", returncode=0) @@ -187,7 +205,7 @@ def fake_run(cmd, **kwargs): monkeypatch.setattr(environment.subprocess, "run", fake_run) probe = probe_agent("claude") assert probe.version == "9.9.9" - assert probe.version_known_good is False + assert probe.version_known_good is True def test_probe_agent_help_missing_substring(monkeypatch): diff --git a/tests/test_execution.py b/tests/test_execution.py index 6e5f270..e95dbbd 100644 --- a/tests/test_execution.py +++ b/tests/test_execution.py @@ -110,10 +110,23 @@ def test_agents_config_has_required_keys(): assert "base_args" in cfg -def test_prompt_prefix_is_prepended(tmp_path, monkeypatch): - monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path)) +def test_prompt_prefix_snapshot_is_prepended(tmp_path): + # Prefix is now a per-job immutable snapshot written at submit time by + # scheduler_backend; build_agent_cmd simply reads the path from + # job["prefix_snapshot_file"] and cats it before the prompt. + snapshot = tmp_path / "prefix.snapshot" + snapshot.write_text("PREFIX TEXT\n", encoding="utf-8") + cmd = build_agent_cmd( + _job("claude", prompt_file="/tmp/p.md", prefix_snapshot_file=str(snapshot)) + ) + assert str(snapshot) in cmd + assert cmd.index(str(snapshot)) < cmd.index("/tmp/p.md") + + +def test_prompt_prefix_absent_when_no_snapshot(): + # Legacy job records (pre-snapshot era) lack the field; build_agent_cmd + # omits the prefix fragment entirely rather than referencing a file + # that will never exist. cmd = build_agent_cmd(_job("claude", prompt_file="/tmp/p.md")) - # prefix file path for claude should appear before the prompt cat - prefix_token = "prompt-prefix-claude.md" - assert prefix_token in cmd - assert cmd.index(prefix_token) < cmd.index("/tmp/p.md") + assert "prefix.snapshot" not in cmd + assert "cat /tmp/p.md" in cmd diff --git a/tests/test_operations_preflight.py b/tests/test_operations_preflight.py index 3a1b57f..3fb6491 100644 --- a/tests/test_operations_preflight.py +++ b/tests/test_operations_preflight.py @@ -80,7 +80,7 @@ def test_create_job_populates_provenance_fields(app_modules, monkeypatch): ops = app_modules.operations monkeypatch.setenv("PATH", "/home/u/.local/bin:/usr/bin") monkeypatch.setattr( - environment, "capture_path", lambda raw=None: ["/home/u/.local/bin", "/usr/bin"] + ops.environment, "capture_path", lambda raw=None: ["/home/u/.local/bin", "/usr/bin"] ) monkeypatch.setattr(ops, "_submit_preflight", lambda agent: (_report(_pass()), _probe())) job = _create(app_modules, monkeypatch) diff --git a/tests/test_preflight.py b/tests/test_preflight.py index 26b9244..a31dbcc 100644 --- a/tests/test_preflight.py +++ b/tests/test_preflight.py @@ -327,6 +327,7 @@ def fake_session(agent, sev): return _cr(f"session_dir_{agent}") monkeypatch.setattr(preflight, "check_session_dir", fake_session) + monkeypatch.setattr(preflight, "check_prompt_prefix", lambda a: _cr(f"prompt_prefix_{a}")) monkeypatch.setattr(preflight, "check_at_roundtrip", lambda: _cr("at_roundtrip")) report = preflight.run_checks() @@ -339,6 +340,8 @@ def fake_session(agent, sev): "agent_codex", "session_dir_claude", "session_dir_codex", + "prompt_prefix_claude", + "prompt_prefix_codex", ] # claude agent PASS → session dir called with PASS; codex FAIL → called with FAIL assert session_calls == [("claude", "PASS"), ("codex", "FAIL")] @@ -350,11 +353,12 @@ def test_run_checks_include_roundtrip(monkeypatch): monkeypatch.setattr(preflight, "check_xdg_dirs", lambda: _cr("xdg_dirs")) monkeypatch.setattr(preflight, "check_agent", lambda a: _cr(f"agent_{a}")) monkeypatch.setattr(preflight, "check_session_dir", lambda a, s: _cr(f"session_dir_{a}")) + monkeypatch.setattr(preflight, "check_prompt_prefix", lambda a: _cr(f"prompt_prefix_{a}")) monkeypatch.setattr(preflight, "check_at_roundtrip", lambda: _cr("at_roundtrip")) report = preflight.run_checks(include_roundtrip=True) assert report.results[-1].name == "at_roundtrip" - assert len(report.results) == 8 + assert len(report.results) == 10 def test_preflight_report_methods(): From 64ea41ad6684661074af018b5465c9929056aa14 Mon Sep 17 00:00:00 2001 From: Tom Pepels Date: Thu, 23 Apr 2026 22:58:14 +0100 Subject: [PATCH 2/3] Update .gitignore to include coverage files and remove obsolete CLI update plan document --- .gitignore | 2 + promts/CLI_UPDATE_PLAN.md | 570 -------------------------------------- 2 files changed, 2 insertions(+), 570 deletions(-) delete mode 100644 promts/CLI_UPDATE_PLAN.md diff --git a/.gitignore b/.gitignore index 0245634..9872407 100644 --- a/.gitignore +++ b/.gitignore @@ -24,3 +24,5 @@ __pycache__/ # Docker .dockerignore.worktrees/ /logs/ + +.coverage \ No newline at end of file diff --git a/promts/CLI_UPDATE_PLAN.md b/promts/CLI_UPDATE_PLAN.md deleted file mode 100644 index 8226ec9..0000000 --- a/promts/CLI_UPDATE_PLAN.md +++ /dev/null @@ -1,570 +0,0 @@ -# Jobs Screen Refactor Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use `superpowers:subagent-driven-development` (recommended) or `superpowers:executing-plans` to implement this plan phase-by-phase. Steps use checkbox (`- [ ]`) syntax for tracking. - -## Goal - -Make the jobs screen the single authoritative interactive surface for job management. - -The fullscreen jobs screen must support all common actions directly inside the screen, without launching separate modal mini-apps that call `.run()` against prompt_toolkit dialogs from inside the running TUI. - -The jobs screen must become structurally simple, width-aware, and comfortable in a normal terminal. - -## Required outcome - -After this refactor: - -* [ ] The fullscreen jobs screen is the primary interactive UI. -* [ ] All job actions are initiated and completed from inside the jobs screen. -* [ ] The jobs screen no longer mixes fullscreen TUI control flow with prompt_toolkit dialog `.run()` calls from helper functions. -* [ ] The jobs screen no longer uses `asyncio.run()` inside the active TUI event loop. -* [ ] The jobs list is compact and width-aware. -* [ ] The left pane is a summary list, not a pipe-delimited fake table. -* [ ] Secondary metadata moves to the detail pane. -* [ ] The UI works at normal terminal widths without requiring an unusually wide screen. -* [ ] Message handling is consistent and not overwritten by generic success text. -* [ ] Legacy mixed-interaction behavior is removed rather than preserved. - -## Product decision - -The jobs screen is a real application surface, not a wrapper around a chain of separate prompt dialogs. - -That means the following patterns are no longer valid inside the jobs screen: - -* calling `input_dialog(...).run()` from a keybinding -* calling `yes_no_dialog(...).run()` from a keybinding -* calling `radiolist_dialog(...).run()` from a keybinding -* returning either a bool or a coroutine depending on loop state -* calling `asyncio.run()` from a TUI action -* using ad hoc terminal handoff logic to patch over the above - -These patterns may remain available for non-TUI CLI flows if needed, but **must not be used by the fullscreen jobs screen**. - -## Root problems to fix - -### 1. Mixed interaction models - -The current file combines: - -* noninteractive command CLI -* modal prompt/dialog helpers -* fullscreen TUI - -The fullscreen TUI currently calls into helpers designed for standalone dialog workflows. This creates conflicting control flow and brittle behavior. - -### 2. Broken event-loop model - -The current jobs screen defines async action plumbing but then calls it from synchronous key handlers without awaiting or scheduling it consistently. The delete flow then tries to force execution with `asyncio.run()` inside the running application loop. - -This must be removed. - -### 3. Table rendering is fake and too wide - -The left pane currently renders pipe-delimited strings as if they were a table. This causes: - -* header/row misalignment -* unstable widths -* poor truncation behavior -* excessive minimum terminal width - -The left pane should instead be a compact summary list with fixed-width rendering rules. - -### 4. Status/message handling is muddy - -Actions set detailed messages, then a generic wrapper may overwrite them with `Updated.` or similar. User feedback must come from one authority. - -### 5. Legacy behavior should not be preserved - -This is pre-alpha. There is no need to preserve mixed legacy UI patterns. Remove obsolete behavior instead of layering compatibility code over it. - -## Design rules - -### Jobs screen ownership - -The fullscreen jobs screen owns: - -* selection -* filtering -* confirmation -* temporary input prompts -* session selection -* status messages -* refresh behavior -* post-action redraw - -### Single interaction model inside the TUI - -Inside the jobs screen, there must be one consistent model: - -* synchronous action dispatch from keybindings -* app-owned overlays / inline prompts / inline confirm UI -* explicit refresh/invalidate after state changes - -Do not mix in separate prompt_toolkit mini-apps. - -### Compact information hierarchy - -The left pane is for quick scanning and navigation only. - -The right pane is for detailed metadata and full summary. - -Do not try to display every field in the left pane. - -### Width-aware UI - -The jobs screen must adapt to narrower terminals. - -Minimum expectation: - -* narrow width: summary pane only, detail view toggleable -* medium width: summary + detail split -* wide width: richer summary columns - -### Remove legacy, do not phase-preserve it - -If old helper patterns or layout behaviors conflict with the above, remove them. - -## Target screen behavior - -## Main layout - -The jobs screen should have 3 regions: - -1. Header bar -2. Main body -3. Footer/status bar - -### Header bar - -Shows concise global state: - -* app name or screen name -* current filter -* local timezone -* optional count summary - -Example shape: - -`Jobs Filter: Active TZ: WEST 8 items` - -### Main body - -The main body should render one of these modes depending on available width: - -#### Mode A: split view - -* left: job summary list -* right: selected job detail - -#### Mode B: list-only view - -* full width summary list -* enter/tab toggles detail overlay or detail mode for selected job - -### Footer/status bar - -Footer shows: - -* key hints -* current contextual status message - -Keep hints compact and stable. - -## Left pane redesign - -Replace the current pipe-delimited fake table with a structured summary row renderer. - -### Left pane columns - -Default summary fields should be limited to: - -* title -* state/status - n- scheduled time -* agent/session marker - -Optional fifth compact field if width allows: - -* queue/submission marker - -### Fields that must move out of the left pane - -Move these to detail only: - -* created time -* updated time -* dependency id -* full scheduler label -* full prompt path -* full cwd -* verbose lifecycle fields - -### Row behavior - -Each row must: - -* truncate predictably -* use fixed column widths per active layout mode -* visually distinguish selected row -* visually soften completed/removed/invalid items -* not rely on pipe separators for alignment - -### Recommended row rendering approach - -Implement a width-aware formatter that returns one formatted row string from a defined column spec. - -For example, define summary column policies such as: - -* title: flex, min 18, ideal 28, max 40 -* status: fixed 10 -* run_at: fixed 16 -* agent/session: fixed 12 - -Then derive actual widths from terminal width. - -## Detail pane redesign - -The detail pane becomes the authoritative full job inspector. - -It should show: - -* job id -* title -* current display state -* scheduler/submission state -* scheduled time -* created time -* updated time -* agent -* session mode / session id -* cwd -* dependency info -* prompt file path -* queue id / at job id when relevant -* full formatted summary text if useful - -The detail pane can be verbose because it only serves the selected item. - -## Actions that must remain inside the jobs screen - -The jobs screen must support these actions directly: - -* [ ] new job -* [ ] edit prompt -* [ ] reschedule -* [ ] change session -* [ ] unschedule -* [ ] submit / repair -* [ ] retry -* [ ] delete -* [ ] refresh -* [ ] cycle filter -* [ ] toggle detail / responsive mode behavior - -## Input model for actions - -For actions that require extra input, use app-owned overlays or inline prompt components. - -### Required overlays / inline prompts - -* [ ] confirm delete -* [ ] input schedule spec -* [ ] session picker -* [ ] transient informational message area - -Do not use standalone dialog helpers from the fullscreen app. - -## Action architecture - -## Replace async action wrapper with synchronous dispatch - -The jobs screen should use a single synchronous action dispatcher. - -Recommended shape: - -* keybinding handler calls a local action function -* action function performs synchronous operation -* action function returns a user-facing message string or raises `OperationError` -* dispatcher stores message, refreshes view, invalidates app - -### Rules - -* no `asyncio.run()` -* no return type that changes between bool/coroutine -* no un-awaited coroutine creation in key handlers -* no action wrapper that overwrites specific messages with generic text - -### Message model - -Every action should either: - -* return a success message -* raise `OperationError` -* raise `KeyboardInterrupt` / explicit cancellation signal - -The dispatcher alone should convert those into footer/status text. - -## Confirmation model - -Delete confirmation must happen inside the jobs screen. - -Implement one of these patterns: - -* a centered confirm overlay with Yes/No keybindings -* an inline footer prompt like `Delete job abc123? [y/N]` - -Do not call standalone confirm dialogs from the active TUI. - -## Session selection model - -Session choice must also happen inside the jobs screen. - -Do not call a separate radiolist dialog from the TUI. - -Recommended behavior: - -* open a session picker overlay -* show `New session` plus discovered sessions -* filterable or scrollable if needed -* confirm selection and return to jobs screen - -## Prompt editing model - -Editing in `$EDITOR` is allowed, but must be treated as a controlled suspend/resume operation. - -Required behavior: - -* [ ] suspend/leave the visual screen cleanly -* [ ] launch editor on the prompt file -* [ ] resume the jobs screen -* [ ] refresh prompt metadata/state -* [ ] show result message - -This must be implemented as a dedicated TUI-safe path, not by reusing legacy modal helper logic. - -## New job flow inside the jobs screen - -The new job flow must stay within the jobs screen UX. - -At minimum it should support: - -* agent selection -* session selection (new/resume) -* schedule input -* prompt editing / prompt capture -* submit on creation - -This can be a staged overlay flow inside the same application. - -It does not need to be perfect on the first pass, but it must no longer rely on nested standalone dialogs launched from inside the fullscreen app. - -## Concrete refactor requirements - -## Phase 1: stabilize the event model - -* [ ] Remove the async `run_action` pattern from the jobs screen. -* [ ] Remove any `asyncio.run()` usage from jobs screen action paths. -* [ ] Make all jobs screen actions synchronous from the TUI’s perspective. -* [ ] Remove any helper that returns a coroutine sometimes and a plain value other times for TUI use. -* [ ] Ensure every keybinding action fully executes or cleanly cancels without loop errors. - -### Phase 1 acceptance criteria - -* [ ] Delete no longer crashes with event-loop errors. -* [ ] No jobs screen action relies on `asyncio.run()`. -* [ ] No TUI keybinding silently drops an un-awaited coroutine. - -## Phase 2: stop using standalone dialogs inside the fullscreen app - -* [ ] Identify all jobs screen flows that call prompt_toolkit `.run()` dialogs indirectly or directly. -* [ ] Replace them with jobs-screen-owned overlays or inline prompts. -* [ ] Ensure confirm, input, and list selection are handled inside the TUI. - -### Phase 2 acceptance criteria - -* [ ] Delete confirmation is TUI-native. -* [ ] Reschedule input is TUI-native. -* [ ] Session selection is TUI-native. -* [ ] Filter control is TUI-native. -* [ ] No jobs screen action depends on `input_dialog`, `yes_no_dialog`, `radiolist_dialog`, or `message_dialog`. - -## Phase 3: redesign the left pane - -* [ ] Remove the current pipe-delimited header/body rendering model for the left pane. -* [ ] Implement a compact summary row renderer with explicit column widths. -* [ ] Reduce the summary fields to a small scan-friendly set. -* [ ] Move secondary metadata to the detail pane. -* [ ] Add truncation/ellipsis behavior where needed. - -### Phase 3 acceptance criteria - -* [ ] Header and rows visually align. -* [ ] The screen is usable in a normal terminal width. -* [ ] The left pane remains readable without horizontal sprawl. - -## Phase 4: responsive layout behavior - -* [ ] Add width-aware layout selection. -* [ ] Support at least list-only mode and split mode. -* [ ] Add a keybinding to toggle detail when in narrow mode, if automatic split is not possible. -* [ ] Ensure the layout does not assume an excessively wide terminal. - -### Phase 4 acceptance criteria - -* [ ] Narrow terminals remain usable. -* [ ] The user can still access detail information without widening the screen excessively. - -## Phase 5: unify messaging and action feedback - -* [ ] Centralize success/error/cancel message handling. -* [ ] Remove generic action wrapper text that overwrites specific messages. -* [ ] Ensure each action returns a precise, user-facing result. - -### Phase 5 acceptance criteria - -* [ ] Footer/status text consistently reflects the last action. -* [ ] Specific messages are preserved. - -## Phase 6: remove conflicting legacy TUI behavior - -* [ ] Remove TUI code paths that depend on mixed dialog/TUI assumptions. -* [ ] Remove obsolete helper usage from the fullscreen jobs screen. -* [ ] Prefer deleting dead or conflicting code over preserving compatibility. - -### Phase 6 acceptance criteria - -* [ ] The jobs screen code path is internally consistent. -* [ ] No legacy mixed-surface behavior remains in the fullscreen TUI. - -## Non-goals - -These are not required for this refactor unless directly needed by the implementation: - -* redesigning argparse command structure -* splitting the file into modules right now -* changing backend job semantics -* changing persistence format -* building a mouse-driven UI -* preserving pre-alpha legacy UI quirks - -## Implementation guidance - -## Keep one file for now - -Implement this in the current file if that is the fastest path. - -Do not block the refactor on code organization purity. The priority is to make the jobs screen correct and coherent first. - -## But structure the code for later extraction - -Even in one file, create clearly separated sections for: - -* TUI state/model -* summary row formatting -* detail formatting -* overlay/input state -* synchronous action dispatch -* keybindings -* layout selection - -This prepares the later split without delaying the refactor. - -## Suggested internal architecture in one file - -Recommended logical sections: - -1. `JobsScreenState` - - * selected index - * filter - * message - * current mode - * overlay state - * cached jobs if needed - -2. `SummaryRowRenderer` - - * width policy - * row formatting - * header formatting - -3. `DetailRenderer` - - * selected job detail formatting - -4. `OverlayState` - - * none / confirm / text input / picker / new-job flow - -5. `dispatch_action(...)` - - * execute action - * catch exceptions - * set message - * refresh invalidate - -6. `jobs_menu()` - - * build layout - * bind keys - * run app - -## Visual design guidance - -The user wants terminal menus with a black background and a more modern feel. - -Within prompt_toolkit styling constraints: - -* use dark background as the baseline -* use restrained contrast -* highlight selection cleanly -* avoid noisy separators -* keep the footer compact -* keep the header calm and informative - -Do not solve visual modernity with more text density. - -## Behavior guidance for list density - -When listing jobs, sessions, or similar items, always show enough information to scan meaningfully. - -For jobs in the main list, that means the user should be able to see at a glance: - -* what the job is -* whether it is active/completed/failed/invalid -* when it will run or ran -* which agent/session context it belongs to - -But this must be done compactly. - -## Removal policy - -Because this project is pre-alpha, prefer the following policy: - -* [ ] if an old TUI behavior conflicts with the new jobs-screen-first design, remove it -* [ ] if a helper exists only to bridge incompatible interaction models, remove or stop using it from the TUI -* [ ] do not keep compatibility code just because it existed first - -## Final acceptance test checklist - -* [ ] Start the jobs screen in a normal-width terminal and verify it is usable without excessive width. -* [ ] Navigate up/down through jobs. -* [ ] Cycle filters. -* [ ] Open selected job details. -* [ ] Reschedule a job entirely from the jobs screen. -* [ ] Change session entirely from the jobs screen. -* [ ] Unschedule from the jobs screen. -* [ ] Submit/repair from the jobs screen. -* [ ] Retry from the jobs screen. -* [ ] Delete from the jobs screen without event-loop errors. -* [ ] Edit prompt and return cleanly to the jobs screen. -* [ ] Confirm specific status messages appear after actions. -* [ ] Confirm there are no dialog `.run()` calls used by the fullscreen jobs screen path. -* [ ] Confirm there is no `asyncio.run()` in the fullscreen jobs screen path. -* [ ] Confirm obsolete mixed-interaction TUI code has been removed. - -## Definition of done - -This refactor is done when the jobs screen is a coherent standalone interface that manages jobs directly, works in a normal terminal, and no longer depends on mixed modal-dialog behavior or broken async bridging. - -At that point, the code can be split into separate modules later without changing the interaction model again. From a25384be2fa854bd52393792573f8d11ed69dd6c Mon Sep 17 00:00:00 2001 From: Tom Pepels Date: Fri, 24 Apr 2026 10:21:17 +0100 Subject: [PATCH 3/3] fixes after audit --- schedule_agent/cli.py | 38 ++++++++++++++++++++++++++++++++------ tests/test_audit_fixes.py | 18 ++++++++++++++++++ 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/schedule_agent/cli.py b/schedule_agent/cli.py index d9b915c..c8cef2e 100644 --- a/schedule_agent/cli.py +++ b/schedule_agent/cli.py @@ -931,11 +931,21 @@ def render_detail(job: dict | None) -> str: def _input_char_accept(ch: str) -> bool: + # Accept single characters (keystrokes) AND multi-char runs (paste). + # Session ids contain uuid-shaped strings; a keyboard paste arrives as + # a single event.data blob when bracketed paste is active, and as a + # burst of single-char events otherwise — both paths must land in the + # input buffer. if not ch: return False - if len(ch) != 1: - return False - return ch in _PRINTABLE_CHARS + return all(c in _PRINTABLE_CHARS for c in ch) + + +def _sanitize_paste(text: str) -> str: + # Strip non-printable control bytes (newlines, tabs, ESC sequences) + # while preserving every accepted printable character. Used for both + # bracketed-paste events and oversize bursts. + return "".join(c for c in text if c in _PRINTABLE_CHARS) # --- dispatch_action ------------------------------------------------------ @@ -1695,11 +1705,27 @@ def _input_backspace(event): def _input_clear(event): state.overlay.buffer = "" + # Bracketed paste (Ctrl+Shift+V, Cmd+V, terminal paste keystroke) arrives + # as a single Keys.BracketedPaste event with the full pasted string in + # event.data. Without this handler the handler below sees it but + # rejects multi-char runs (and earlier it required len == 1). + from prompt_toolkit.keys import Keys + + @kb.add(Keys.BracketedPaste, filter=input_overlay) + def _input_paste(event): + pasted = _sanitize_paste(event.data or "") + if pasted: + state.overlay.buffer += pasted + @kb.add("", filter=input_overlay) def _input_any(event): - ch = event.data or "" - if _input_char_accept(ch): - state.overlay.buffer += ch + data = event.data or "" + # Filter non-printable control bytes but preserve the rest of the + # burst — terminals that do not emit bracketed paste deliver paste + # as one multi-char data blob here. + clean = _sanitize_paste(data) + if clean: + state.overlay.buffer += clean # ----- overlay: picker ----- @kb.add("up", filter=picker_overlay) diff --git a/tests/test_audit_fixes.py b/tests/test_audit_fixes.py index 2e55e0e..499e503 100644 --- a/tests/test_audit_fixes.py +++ b/tests/test_audit_fixes.py @@ -343,6 +343,24 @@ def __init__(self, argv, env=None, **kwargs): assert env["JOB_LOG_FILE"] == "/tmp/x.log" +def test_input_char_accept_allows_paste_burst(app_modules): + cli = app_modules.cli + # Multi-char printable burst (a paste of a UUID-shaped session id). + assert cli._input_char_accept("a1b2c3d4-5678-90ab-cdef-1234567890ab") + # Single char works. + assert cli._input_char_accept("x") + # Empty is rejected. + assert not cli._input_char_accept("") + # Any control byte in the run causes rejection. + assert not cli._input_char_accept("abc\n") + + +def test_sanitize_paste_strips_control_bytes(app_modules): + cli = app_modules.cli + pasted = "session-id-abc\n\t\x1bdef" + assert cli._sanitize_paste(pasted) == "session-id-abcdef" + + def test_post_hook_is_noop_when_env_unset(app_modules, monkeypatch): operations = app_modules.operations monkeypatch.delenv("SCHEDULE_AGENT_POST_HOOK", raising=False)