Skip to content

Session watcher: queue and history for the live author session, launch ledger, --why - #319

Merged
renmengye merged 3 commits into
mainfrom
feat/session-watcher
Sep 7, 2026
Merged

renmengye merged 3 commits into
mainfrom
feat/session-watcher

Conversation

@renmengye

Copy link
Copy Markdown
Member

Rollout step 1 of the session-watcher design note (#316): the author can see the cluster queue and its own launch history while its session runs, instead of guessing why nothing has come back.

What the author gets

  • python .outerloop/syscall queue — the kernel's jobs in the queue right now, every agent's (the board's attribution, squeue --me), each launch with its author's --why, the pending reason, elapsed of limit, gres, plus the GPU lane's node states from sinfo. Answered within seconds.
  • python .outerloop/syscall history — this run's launches sleep by sleep and how each job ended.
  • launch --why "one line" (≤ 200 chars), shown in the queue view to every agent and echoed in the wake text. The brief documents all three.

How

  • watcher.py: a daemon thread started by attempt_once around each run_role (a context-manager factory, watcher=, None in deployments without one). It polls the channel every 2 s for <verb>-request markers, writes <verb>.json, stamps <verb>-done — the sync protocol, generalized in syscall.py (marker_requested, mark_done, write_channel_json, same O_NOFOLLOW/O_EXCL discipline). One scheduler query per 5 s at most; a failed query answers with the error; an exception leaves the request standing and the tool times out with a plain message. It changes no lifecycle state.
  • launchlog.py: the per-run launches.jsonl ledger. _park_run appends one record per launch with its job ids (positional over the array, mirroring _stage_launch_job_ids); the wake appends the ended records. history joins them by (sleep, name); why_by_job labels queue rows.
  • compute: QUEUE_FIELDS gains limit (%l); lane_load(partition) from sinfo; LocalCompute has no lanes and no queue. climbboard.queue_rows passes reason/gres/limit through (the strip's shape check is unaffected).
  • Tests: ledger join and torn-line safety; watcher attribution, rate limit, error path, no-compute, history, the thread, symlinked channel; the tool's verbs and --why; request parsing; park writes the ledger; lane_load.

Not in this PR (step 2): always-queue (removing #315's refusal), sweeps as throttled Slurm arrays with the GPU-counted ceiling, and the per-sweep concurrency context lines.

🤖 Generated with Claude Code

…sion; launch ledger; --why

A thread in the attempt (a context manager around every harness run,
first leg and wakes alike) polls the syscall channel and answers two new
tool verbs within seconds through the sync marker protocol:

- queue: the kernel's jobs as the board projects them (every agent's,
  by agent id), each launch labelled with its author's --why, the
  pending reason, elapsed/limit, gres, and the GPU lane's node states
  from sinfo. Rate-limited to one scheduler query per five seconds
  however often the marker is rewritten; a failed query answers with
  the error instead of hanging.
- history: this run's launches sleep by sleep with how each job ended,
  from a new append-only per-run ledger (launches.jsonl) the park and
  the wake write.

launch --why is a bounded one-line field carried through the request,
the stage, the ledger and the wake text. The channel helpers are
generalized (marker_requested / mark_done / write_channel_json) with the
same O_NOFOLLOW + O_EXCL discipline sync already had. Nothing new enters
the container. Rollout step 1 of docs/design/session-watcher.md.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Round 1 — reviewed head 1ed72cd7 — reviewer summarizer:hermes/gpt-5.6-terra over coverage+credentials+deployment+general+lifecycle+prose.

terra
Advisory findings from outerloop — the code owner decides. Reply to disagree; the outerloop:no-review label opts this PR out.

Verdict: 2 blocking, 3 advisory.

4 findings attached to the lines below.

Advisory (non-blocking):

  • Queue and history can return a stale answer when mtimes collide. [lifecycle] When two requests receive the same filesystem mtime, the prior done value equals started at lines 898–904. The client can then read the previous JSON without the watcher serving the new request. (src/outerloop/syscall_cli.py:898; medium confidence)

Verdict: block on two lifecycle/resource-handling defects: unvalidated FIFO done markers can permanently block the watcher, and a Slurm query can outlive the session after the 10-second join. Also address the queue view making two scheduler calls per uncached request, surfacing GPU lane-query failures instead of returning an empty successful response, and preventing stale queue/history replies when request mtimes collide. Rejected: the general lens finding at src/outerloop/watcher.py:162 was not recorded separately because it duplicates the same hidden lane-query-failure defect merged with the deployment lens; coverage and prose reported no findings.

Comment thread src/outerloop/syscall.py Outdated
Comment thread src/outerloop/watcher.py Outdated
Comment thread src/outerloop/watcher.py
Comment thread src/outerloop/watcher.py
… sinfo a minute, lane errors said, no stale answers

- _read_done opens O_NONBLOCK and requires a regular file, so a FIFO
  planted as a done marker cannot hang the watcher thread (or the tick's
  sync service, which shared the bug).
- service() writes nothing once the watcher is stopped; the attempt waits
  15 s for an in-flight scheduler query, then moves on.
- The lane's node states are cached for a minute; a failed sinfo is
  reported in the answer and rendered as unavailable, not as no state.
- The tool pushes a request whose mtime would not exceed the previous
  acknowledgement one second past it (sync included), so two requests
  within the filesystem's mtime resolution never read the same answer.
- The verb tests drive the real channel protocol from a fake kernel
  thread instead of a pre-stamped marker.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@renmengye renmengye added the outerloop:review re-request the advisory review label Sep 7, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Round 1 — reviewed head 6672084a — reviewer hermes/gpt-5.6-terra.

terra
Advisory findings from outerloop — the code owner decides. Reply to disagree; the outerloop:no-review label opts this PR out.

Verdict: nothing blocking — 1 advisory note.

Advisory (non-blocking):

  • Re-parking a submitted run drops each launch's why. The submitted re-park rebuilds Launch objects without why, then _park_run appends blank submitted records that overwrite the existing ledger labels, so queue and the eventual wake lose the author-provided reason after a multi-stage gate. (src/outerloop/attempt.py:1707; high confidence)

Submitted runs that re-park during a multi-stage gate lose launch reasons in the ledger and wake output.

…d its why

The submitted re-park rebuilt Launch objects without why and _park_run
appended blank submitted records over the first ones. The rebuild carries
why from the stage, and append_submitted skips a (sleep, name) already in
the ledger, so the record with the author's words stands.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@renmengye renmengye added outerloop:review re-request the advisory review and removed outerloop:review re-request the advisory review labels Sep 7, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Round 2 — reviewed head 4b75993f — reviewer hermes/gpt-5.6-terra.

terra
Advisory findings from outerloop — the code owner decides. Reply to disagree; the outerloop:no-review label opts this PR out.

Verdict: no defects found.

@renmengye
renmengye merged commit 9044406 into main Sep 7, 2026
5 checks passed
@renmengye
renmengye deleted the feat/session-watcher branch September 7, 2026 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

outerloop:review re-request the advisory review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant