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/.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" </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..c8cef2e 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")) @@ -859,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 ------------------------------------------------------ @@ -896,10 +978,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 +989,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 +1014,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 +1035,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 +1108,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 +1229,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 +1256,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 +1333,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 +1395,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 +1459,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 +1519,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 +1571,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", @@ -1594,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) @@ -1671,30 +1798,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 +1835,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 +1855,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 +1893,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 +1966,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 +1996,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 +2087,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 +2139,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 +2207,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 +2284,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 +2317,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..499e503 --- /dev/null +++ b/tests/test_audit_fixes.py @@ -0,0 +1,372 @@ +"""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_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) + + 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():