fix(agent-watchdog): bound Codex job scan, cover untested branches; codex-crew commit-doctrine update - #71
Merged
Merged
Conversation
- CodexScanner.scan(): add a 6-hour terminal-job retention window so per-poll jobs/*.json enumeration and state tracking stay bounded on long sessions; active jobs are retained regardless of age; existing mtime cache and one-time slow-glob warning preserved. - tests: add a real `git init` fixture exercising git_toplevel()/ resolve_workspace() Source-D resolution; cover _task_dir, relative _worktrees (the default production path), _subagent_dirs autodetect, and member_transcripts() fallback/ambiguity branches. - grand-admiral/SKILL.md: note that a TaskStop success response for a Monitor-wrapped background process doesn't prove the OS process actually died. exit-144 startup crash (separate TODO) could not be reproduced after multiple attempts (real + synthetic team dirs, strace, direct launch); left open, no speculative fix applied. Verified: python3 -m pytest tests/test_agent_watchdog.py -v -> 63 passed, 0 failed. ruff check / ruff format --check clean.
Empirically verified 2026-07-16 (commit f2639aa, this repo): Codex successfully ran git add/git commit inside a linked worktree, no approval prompt. This reverses codex-crew's prior hard-block guidance. writable_roots was unchanged at test time (still excludes repo .git), so the likely enabling lever is approval_policy="on-request" + the project's trust_level="trusted", not a sandbox-path change - not independently confirmed. Coordinator-commits kept as documented fallback if the old read-only/index.lock error resurfaces. Bump plugin.json to 5.9.0 and update CHANGELOG per repo versioning policy (this PR touches plugin content: scripts/agent-watchdog.py fixes, grand-admiral doc note, codex-crew doc correction).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens agent-watchdog Codex job scanning so long-running sessions don’t accumulate unbounded per-poll work, and adds/extends tests and documentation to cover previously untested/default-path behaviors and updated Codex commit capabilities.
Changes:
- Bound
CodexScanner.scan()by aging out terminal Codex job records older than 6 hours while retaining active jobs. - Added tests covering previously untested default-path branches (git toplevel resolution, task/worktree/subagent autodetect paths, member transcript fallback/ambiguity cases).
- Updated operational/docs guidance (Monitor/TaskStop nuance; Codex can now commit in linked worktrees) and bumped plugin version + changelog.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/agent-watchdog.py | Adds terminal-job retention cutoff to keep Codex job enumeration/state tracking bounded. |
| tests/test_agent_watchdog.py | Adds fixtures/tests to cover default resolution branches and the new retention behavior. |
| skills/grand-admiral/SKILL.md | Documents that TaskStop success may not imply underlying process death for Monitor-wrapped processes. |
| skills/codex-crew/SKILL.md | Updates guidance to reflect that Codex can commit in linked worktrees (with fallback guidance). |
| skills/codex-crew/references/sandbox-and-recovery.md | Updates sandbox/commit behavior documentation with current observed status + fallback mechanics. |
| CHANGELOG.md | Documents the 5.9.0 changes. |
| .claude-plugin/plugin.json | Bumps plugin version to 5.9.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…reversal <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub> Co-Authored-By: Codex GPT-5 <noreply@openai.com>
Per-poll jobs/*.json enumeration was never bounded by the retention window — only the tracked/returned record set is. Enumeration cost still scales with total accumulated terminal job files on disk since nothing prunes them, which is why the slow-glob warning is retained. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
If /ci-dance itself is delegated to a non-lead teammate, its named
Agent() stream spawns fail outright ("teammates cannot spawn other
teammates" - flat team roster). Document the fallback: unnamed
background subagents + rely on Step 3 merge-time reconciliation
instead of the claim/completion SendMessage protocol.
Found by a concurrent session running ci-dance on lklimek/memcan
PR #30 (2026-07-16); folded into PR #71 per user request since both
are claudius-side hardening in the same area.
CodexScanner.scan() discovers a job under a candidate's state_dir/jobs/ but then requires the job's self-reported workspaceRoot to canonical- match the candidate's resolved path before considering it further. Every other skip path in this function (bad state shape, unsupported version, unsafe job id, unknown status) warns before discarding - this one didn't, so a job whose workspaceRoot pointed elsewhere (e.g. the main repo cwd instead of its actual worktree) vanished with zero trace, invisible to every CODEX_* event. Add a warn-once diagnostic naming both paths so this failure mode is observable instead of requiring manual job-file archaeology. Matching behavior itself is unchanged - relaxing the equality check risks misattributing jobs across workspaces, a correctness concern, not just an observability one. Root-caused from a separate session's incident report (dash-evo-tool-2, 2026-07-16) where this exact silent skip caused a completed/failed Codex job to go permanently unreported. Verified: python3 -m pytest tests/test_agent_watchdog.py -v -> 64 passed, 0 failed. ruff check / ruff format --check clean.
Same day, same repo, two dispatches: one committed cleanly (f2639aa), the next hit the exact old read-only .git/index.lock error and had to be committed by the coordinator instead (7c2d3e8). writable_roots was unchanged across both, so whatever gates this isn't a static setting. Reframe coordinator-commit as the reliable default, not a fallback for a regression - always verify independently after a Codex dispatch rather than trusting its self-reported commit success. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…review Rule 2 and sandbox-and-recovery.md already say commit-in-worktree is inconsistent (b418844), but three other spots in SKILL.md still said "now lifted" / "CAN commit ... won't do so unasked" / "(now working)" - stale confident phrasing that contradicted rule 2 and could lead a coordinator to skip the required independent-verify step. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Make teamless Codex monitoring's empty candidate blind spot explicit while preserving existing discovery behavior. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ssions, escape job filename CodexScanner.scan()'s new workspace-mismatch warning fired for every job in a candidate's jobs/ dir before session filtering ran, so a job belonging to a completely unrelated, never-trackable session still tripped "may be silently invisible to CODEX_* events" - noise that undermines the diagnostic's own purpose. Gate the warning on whether the job's sessionId is missing or plausibly matches effective_session (via the existing _prefix_matches helper); the job is still discarded unconditionally either way, matching semantics unchanged. Also escape job_path.name (externally-influenced, written by the external Codex Companion process) with !r in the stderr-bound message, matching the existing workspaceRoot escaping - it was the one un-escaped token reaching print(..., file=sys.stderr). Corrected the existing mismatch test, which used codex_store()'s default sessionId backfill and so accidentally exercised the matching-session path instead of the missing-sessionId path it claimed to cover. Added two new tests for the prefix-matching and unrelated-session cases. Verified: python3 -m pytest tests/test_agent_watchdog.py -> 66 passed, 0 failed. ruff check / ruff format --check clean. Found via grumpy-review iteration 2 (security + QA reviewers, independently confirmed with a repro). Co-Authored-By: Codex Sol <noreply@openai.com>
…commit-reliability framing CHANGELOG.md and sandbox-and-recovery.md both cited commit 963db57 as the Dispatch-2 coordinator-commit; it's a dangling, unreachable orphan (same message as 7c2d3e8, a pre-rebase ghost) that git show fails on in a fresh clone. SKILL.md's Rule 2 already cited only 7c2d3e8 - the three docs disagreed with each other. Drop 963db57, cite 7c2d3e8 consistently. CHANGELOG.md's workspace-mismatch bullet claimed the new warning closed "the one exception" among scan()'s skip paths - false: two more silent continues (corrupt job file, missing workspaceRoot) sit right next to it, still unwarned. This repo already had to correct an identical overclaim once (5567cbf). Reworded to describe the specific gap closed without claiming exclusivity. skills/codex-crew/SKILL.md's intro, routing bullet, and Additional Resources line still said "now lifted" / "CAN commit" / "now working" about linked-worktree git commit, contradicting Rule 2's accurate "confirmed inconsistent - treat coordinator-commit as the reliable default." A coordinator skimming only those spots (common skim points) would land on the wrong takeaway. Reworded all three to match Rule 2's framing; CHANGELOG's lead clause got the same fix. Found via grumpy-review iteration 2 (project-consistency + technical- writer reviewers, independently verified against git history). Co-Authored-By: Codex Sol <noreply@openai.com>
# Conflicts: # CHANGELOG.md
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
scripts/agent-watchdog.py:996
- Terminal-job retention is applied only after session disambiguation (
sessionsis populated from all job files). That means very old terminal jobs can still contribute tosessionsand cause_session()to pick/disable sessions based on stale history, even though those jobs will later be aged out. Apply the retention filter before adding a job'ssessionIdtosessions(and before appending tojobs).
session = raw.get("sessionId")
if isinstance(session, str) and session:
sessions.add(session)
jobs.append((job_path, raw))
…team The codex-zero-candidates warning hardcoded "no team config" as the cause whenever codex_candidates was empty, and prescribed "dispatch at least one NAMED teammate to create a team". Both are false when a team config exists but yields no cwd: parse_team treats the lead cwd and member cwd as optional and drops members that are not isActive, so a valid, session-matched team whose teammates have completed produces zero candidates. poll_once's own `if team.lead_cwd:` / `if member.cwd` guards show the state is anticipated. The warning then denied the existence of a team that select_team had just matched, and suggested creating one that was already there -- a false-premise diagnostic in the same class as the silent-skip gaps this branch exists to close. Branch cause and remedy on team presence. The teamless message is unchanged byte-for-byte, so its exact-stderr test still passes untouched; only the team-present branch is new. Changelog reworded to describe both shapes instead of only the teamless one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mbiguation CodexScanner.scan() folded every job's sessionId into the disambiguation set before the terminal-job retention cutoff was applied, so an aged-out terminal job's session could still count as an ambiguity candidate for _session() — spuriously disabling Source D (or steering selection) long after the job should have been aged out of "downstream state tracking" per the retention window's own stated purpose. Move the cutoff check before jobs/sessions population; active jobs are still retained unconditionally, matching existing behavior. Copilot review finding on PR #71 (review 4713623583, agent-watchdog.py:996). Co-Authored-By: Claudius Agent <noreply@anthropic.com>
Comment on lines
+49
to
+58
| def git_repo(tmp_path: Path) -> Path: | ||
| """Create a real Git workspace for repository-root resolution tests.""" | ||
| result = tmp_path / "git-repo" | ||
| subprocess.run( | ||
| ["git", "init", str(result)], | ||
| check=True, | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| return result |
| ### Changed | ||
|
|
||
| - **`skills/grand-admiral/SKILL.md`** § Terminating Teammates: note that a `TaskStop` success response for a Monitor-wrapped background process doesn't prove the underlying OS process actually died. | ||
| - **`skills/codex-crew/SKILL.md`** + `references/sandbox-and-recovery.md`: Codex `git commit` inside a linked worktree is confirmed inconsistent (observed both ways 2026-07-16). One dispatch committed as `f2639aa`; a later dispatch in a different worktree hit the exact old `index.lock`/read-only error and was committed by the coordinator as `7c2d3e8`. `writable_roots` was unchanged; the likely enabling lever is `approval_policy = "on-request"` + the project's `trust_level = "trusted"`, not a sandbox-path change. Coordinator-commit is the reliable default, not just a fallback for a regression. |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this PR exists
claudius-project TODOs found several live gaps inscripts/agent-watchdog.py's Codex ("Source D") job monitoring — unbounded job-history growth, and several silent-failure paths where a job could go unmonitored with no trace.CodexScanner.scan()globbed every retainedjobs/*.jsonon every poll with no eviction (unbounded per-poll cost over a long session). Separately, several skip paths discarded jobs silently: aworkspaceRootmismatch, a session that resolved to zero candidate workspaces (and, when it did warn, sometimes blamed the wrong cause), and — most seriously — a terminal job'ssessionIdcould still poison session disambiguation after it should have aged out of the retention window, spuriously blanking Source D monitoring for the rest of a poll while presenting as a healthy, quiet fleet.Watchdog._worktrees' default relative-path resolution (the actual default production code path) and four related branches also had zero test coverage.What was done
scripts/agent-watchdog.pyCodexScanner.scan(): added a 6-hour terminal-job retention window (active jobs retained regardless of age); warn once on aworkspaceRootcanonical mismatch, gated to plausible sessions so it doesn't fire for unrelated/untracked sessions; moved the retention cutoff to apply before a job'ssessionIdis folded into session disambiguation, closing a gap where an aged-out terminal job could still trigger a false ambiguity and silently disable monitoring.scripts/agent-watchdog.pyWatchdog.poll_once(): warn once when a session has zero discoverable Codex candidate workspaces, naming the actual cause (no team config, vs. a team whose lead/active members report no cwd) instead of assuming the former unconditionally.tests/test_agent_watchdog.py: 69 tests passing — added coverage for_task_dir,_worktrees(default relative-path resolution),_subagent_dirsautodetect, a realgit initSource-D fixture forgit_toplevel()/resolve_workspace(),member_transcripts()fallback/ambiguity, the workspace-mismatch warning (plus its session-plausibility gating), the zero-candidates warning (both cause branches), and the retention-before-disambiguation ordering fix.skills/grand-admiral/SKILL.md: note that aTaskStopsuccess response for a Monitor-wrapped background process doesn't prove the underlying OS process died.skills/codex-crew/SKILL.md+references/sandbox-and-recovery.md: Codexgit commitin a linked worktree is confirmed inconsistent — observed both succeeding and failing the same day, same repo. Treat coordinator-commit as the reliable default, not a fallback.skills/ci-dance/SKILL.md: documented the flat-team-roster fallback when/ci-danceitself runs as a delegated (non-lead) teammate..claude-plugin/plugin.json/CHANGELOG.md: version bumped to 5.10.0 (bumped once to 5.9.0, then re-bumped after mergingmain/PR feat(grand-admiral): track tasks in memcan TODOs (plain-file fallback) #70, which had independently landed its own 5.9.0 release — see CHANGELOG for both entries).Left open (tracked as separate
claudius-project TODOs, out of scope here):agent-watchdog.pyexiting 144 on startup under--team-dir(investigated with real+synthetic team dirs and strace; not reproducible, no speculative fix applied); CI runsruff format --checkbut neverruff check, so real lint bug classes pass silently; a LOW-severity malformed-job-JSON path that can blind the whole poll rather than just skipping one bad file.Testing
python3 -m pytest tests/test_agent_watchdog.py -q→ 69 passedruff check/ruff format --check→ cleanplugin-dev:plugin-validator→ PASS, no issuesBreaking changes
None.
Checklist
plugin-dev:plugin-validatorrun — PASS🤖 Co-authored by Claudius the Magnificent AI Agent