feat(cron): bind scheduled jobs to a project's agents

[mbajaj92]

Problem / Motivation

Cron jobs can only run global agents from ~/.kiro/agents/. A project that defines its own agents in <project>/.kiro/agents/*.json cannot use them from a scheduled job — the Schedule page's agent picker only ever lists global agents, and even naming a project agent by hand at fire time silently falls back to the default global agent.

Why it matters

Project-scoped agents are how a repo customizes tool access, prompts, and permissions for its own workflows. Anyone who wants a cron job to act as one of those agents (e.g. a nightly job that runs a repo's own release-checklist agent) currently cannot — the job always runs as a global agent instead, with no error or warning that anything was substituted.

What changed (motivation → approach → change)

The gap has two parts: the dashboard's agent picker only reads global agents, and cron fire-time execution only resolves agent names against the global registry. Both needed the same missing piece — a way to ask "what agents does this folder define" — so a scheduled job's project_path became the one new field threading through both.

On the backend, CronJob gains an optional project_path (validated the same way project directories are validated elsewhere: absolute path, not a sensitive path, must exist). GET /api/agents gained a project_path query-param fallback for callers with no live chat session behind them (the Schedule page's job form has no session to key off of). Cron fire-time execution resolves job.project_path through the existing project-agent-discovery path before launching, so the job's agent behaves exactly like a chat session opened in that folder.

A folder that existed when the job was saved but is gone by fire time is treated as a normal failure, not run against the wrong (global) agent: the job is skipped, marked last_status: error with a message naming the missing folder, and no auto-pause strike is spent — mirroring how an "overlapping run" refusal is already handled elsewhere in this file.

On the frontend, the job form gained an "Operating folder" field reusing the same folder picker component already used for new-chat and folder settings, so the interaction is consistent across the app. Selecting a folder merges its project-scoped agents into the picker (global agents win on a name collision), and the field's own validation errors are shown as a field-scoped message rather than a raw backend string.

Tests

• test/test_cron.py: default-empty, valid-path persists across a reload, nonexistent/relative/sensitive-path rejection, update sets/clears the path, an invalid update leaves the existing path unchanged.
• test/test_dashboard_cron_approval.py: project_path/project_path_missing present (and correctly typed) in the job list response, and None when absent.
• test/test_cron_gateway_integration.py: a missing operating folder at fire time skips the run and records an error (both the single-agent and sequential-agent paths), the flag persists, and a job whose folder still exists runs normally (control case).
• test/test_api_agents_order.py / test/test_agents_roster_contract.py: existing tests updated for a session-precedence fix found while building this (see below).
• website/src/test/JobForm.saveError.test.tsx: a rejected save surfaces the real backend message (not a generic "failed to save"), including the three project-path-specific error remaps.

While building this, live testing surfaced a real precedence bug in the existing GET /api/agents handler: it resolved project scope through a helper whose "single project shared by every open slot" fallback fired for any session key, including the sentinel every request without a real session sends. That let an unrelated open chat tab's project silently override an explicit project_path query param. Fixed by switching to the narrower session-scoped resolver that returns None for a sentinel key with no cross-slot fallback — unrelated to this feature's own code path, but found and fixed in the same diff since the new fallback depends on it behaving correctly.

Manual verification

Live-tested end to end against a dev gateway: created a cron job with an operating folder set, confirmed the agent picker showed that folder's project agents merged with globals, saved and re-opened the job to confirm the folder persisted and round-tripped correctly, and fired the job to confirm it ran the resolved project agent in the correct working directory. Also verified the missing-folder path by deleting a job's folder after save and confirming the next fire skipped with a normal error rather than running against the wrong agent.

Screenshots / video

Related Issues

Closes #8952

Pattern harvest

Not generalizable: the session-precedence fix (requesting_slot_project vs. the shared-fallback resolver) is specific to this one call site's existing behavior, not a repeated pattern elsewhere in the diff.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[mbajaj92]

Intent: Let a scheduled cron job optionally bind to a project directory, so both the Schedule page's agent picker and fire-time execution can use that project's own .kiro/agents/*.json agents instead of only global ones.
Not a goal: Per-project memory stores, cron-folder-level path binding (path lives on the individual job), or changing agent-file existence checks at launch time (that is a separate, pre-existing gap tracked outside this PR).

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 7b37afb197491cbb9cd105279d1ece44dea7390c via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound, root-cause design mirroring chat's project-agent resolution — but the PR ships stray binary screenshots and describes a project_path_missing field that doesn't exist in the diff.

Watch

• Two PNGs under temp-screenshots/ are added to the tree (diff --git a/temp-screenshots/...). Merging embeds throwaway binaries permanently into main's history — the description already links them at a branch commit, so the in-tree copies serve no one.
  Clears when: the temp-screenshots/ directory is dropped from the PR.
• Phantom description: the tests section claims "project_path/project_path_missing present … in the job list response," and a gateway comment references "a list-page badge," but project_path_missing appears nowhere in the patch. As shipped, a vanished folder is only discoverable after a skipped fire via last_status: error — either ship the flag or correct the description so reviewers and users aren't promised a pre-fire signal that doesn't exist.
  Clears when: the field/badge is added, or the description and the gateway comment stop claiming it.
• The owning spec is untouched: docs/system-specs/modules/learn-cron-dashboard.md still says "gateway passes agent=job.agent_id to get_or_create()" and enumerates the job's first-save fields — this PR adds a persisted project_path, fire-time binding resolution, cwd, and a stale-session reset, so the spec now documents the old contract (only docs/feature-map/README.md was updated, and the checklist's docs box is unchecked).
  Clears when: learn-cron-dashboard.md covers project_path persistence and the new fire-time resolution/skip semantics.

Suggestions

• The missing-folder skip + agent-not-found skip + bindings resolution now exists twice in slack/gateway.py (sequence and single-agent paths, ~50 near-identical lines each); extracting one helper would keep the two fire paths from drifting on the next edit.

[DESIGN-REVIEWED] 7b37afb

[github-actions]

UX Review (Fable 5, fork) — 🟡 CONCERNS

UX-level review of 7b37afb197491cbb9cd105279d1ece44dea7390c via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence reviewed: the frontend hunks, backend error strings, sibling vocabulary, and the (non-materialized) screenshots. Emitting the review.

UX-Verdict: CONCERNS

One concept ships under three names — "Working directory" (form), "Operating folder" (fire-time errors), "project folder" (rest of the product) — and no first-time reader has seen the new field.

Watch

• Vocabulary drift at the failure moment: the field is labeled Working directory, but the fire-time errors it produces say "Operating folder no longer exists: {path}" / "not found in operating folder" (slack/gateway.py), and the product's established term is "project folder" (choose_a_project_folder, chat picker). Users hit the mismatch exactly when a job breaks, every time it breaks — moderate. Fix: one term across label, helper, and both last_error strings; product precedent favors "Project folder".
• Phantom claim + delayed discovery: the PR body says the job list carries project_path_missing, and a gateway comment references a "list-page badge" — neither exists in the diff. A vanished folder is invisible until the next fire, then surfaces only as a generic red "Error" badge whose explanation lives in a hover title tooltip (SchedulePage.tsx:933). Low frequency, medium impact. Fix: ship the claimed missing-folder indicator, or correct the description.

Evidence gaps

• "Working directory" field (label, helper text, path input, Browse button) — jobform-operating-folder.png is added by this fork PR and not materialized here; needs the blind read on a same-repo branch.
• Folder picker opened from the job form — jobform-projectpicker-open.png, same.
• Agent picker with a folder's project agents merged in (the PR's main payoff) — no screenshot exists at all.
• Inline roster-error state ("… Showing the global agent list instead.") and the three remapped save errors — no screenshot of any error state.

Suggestions

• Replace the hardcoded JSX placeholder "/Users/you/projects/myrepo" (macOS-shaped, not a catalog key) with a localized key like the existing path_to_folder pattern; the three error strings embed the same example.

[UX-REVIEWED] 7b37afb

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 7b37afb197491cbb9cd105279d1ece44dea7390c via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: CONCERNS

New resolve_project_path claims to be "the one place" the path-verdict core lives, but chat_folders._validate_project_dir still ships its own inline copy.

Not justified as shipped

• Item 6 — undeclared, symptom-level: the new stale-session reset keys on (project_path, agent) only; job.model still passes through get_or_create and a model edit still reuses the stale live session (1 uncovered argument at the same call site).
• Item 9 — duplicate of src/kiro_crew/dashboard/chat_folders.py:312: the docstring says "the one place the realpath → is_sensitive_path → isdir core lives, so the cron, chat-folder, and agents validators cannot drift apart", yet chat_folders' inline copy (lines 318–328) is untouched — grep _validate_project_dir: 2 real call modules, neither converted, so two spellings now coexist.
• Item 10 — rides along: FileChangeChipsAnim flake fix and black-baseline reformats are undeclared; harmless (screenshots follow the documented temp-screenshots/ convention).

What this change ships

Inventory (10 items) — 7 justified

Intent: let a scheduled job run one of a project folder's own agents instead of only global ones — an ADDITION (closes #8952), with one declared rider fix.

1. A cron job can bind an "Operating folder" and fires as that folder's agent, in that cwd — justified
2. Job form gains a Working directory field with Browse, merging that folder's agents into the picker — justified
3. A folder or agent gone by fire time skips the run with a recorded error, no auto-pause strike — justified
4. GET /api/agents accepts an owner-only project_path param for session-less callers — justified
5. An unrelated open chat tab's project no longer hijacks the session-less agents list — justified
6. Editing a job's folder/agent now takes effect on the next fire via live-session reset — undeclared, symptom-level: model edits stay stale
7. Rejected job saves now show the real backend message on edits too, with friendly path errors — justified
8. Shared folder picker now works inside modal dialogs (click, scroll, dismiss) for all its consumers — rides along, named live-confirmed harms
9. New shared security.resolve_project_path verdict helper — duplicate of chat_folders.py:312, sibling left unconverted
10. Committed PR screenshots, a chip-animation flake fix, black-baseline reformats — rides along

Watch

• Second spelling: replace _validate_project_dir's inline realpath/sensitive/isdir block (chat_folders.py:318–328) with a resolve_project_path call, or the two will drift exactly as the docstring promises they can't. Clears when: the inline copy delegates to the shared core.
• _cron_session_binding omits job.model; the mechanism built to make binding edits take effect leaves model edits reusing the stale session. Clears when: model joins the binding tuple, or the leftover is stated as deliberate.
• Description/diff mismatch: it claims tests pin "project_path/project_path_missing present (and correctly typed)" and a gateway comment references a "list-page badge" — grep project_path_missing in the diff: 0 hits; no badge ships. Clears when: the description drops the unshipped field/badge or the diff adds them.

Subtractions

• Shrink chat_folders._validate_project_dir to a resolve_project_path call plus its error-string shape — deletes the last inline copy of the check this PR centralized.

[FIRST-PRINCIPLES-REVIEWED] 7b37afb

[github-actions]

GPT 5.6 Review (fork) — 🔴 changes requested (blocking)

Reviewed 7b37afb197491cbb9cd105279d1ece44dea7390c via the fork AI-review pipeline; updated in place on each push.

2 of 3 blocking finding(s) are security-class and were withheld from adjudication, so the blocking verdict stands.

BLOCKING -- docs/feature-map/README.md:149 -- Schedule ownership omits the project-agent handler (origin: validation)
| Schedule | ... | handlers/cron.py | GET,POST /api/crons ... |
Project-agent schedule lookup -> GET /api/agents?project_path= -> handlers/agents.py, absent from the changed ownership map.
Anchor: feature-map-correctness
Fix: Add handlers/agents.py and GET /api/agents to the row.

BLOCKING -- src/kiro_crew/cron.py:1549 -- Normalize persisted project paths before use
project_path=j.get("project_path", ""),
Non-string persisted value -> _job_from_record accepts it -> cron listing redaction raises TypeError and returns HTTP 500.
Anchor: residual/crash-data-loss-corruption
Fix: Wrap the value with _str_or_empty().

BLOCKING -- website/src/components/JobForm.tsx:578 -- Clearing a folder retains its project-only agent
onChange={e => setProjectPath(e.target.value)}
Select project agent -> clear folder -> save stale agent without project_path -> global resolution substitutes the default agent with different permissions.
Anchor: residual/security
Fix: Clear the agent selection when it is absent from the global roster as the path changes.

FINDING -- website/src/components/JobForm.tsx:315 -- "kirocrewAgents(...).then" bypasses React Query, leaving global-only results after a transient rejection until the path changes -> Fix: use an enabled useQuery keyed by projectPath.

FINDING -- .github/black-baseline.txt:313 -- removing "src/kiro_crew/security/__init__.py" and other baseline entries changes CI enforcement outside the stated cron scope -> Fix: revert the unrelated baseline, ProjectPicker, and FileChangeChipsAnim hunks.

[BLOCK-MERGE] 7b37afb
[GPT-REVIEWED] 7b37afb

Adjudication (Opus 4.8) — is blocking on each finding proportionate?

F1 is anchored to the feature-map-correctness rule in AUTOSDE.yaml:12-13, which carries blocking: true. The rule requires every changed map row to name the feature's real backend ownership, and the PR's own diff (JobForm.tsx, handlers/agents.py project-roster lookup) is dashboard-structural. The rule's flag is authoritative and outranks weighing.

F2: _str_or_empty exists precisely for hand-edited non-string fields (cron.py:1460-1469), and the new project_path reads raw j.get(...) (diff line 143) instead. The codebase's own docstring at cron.py:1463 treats a hand-edited non-string as a real, expected input class — not an extreme/impossible condition — so no rarity argument for FLAG completes; UPHOLD-FENCED.

F3: The clear-folder path leaves agent state pointing at a now-absent project agent while body.project_path="" (diff lines 5327, 5404); at fire time global resolution silently substitutes the default agent. The triggering sequence (pick project agent, then clear folder) is an ordinary user action, not an extreme combination; UPHOLD-FENCED.

[ADJUDICATION] 7b37afb197491cbb9cd105279d1ece44dea7390c total=1 uphold=1 downgrade=0
UPHOLD F1 docs/feature-map/README.md:149 reason=autosde-blocking-rule
[GPT-ADJUDICATED] 7b37afb197491cbb9cd105279d1ece44dea7390c
[ADJUDICATION-FENCED] 7b37afb197491cbb9cd105279d1ece44dea7390c fenced=2 flagged=0
UPHOLD-FENCED F2 src/kiro_crew/cron.py:1549 -- Hand-edited non-string records are an input class the surrounding code (`_str_or_empty`) explicitly defends against, so the crash condition is ordinary, not extreme.
UPHOLD-FENCED F3 website/src/components/JobForm.tsx:578 -- Selecting a project agent then clearing the folder is an ordinary edit sequence that silently substitutes a different-permission agent at fire time; no rarity argument for clearance completes.
[GPT-ADJUDICATED-FENCED] 7b37afb197491cbb9cd105279d1ece44dea7390c

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 7b37afb197491cbb9cd105279d1ece44dea7390c via the fork AI-review pipeline; updated in place on each push.

Review details

Based on my analysis of the diff and base code:

Candidate 2 — The sequential missing-folder skip omits clear_active_session_key, but this is not a defect. In the single-agent path, register_active_session_key runs at line 4900 before the skip inside the try, so its skip must clear a registration made this fire (and _acquired=False means the finally at 5577 won't). In the sequential path, register_active_session_key runs inside the per-agent loop (line 4766), which is after the missing-folder skip — nothing was registered this fire, so there is nothing to clear. A stale key from a prior fire is not actionable: the job isn't running (so the reaper won't target it), and the next real fire overwrites the entry. No observable wrong outcome. Dropped.

Candidate 3 — Owner-gated (redact false / is_owner_dashboard_request) and offloaded to the discovery executor. The owner is a trusted boundary; unbounded query length is at most a trivial self-inflicted executor cost by an authenticated owner. No named boundary crossed. Dropped.

Candidate 1 — Confirmed: the new _cron_session_binding dict is written on every fire in both paths (self._cron_session_binding[session_key] = _binding_now and [agent_session_key] = _seq_binding_now) and is never popped anywhere in the full gateway.py diff or base — unlike the sibling _cron_injecting, which is explicitly .pop'd. Recurring jobs keep stable keys (bounded), but every one-shot/delete_after_run job fires under a fresh cron:{uuid} key that is retained for the process lifetime after the job and session are gone. Slow, genuine unbounded growth; advisory, not blocking.

No blocking issues; one advisory finding.

FINDING — src/kiro_crew/slack/gateway.py:1811 — self._cron_session_binding is written on every cron fire (_cron_session_binding[session_key] = _binding_now) but never popped, so each churned/one-shot cron:{job_id} key leaks a (cwd, agent) entry for the process lifetime → Fix: pop the entry alongside clear_active_session_key(job.id) in the fire cleanup (and on job deletion), mirroring the .pop already done for _cron_injecting.

[OPUS-REVIEWED] 7b37afb

[mbajaj92]

• BLOCKING — src/kiro_crew/cron.py:2168 — Synchronous filesystem probes block the event loop

Fixed. Added _validate_project_path_async (a worker-thread offload via asyncio.to_thread) and a _resolved_project_path escape-hatch parameter on _build_job. add_job_async and add_job_if_absent_async now pre-resolve project_path off-loop before calling _build_job, so the sync os.path.realpath/os.path.isdir calls never run on the event loop from an async caller. Sync callers (add_job, add_job_if_absent, the MCP/CLI update paths) are unaffected since they never run on the loop. Verified via the repos own check_sync_io_in_async.py gate, which now passes.

[mbajaj92]

• BLOCKING — src/kiro_crew/slack/gateway.py:4771 — Unresolved project agent silently runs as default

Fixed. Both fire-time paths now check resolve_agent_bindings(...).requested_resolved before dispatching. When False (the job named a specific agent inside project_path that could not be found there), the run is skipped with the same neutral shape as the existing missing-folder check: last_status="error", run_never_started=True, no record_failure() call. Added 3 regression tests in test_cron_gateway_integration.py::TestUnresolvedProjectAgentSkipsRun covering the single-agent skip, the sequential skip, and a resolved-agent control case.

[mbajaj92]

• BLOCKING — src/kiro_crew/dashboard/handlers/cron.py:627 — Non-string project path crashes PATCH

Fixed. project_path now routes through validate_string_field(body, "project_path", max_len=MAX_SHORT_STRING) wrapped in a try/except ValidationError, mirroring the create/POST paths existing pattern and the name/message PATCH fields on the same handler. Added test/test_cron_patch_project_path_validation.py (5 tests: valid path, oversize cap, non-string array, falsy non-string 0, empty-string clears binding) modeled on the existing test_cron_patch_name_validation.py.

[mbajaj92]

• FINDING — website/src/components/JobForm.tsx:277 — manual .then() bypasses use-react-query

Rebutted as disproportionate for this PR. useAgents() (the hooks own react-query wrapper) unconditionally calls syncKirocrewAgents() on every mount regardless of arguments, which would double-fire the sync/fetch for every JobForm mount even when no project path is set — the common case. Trying the useAgents() route while building this feature broke 11 existing JobForm.agentRoster tests via a React "Should not already be working" act() cascade. The local useEffect gated on a non-empty projectPath, calling the client fetch directly, is a deliberate choice made specifically to avoid that regression, not an oversight. Migrating useAgents() itself to skip the unconditional sync when unneeded is a real improvement but a separate, larger change than this PRs stated scope.

[mbajaj92]

• Term drift for the run folder — a third name ("Operating folder") for a concept already called "Working directory"/"Project directory"

Fixed. Renamed operating_folder* -> working_directory* across en.json, all 11 translated locale files, JobForm.tsx, AgentSelector.tsxs comment, and en-XA.json (regenerated), reusing the apps existing "Working directory" term rather than the coined one.

[mbajaj92]

• Redundant "Optional." — helper text repeats the (Optional) chip already on the label

Fixed. Dropped the leading "Optional." from the description string across all 12 catalogs; the field label already carries the (Optional) chip.

[mbajaj92]

• Two undocumented, out-of-scope hunks: temp-screenshots/*.png and package-lock.json libc-field removals

The screenshots are intentional (the PR templates own mandatory UI-screenshot requirement) and stay, committed under temp-screenshots/<feature>/ per that templates own convention. The package-lock.json libc-field diff was investigated and found to be a false read against a stale fetched origin/main mid-session — against the actual merge-base the file has zero diff. No change needed on this PR.

[mbajaj92]

• Sequential cron jobs never clear a restored project_path_missing

Fixed. The sequential paths missing-folder check now computes _path_missing unconditionally (not only inside the missing branch) and persists it whenever it differs from job.project_path_missing, mirroring the single-agent paths two-way update exactly. Added 2 regression tests (test_sequential_job_persists_the_missing_flag, test_sequential_job_clears_the_missing_flag_once_the_folder_returns) in test_cron_gateway_integration.py.

[mbajaj92]

• project_path_missing has zero frontend consumers

Rebutted as not dead. The flag still has a real backend purpose independent of any frontend render: it gates whether the fire-time path re-persists a state change (if _path_missing != job.project_path_missing), avoiding an unconditional write on every tick. The visible-to-users signal is the runs own last_status="error"/last_error, which the design deliberately consolidated onto (the separate list-page badge this flag once drove was removed per an earlier explicit product decision). Adding a frontend consumer for it is a legitimate future enhancement, not a defect in this PR.

• Duplicate validator: CronService._validate_project_path vs chat_folders._validate_project_dir

Rebutted as disproportionate for this PR. The duplication is real and worth consolidating, but doing so safely (extracting a shared helper both modules import, checking no caller relies on a subtle difference between the two) is a bigger refactor than this PRs stated scope. Flagging as a legitimate follow-up rather than fixing inline.

[mbajaj92]

• Subtraction: drop sourceLabel — only one folder is ever merged, so the existing source: "project" badge already identifies it

Fixed. Removed the sourceLabel field from AgentSelector.tsx (type + render) and the folderName computation + merge in JobForm.tsxs effectiveAgents. Verified: no test in the repo asserts on this specific field (the 18 sourceLabel hits in appstoreCategories.test.ts are an unrelated same-named function in the appstore module). 69 tests across the 4 affected files still pass.

[mbajaj92]

• Subtraction: replace _validate_project_path with a call to chat_folders._validate_project_dir

Rebutted, with the concrete check actually done rather than a blanket "bigger refactor": chat_folders.py imports dashboard.state.DashboardState, dashboard.token_auth, and sandbox.voice_runtime_workspace_conflict -- a dashboard-layer import chain. cron.py is used standalone by the CLI and MCP surfaces with no dashboard dependency today; importing chat_folders into it would pull that whole chain into a module that currently has none of it, which is a real architectural regression, not just extra lines. The genuinely safe version of this fix is extracting the shared 4-check sequence into the leaf kiro_crew.security module (which both files already import is_sensitive_path from) and having BOTH cron.py and chat_folders.py call it -- but that touches chat_folders.py, a file entirely outside this PRs diff, which is a scope decision I am not making unilaterally mid-review. Filing as a follow-up.

[mbajaj92]

• Subtraction: drop project_path_missing entirely — last_status/last_error already carry the signal

Rebutted, with the added cost now on the table: this field is not incidental residue -- both fire-time persist blocks that carry it are the exact code the Opus-flagged sequential-clear bug lives in, which this same round already fixed and covered with 2 new regression tests. Deleting the field now would mean re-touching that just-fixed code, deleting TestCronListFields list-serializer test coverage added deliberately last session, and losing the only mechanism that keeps the persist call from being unconditional on every tick (if _path_missing != job.project_path_missing). The "write-only from the frontend" observation is accurate and a legitimate case for adding a consumer later, but removing the field outright discards backend-side value the finding does not account for.

[mbajaj92]

• Frontend Tests (1) — FileChangeChipsAnim.test.tsx > restores focus when the row is reopened before collapse unmounts Pierre flaked

Investigated and fixed as a drive-by, since it was blocking this PR's CI even though it is not this PR's regression: confirmed the same test passed on the immediately-prior CI run against identical component code (only a two-line config-file comment/number change separated the two runs), and it passed 5/5 locally on repeated runs. Root cause is a genuine, if narrow, race in the test itself, not the component: completeOpenFocus() moves document.activeElement and clears the imperative tabindex attribute synchronously in one call, but role is a declarative React prop (role={focusProxy ? 'button' : undefined}) whose removal from the DOM waits on React's next render commit after the paired setFocusProxy(false) — a separate async step the test's waitFor(() => activeElement === chevron()) does not cover. Reliable on an idle local machine (the commit flushes fast enough), but exposed under CI's throttled scheduler. Fixed by adding a second waitFor for the role removal before asserting on it, mirroring the pattern already used elsewhere in this file. Verified the fix still catches a real regression: temporarily hardcoding role={'button'} in the component made the test fail with a real timeout, confirming it is not a vacuous wait. No change to FileChangeChips.tsx itself.

[mbajaj92]

• BLOCKING — website/src/components/JobForm.tsx:301 — Roster failures are mixed into validation state

Fixed. The project-scoped roster fetch's .catch() was writing into the same error state Save-time field validation uses, which is worse than a naming collision: setError also auto-scrolls the page to the bottom-of-form notice on every set, so a background fetch failing mid-typing would yank the user's scroll position for a failure unrelated to what they were doing, and the two failure kinds had no independent clear (a stale roster error could survive an otherwise-successful save; a real validation error could be silently clobbered by a late-resolving roster retry). Added a dedicated projectRosterError state, rendered beside the working-directory field via the same ErrorNotice component the sibling AgentSelector roster-failure UI already uses (variant="inline"), instead of borrowing the form-wide notice. Added test/JobForm.projectRosterError.test.tsx (3 tests): the roster failure renders beside the field and not in the form-wide alert, it survives an unrelated successful save (it describes the folder, not the save outcome), and it clears when the folder changes to one that resolves.

• FINDING — src/kiro_crew/cron.py:651 — stale comment contradicts the shipped skip behavior

Fixed. Second instance of the same class of stale comment already fixed once in gateway.py (this PR's earlier round) — this one in cron.py's own project_path_missing field doc was missed. Rewrote it to state the shipped fire-time SKIP behavior instead of the abandoned soft-fail-to-global-default design it was still describing.

• FINDING — src/kiro_crew/slack/gateway.py:789 — _project_path_still_canonical's docstring overstates what the check proves

Fixed. The finding is correct: re-resolving realpath and comparing against the stored string catches a symlink retargeted to point elsewhere, but does NOT catch the original directory being deleted and a plain (non-symlinked) directory recreated at the identical literal path — that case re-resolves to the same string with nothing left to distinguish "same inode" from "same path, different directory" once no symlink is involved. Rewrote the docstring to describe the symlink-retargeting case specifically and state the delete-and-recreate gap explicitly, rather than the general "proves nothing was swapped underneath it" claim. No behavior change — the check itself is unchanged; only the doc was wrong about its own coverage.

Re-affirming the earlier rebuttal on JobForm.tsx:277 (manual .then() vs use-react-query), unchanged since the last two rounds: useAgents() unconditionally calls syncKirocrewAgents() on every mount regardless of args, which would double-fire for every JobForm mount even when no project path is set (the common case), and broke 11 existing JobForm.agentRoster tests via a React act() cascade when tried during development. No code changed at this call site in this round.

[mbajaj92]

• Backend Tests (Windows) (4) — test_work_ledger.py::test_two_conductors_binding_one_worker_at_once_yield_exactly_one_binding failed

Not this PR's regression, not fixing inline. This file is entirely outside this PR's diff (touches work_ledger.py/test_work_ledger.py, unrelated to cron project-path binding). It's a concurrency race test (4 threads racing to bind one worker), and its own docstring/comments reference a previously-fixed class of Windows-only bug (issue #9248/#9250, PR #9237: a lock file opened in truncating "w" mode before the lock is held causes a bare OSError sharing violation on Windows, invisible on POSIX). Checked work_ledger.py's _open_lock — it already uses "r+" (the fix), so this occurrence is either a different failure mode or a transient one; either way it's a pre-existing test in an unrelated subsystem I have no standing basis to diagnose or fix inside this PR. This head's push was otherwise just a rebase onto upstream/main (conflict in comment-history-baseline.json only, resolved by recomputing the total) with no code changes of my own.

[mbajaj92]

• BLOCKING src/kiro_crew/slack/gateway.py:4906 — sequential cron path called ctx_builder.build_message(..., agent=agent) with the raw sequence-member name, while the session was acquired via _acquire_with_model_fallback(..., _resolved_seq_agent, ...) — the pre-resolved kiro_agent from resolve_agent_bindings. For a project-scoped job whose sequence member is a Kiro Crew alias whose kiro_agent differs from the alias name, build_message's own agent lookup (_load_agent_prompt, matched by config file name/stem) would build the system prompt for the wrong (or a nonexistent) agent while the actual kiro-cli session runs under the resolved name — exactly the same identity-mismatch class the single-agent cron path (line ~5146) already correctly avoids by passing agent=_resolved_agent_id.

Fixed: pass agent=_resolved_seq_agent instead of the raw agent, matching the single-agent path's existing pattern. Added test_cron_multi_agent_sequence_build_message_uses_resolved_agent in test/test_slack_gateway.py — confirmed red-before (asserted 'review-alias' was passed instead of the resolved 'claude-opus-reviewer') and green-after. No behavior change for the common case of no project_path or an alias that already matches its kiro_agent name.

[mbajaj92]

• FINDING (advisory) website/src/components/JobForm.tsx — the working-directory field (path input, browse button, and roster-error notice) rendered unconditionally in the vertical/detail layout, outside the {!isLlmless && ...} guard used for the Agent/Approval fields right below it. Verified genuine: project_path is read at fire time only by the two LLM-agent cron paths in gateway.py (single-agent and sequential) — a script/command job's subprocess dispatch in cron.py never derives a cwd from it. So for jobKind === 'script' | 'command', the field showed help text referencing "this job's agent" (a concept those job kinds don't have) for a value that does nothing at fire time, and — since save-time _validate_project_path_async runs unconditionally whenever project_path is non-empty — an invalid path there could 400 the save over a value the job would never use.

Fixed: merged the working-directory block into the same {!isLlmless && ...} guard already wrapping the Agent field, so it now hides consistently with the fields it functionally depends on. Extended test/JobForm.scriptCommand.test.tsx's existing "hides the Agent selector" test to also assert the working-directory field is absent for a script job, and added the inverse assertion (field present) to the existing message-job test. Confirmed red-before/green-after. Full frontend suite (1947 files / 30630 tests) passes; tsc --noEmit clean.

[mbajaj92]

• BLOCK whole-design first-principles review — project_path_missing was a persisted CronJob field + API-serialized value with zero readers: verified by grepping website/src (a type declaration only, never rendered) and src/kiro_crew (write-only — the two fire-time skip paths set it, the API list serializer emitted it, but nothing consumed it). Justified in the diff by "kept for at-a-glance visibility" / "the persisted flag (and its dashboard badge)," but no such badge exists anywhere in base or diff.

Fixed exactly as recommended: deleted project_path_missing entirely.

• CronJob.project_path_missing field removed (src/kiro_crew/cron.py)
• Both fire-time write-sites removed from src/kiro_crew/slack/gateway.py (single-agent and sequential paths) — _path_missing is now computed fresh from disk each fire and used only for the skip decision, never persisted; the skip behavior itself is unchanged
• Serializer/update/_job_from_record entries removed (src/kiro_crew/cron.py, src/kiro_crew/dashboard/handlers/cron.py)
• TS type entry removed (website/src/types/index.ts)
• 3 whole tests removed that exercised only the now-deleted persist-write behavior (test/test_cron_gateway_integration.py); assertions on the field removed from test/test_dashboard_cron_approval.py, test/test_cron.py, and 16 now-dead fixture lines from test/test_slack_gateway.py

All other items in the review's inventory (owner gate, precedence fix, roster fallback, error-surfacing fix) were already marked justified — untouched here.

Verified: 690 backend tests across the touched files + full mypy/isort/flake8 clean; 81 frontend JobForm tests + tsc --noEmit clean. Rebased onto upstream/main (2 conflicts: comment-history-baseline.json _total recomputed; website/src/components/JobForm.tsx merged with upstream's new "cheap cron"/minimalContext feature — both landed side-by-side, no logic overlap). One of my own pre-existing tests (JobForm.saveError.test.tsx, JobForm.projectRosterError.test.tsx) used a /save/i regex selector that started colliding with upstream's new "Minimal context" toggle (its description text contains "saved context") — tightened both to an exact 'Save' match; not a functional change.

[mbajaj92]

• Backend Lint & Type Check — comment-history gate failed (real regression, introduced by my own prior push on head 5b5502e50f95): a comment I added while fixing the FIRST-PRINCIPLES finding referenced "PR feat(cron): bind scheduled jobs to a project's agents #9201" by number, tripping the ratchet's bare-issue-reference pattern (review bookkeeping in a code comment, not the code's own rationale). Rewrote the comment to describe the code's actual behavior without the self-reference. Verified clean with python3 scripts/check_comment_history.py.

[mbajaj92]

• BLOCKING src/kiro_crew/dashboard/handlers/agents.py:2977 — Owner-gate denials were not audited: a non-owner passing project_path to GET /api/agents got no fallback and no error, silently — only the sensitive-path denial inside the owner-only branch was audited via sel().log_api_access. Fixed: added a symmetric outcome="denied", error="not owner" audit call on the non-owner branch, so both halves of this owner check now leave a trace.

• BLOCKING website/src/components/JobForm.tsx:567 — The roster-fetch error notice showed only the raw fetch error, without stating that the agent picker had silently fallen back to the global roster (effectiveAgents returns agents when projectAgents is empty, on failure exactly as on an empty folder). Fixed: appended a fixed explanatory suffix ("Showing the global agent list instead.") to the roster error message, added the new i18n key to en.json + all 11 real locales + regenerated the en-XA pseudolocale via gen-pseudolocale.mjs. Verified catalogParity.test.ts (77 tests) and the existing roster-error tests (substring assertions unaffected) both pass.

• FINDING src/kiro_crew/mcp_cron.py:2249 — project_path was read from cron_add/cron_update's MCP args but never declared in either tool's inputSchema, letting an LLM agent point its own scheduled jobs at arbitrary non-sensitive host folders through an undeclared parameter — scope creep beyond this PR's stated design (a human sets the operating folder via the Schedule page). Fixed exactly as recommended: removed all 4 project_path references from mcp_cron.py (the two handlers' read/kwarg-forward sites); the dashboard/API path (add_job/update_job_async, PATCH /api/crons) is untouched and remains the only way to set it. No existing test exercised the MCP-level path (confirmed via grep across test/test_mcp_cron*.py); 253 MCP cron tests still pass.

• FINDING src/kiro_crew/slack/gateway.py:4713 — _acquire_with_model_fallback threaded only the resolved alias's model (ResolvedBindings.model) into get_or_create, dropping ResolvedBindings.resolved_alias — the field crew_pinned_effort/resolve_session_effort/rebind_watchdog all resolve against (same crew_agent=crew_alias pattern already used in dashboard/chat_runner.py for a live chat slot). A project-bound global alias would run under the wrong crew-specific reasoning effort and watchdog settings. Fixed: added alias_crew_agent param threaded through _acquire_with_model_fallback (both its primary and model-fallback-retry get_or_create calls) in both the single-agent and sequential cron paths, populated from _bindings.resolved_alias/_seq_bindings.resolved_alias. Added 2 new regression tests (test_cron_multi_agent_sequence_build_message_uses_resolved_agent extended; test_cron_callback_single_agent_threads_resolved_alias_as_crew_agent new) asserting crew_agent is threaded correctly; both pass.

Full local gate re-run on this head: comment-history/isort/flake8/mypy (1382 files) clean; 944 backend tests (all touched files) + 158 frontend tests (JobForm + i18n catalog parity) + tsc --noEmit all pass.

[mbajaj92]

• BLOCKING (security-class, OPUS-adjudicated UPHOLD) src/kiro_crew/slack/gateway.py:4748 — A live persistent cron session (persistent_session defaults True) is reused as-is by SessionManager.get_or_create regardless of the cwd/agent arguments passed to it — confirmed in session_allocation.py:1126-1166. Editing a project-scoped job's project_path or agent binding between runs never reset the session anywhere, so a routine binding edit would silently keep the stale provider running under the OLD cwd and the OLD agent's permissions until an idle eviction or gateway restart. Fixed exactly as recommended ("reset the session whenever its effective project or agent binding changes"): added self._cron_session_binding: dict[str, tuple[str, str]], an in-memory (deliberately not persisted) map of each cron session key's last-fired (project_path, resolved_agent). Checked at fire time in both the single-agent and sequential cron paths — a mismatch against the prior fire's binding calls sessions.reset(key) before acquiring. Added test_cron_callback_single_agent_resets_stale_session_on_binding_change, which fires the same job 3 times (same binding twice, then a changed project_path) and asserts reset is NOT called on the first two fires and IS called exactly once on the third; passes.

• FINDING website/src/components/JobForm.tsx:293 (projectRosterError's manual useEffect fetch) — same finding already re-affirmed twice this session at its prior line number (277); unchanged code, same rebuttal: migrating to useQuery breaks 11 existing tests via an act() cascade. No action.

• FINDING src/kiro_crew/validation.py:2328 — real, and self-inflicted by my own prior-round fix: removing project_path from mcp_cron.py's handlers (the earlier GPT round on this same head lineage) left the field still declared valid in CRON_ADD_SCHEMA/CRON_UPDATE_SCHEMA, so an LLM agent's project_path argument to cron_add/cron_update would pass validation and then be silently dropped — worse than an outright rejection. Fixed: removed both FieldSpec("project_path", ...) entries. Added project_path to test/test_cron_string_field_validation.py's _NO_SCHEMA_FIELDS set (it is intentionally REST/dashboard-only now, validated separately via validate_string_field, unaffected by this change) with a comment explaining why. All MCP/validation/dashboard-PATCH tests re-verified passing.

Verified this round: black/isort/flake8/mypy(1382 files)/comment-history (re-baselined: this round's removals shrank gateway.py's marker count 64→62, written via --write-baseline) all clean; 1098 targeted backend tests + tsc --noEmit pass.

[github-actions]

AI-review override not recorded: only a repository writer may override an AI finding.

[mbajaj92]

Status: rebased onto current main; green pending CI, except two maintainer-only gates (head 20e72e1d8)

This PR is freshly rebased onto current main (was conflicting; now MERGEABLE) and CI is re-running. On the prior clean run, every CI / build / test / lint / coverage / whole-design-reviewer check passed — including all Backend Tests shards (an earlier shard-4 timeout was a stale test_workflows_app.py already fixed on main; the rebase pulled that fix in).

Two checks are expected to remain red, and both require a repository writer — a fork contributor cannot clear either (confirmed: label add and /ai-review override are both rejected/ignored from the fork account):

1. GPT 5.6 Review — BLOCK-MERGE. One blocking finding remains: "rebinding resets sessions with pending subagents" in gateway.py _cron_callback → _acquire_with_model_fallback. This is deliberately deferred, because that exact function is being rewritten by three in-flight PRs whose landing order was already agreed with the maintainers:

• fix(cron): treat launches from a pruned live install as retryable skips #9038 (pruned-install launch fix) rewrites _acquire_with_model_fallback to a ... | None + FileNotFoundError shape; agreed it lands first and this PR reintegrates on top.
• fix(cron): release ownership when session is deleted #9109 (cron ownership release) shares the cron.py locked add-path; agreed it lands first.
• fix(chat): bound the @-menu file-search fetch so a slow gateway fails visibly #7068 (ProjectPicker rewrite) shares the frontend picker; agreed it lands first.

Fixing the subagent-reset now would be rewritten by that reintegration, so it is scoped to the post-#9038 rebase. Every other GPT/Opus/Design/UX/First-Principles finding on this head is addressed or rebutted (the committed temp-screenshots/ are per temp-screenshots/README.md convention, pruned by the scheduled cleanup job — not to be attached as uploads).

Suggested override for a maintainer who agrees (re-pin the SHA if CI advances the head):

/ai-review override gpt 20e72e1d8: subagent-reset finding is deferred to the #9038 reintegration rebase (it rewrites the same _acquire_with_model_fallback path); all other findings addressed/rebutted per repo convention.

2. Fork workflow-change guard — fail. Fires because the diff edits .github/black-baseline.txt (a required black-baseline prune — the touched test files graduated to black-clean, so leaving them listed fails the black gate instead). It is a benign formatter-baseline edit, not a workflow change. Clear with the allow-fork-workflow-change label after review.

PR Readiness is red only as a consequence of the GPT block; it clears automatically once the override lands.

Blocked on: #9038, #9109, #7068 merging (for the reintegration), plus the two maintainer actions above. Happy to do the reintegration rebase the moment those land.