fix(jobs): scope jobs ls state-file rows to the resolved --where target (add --all)

[mattmillerai]

ELI-5

comfy jobs ls has two sources: the ComfyUI/cloud server it queries, and the
job state files this CLI wrote to disk when you submitted something. --where
only ever routed the server half — the state-file half read every file
regardless of which target the job ran on. So comfy --json --where local jobs ls
happily listed cloud jobs from last week, forever (state files never expire).

Now the state-file rows are scoped to the resolved --where target too, and the
union view is one flag away: --all.

What changed

• _gather_local_state_files(..., where=...) — new keyword filter. Rows
  whose state.where doesn't match are skipped; a missing/empty state.where
  reads as "local" (a defensive default — see the note below). where=None
  (the default) keeps the unfiltered union view, so every existing caller is
  behaviourally unchanged.
• jobs ls resolves the target once (--where flag > COMFY_WHERE env —
  which is how the top-level comfy --where arrives > config default) and
  scopes state rows to it. New --all flag restores the union.
• --orphaned stays unfiltered. Watcher cleanup is where-agnostic — you
  want to see a crashed cloud watcher from a default (local) shell. The
  where filter is also applied after the stale-watcher reap, so reaping
  keeps working the same in every view.
• --watch threads the same scope, so the live table matches jobs ls.
• server_rows are deliberately not filtered — they're already scoped by
  whichever backend was queried.
• Payload gains scope ("local" / "cloud" / "all") so agents can see
  which view they got. Additive, envelope/1 unchanged; documented in
  comfy_cli/schemas/jobs.json and the shipped comfy skill doc.

Behaviour change worth flagging

This narrows the default jobs ls view. Anything relying on "one call shows
everything I ever submitted via this CLI" must now pass --all. That's the
intent — the pretty header already renders a single target's host, so a scoped
list is what the UI claimed to show — but it is a visible change and scope is
there so callers can detect it.

Downstream: comfy-local-mcp's get_queue always passes --where local and is
documented local-only, so it gets correct behaviour for free once this ships.

Verification

uv sync --frozen --extra dev, then:

• pytest tests/ — 2776 passed, 37 skipped.
• ruff check . / ruff format --check . — unchanged from main (the 15
  UP038 findings and the one unformatted file, tests/comfy_cli/command/github/test_pr.py,
  are pre-existing on main; this diff adds none).

New tests (tests/comfy_cli/jobs/test_jobs.py): a seeded temp state dir with a
local, a cloud and a legacy row, covering default/--where cloud/--all/
--orphaned at both the gather level and through ls_cmd's emitted envelope,
plus --all in the machine-readable help contract.

End-to-end, against the real CLI with a seeded state dir (HOME pointed at
a temp tree holding one local + one cloud + one legacy state file):

$ comfy --json --where local jobs ls --port 65431
scope=local where=local rows=[('job-legacy','local'), ('job-local','local')]
$ comfy --json --where local jobs ls --port 65431 --all
scope=all   where=local rows=[('job-legacy','local'), ('job-cloud','cloud'), ('job-local','local')]
$ comfy --json --where local jobs ls --orphaned
scope=all   where=local rows=[]

No capability is removed — the union view was empirically re-attempted after
the change and is fully reachable via --all (second command above).

Judgment calls

1. "Legacy files with no where" don't actually exist in a readable form.
   JobState.where has been a required, non-defaulted field since the state
   file was introduced, so jobs_state.read() already rejects a file that omits
   the key entirely (returns None) — it never reaches the filter. The
   reachable legacy shape is an explicitly null/empty value, which the
   state.where or "local" default handles. Both are pinned by tests. I did
   not give JobState.where a default: that's a shared-dataclass change
   well outside this ticket, and it would resurrect files today treated as
   truncated.
2. scope is "all" for --orphaned, not the target. The ticket says
   scope is "all" or the target; since --orphaned gathers unfiltered, the
   truthful value is "all". scope is derived from the same variable passed
   to the gatherer, so it can't drift from the actual view.
3. Normalised the emitted row where to "local" for a null/empty state
   where (JobRow.where is typed str and already defaults to "local"), so
   a row scoped as local can't report where: null. Nil blast radius —
   jobs_state.new() always writes a target.
4. Replaced the three repeated _is_cloud(where) calls in ls_cmd with the
   single resolved target_where. Same semantics (and still monkeypatchable),
   but it guarantees the filter, the emitted where, and the pretty header can
   never disagree.
5. Doc touch beyond the plan: the shipped comfy skill doc described
   jobs ls as "local state files + server queue", which is now incomplete for
   agents. One short paragraph + an example line were added.

Pre-existing issue observed, not fixed (out of scope)

While verifying the cloud path live: jobs ls --where cloud with credentials
present but the cloud API unreachable raises an uncaught URLError instead of
degrading to the state-file view its own comment promises (only typer.Exit is
caught). And when the preflight fails, its error envelope wins the
one-envelope-per-process guard, so the state-only fallback never reaches a JSON
caller. Both predate this change and are untouched here; the cloud scoping is
covered by unit tests instead.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 55 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7584a37c-49b7-4037-afdf-0a7cdbbc19e8

📥 Commits

Reviewing files that changed from the base of the PR and between 98325c9 and 50dd5e9.

📒 Files selected for processing (4)

• comfy_cli/command/jobs.py
• comfy_cli/schemas/jobs.json
• comfy_cli/skills/comfy/SKILL.md
• tests/comfy_cli/jobs/test_jobs.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch matt/be-4219-jobs-ls-scope-where

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch matt/be-4219-jobs-ls-scope-where

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

Found 1 finding(s).

Severity	Count
🟡 Medium	1

Panel: 6/8 reviewers contributed findings.

Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)

[bigcat88]

This PR currently conflicts with main — GitHub reports mergeable: CONFLICTING, so it needs a rebase before I can review it and I'm skipping it in the current review sweep.

Please rebase (or merge main in) and I'll pick it up on the next pass. main moved a fair bit in the last day, including #614 (ANSI sanitisation across the pretty-print call sites) and #628 (the duplicate server_died error-code fix that had main red), so a refresh may also clear unrelated CI noise on this branch.

[bigcat88]

Approving. I reproduced the bug with real state files on disk (backed up and restored my own job state around it), and every claim in the description holds.

The bug is exactly as described

Six state files, tagged where: local / cloud / "":

main:        comfy --json --where local jobs ls
             scope=None   rows=['cloud-xxx','cloud-yyy','empty-www','local-aaa','local-bbb']

this branch: comfy --json --where local jobs ls
             scope=local  rows=['empty-www','local-aaa','local-bbb']

             comfy --json jobs ls --all
             scope=all    rows=['cloud-xxx','cloud-yyy','empty-www','local-aaa','local-bbb']

main hands a --where local caller two cloud jobs and no scope field to notice with. --all doesn't exist there (No such option: --all), so today there's no way to get the union deliberately — you just always get it.

--orphaned stays where-agnostic, and I checked that specifically

Marked the cloud job running with a dead watcher_pid, then listed orphans from a --where local shell:

comfy --json --where local jobs ls --orphaned
scope=all   rows=['cloud-xxx']

The cloud orphan surfaces from a local shell, exactly as you argued it should — a crashed cloud watcher is precisely what you need to see from a default shell. And the schema's scope description already says --orphaned yields "all", so the payload is self-describing rather than surprising.

One fixture that taught me something

I seeded a record with the where key entirely absent to exercise the "missing reads as local" default, and it never appeared in any view — including --all. That's not a scoping gap: JobState.where is a required non-defaulted field, so jobs_state.read() rejects such a record outright, and jobs_state.write(..., where=where) always supplies one. A key-absent record can't be produced by this CLI.

So the defensive default effectively covers the empty-string case, and that one does work — empty-www lists under --where local and under --all. Worth knowing the "missing" half of that comment is unreachable rather than untested.

Scope decisions all read right

Leaving server_rows unfiltered is correct — they're already scoped by whichever backend answered. Applying the where filter after the stale-watcher reap keeps cleanup identical in every view. And where=None as the default for _gather_local_state_files means every existing caller is behaviourally unchanged, which is what makes this safe to land.

scope is additive, envelope/1 is unchanged, and it's documented in both schemas/jobs.json (enum ["local","cloud","all"]) and SKILL.md — so an agent can tell which view it got instead of inferring it.

Full suite green on this branch merged with current main; ruff check + ruff format --diff clean under the CI-pinned 0.15.15.

(I couldn't exercise --where cloud end to end — this box isn't signed in, so it fails at cloud_not_configured before listing. The state-row filter itself is proven by the --all view showing both cloud rows and --where local excluding them.)