fix(cron): give a job's first run its tab — and its caller identity — at run start

[javenciu]

Problem / Motivation

A cron job's cron-<job_id> dashboard tab is created only when a run's result is injected, which happens after the turn has finished (inject_cron_result_to_dashboard was the only creator site). During a brand-new job's first run the tab therefore does not exist, and session-control caller identity resolves by walking live slot links (caller_slot_key in src/kiro_crew/dashboard/session_control.py matches the presented cron:<job_id> against each slot's identity), so every session-control verb the run calls refuses with caller_unidentified. From the second run onward the previous delivery's slot masks the whole problem.

Reproduced at base 0d65dc969 with a probe importing only existing symbols (attack + mask):

PROBE-A first-run caller_slot_key: ''  (defect when '')
PROBE-A first-run silent/dedup injection gate: False  (defect when False)
PROBE-B post-injection caller_slot_key: 'cron-job42'  (mask when 'cron-job42')
PROBE-B post-injection gate: True  (mask when True)
VERDICT: DEFECT-CONFIRMED

Why it matters

Since #8335 a cron is a legitimate session-control caller, and the first run is exactly the run a person watches after creating a new job — the refusal reads as an authorization problem, so the natural (wrong) next steps are checking agent.session_control and the agent's tools. Two more consumers share the dependency: every "does this session have a tab?" gate reads the dashboard-surface registry (sub-agent event routing, completion injection, widget/question/approval delivery), and the executor's silent/dedup delivery paths gate injection on has_slot(f"cron-{job.id}") (gateway.py), so a silent job's first result never reaches its tab either.

What changed (motivation → approach → change)

Symptom: first-run-only refusals and lost first-run routing. Root cause: the tab's only creator ran after the turn. Change: create it before the turn dispatches, for exactly the jobs that would get it at delivery anyway.

• ensure_cron_slot (new, cron_inject.py): pre-creates the tab at run start. Eligibility (job.persistent_session and not job.hide_in_chat) lives inside the helper, so ineligible jobs keep the deliberate fail-closed no-tab / no-identity / no-dispatch contract. Cheap from run 2 onward: an existing linked slot returns before any transcript I/O.
• The invariant the issue names moves with it: pre-creating the slot with its link would make the injection's if not slot.linked_session_key: hydration guard silently false. So _bind_cron_slot (extracted) is now the single shared core for BOTH creator paths — it links and hydrates (prefetch_cron_history, off-loop) in the same step, and the injection's unlink guard stays an idempotent no-op whichever path created the slot. Neither invariant half can drift because there is one writer.
• gateway.py: the executor callback calls ensure_cron_slot after the fire-time gate (a denied run dispatches nothing a tab could serve) and before dispatch.
• Decided explicitly, as the issue asks: a first run that starts and then fails now leaves an empty tab where previously none appeared. Gating the pre-create on the run reaching injection would recreate the very hole this fixes — the identity must exist during the run. The alternative (pre-create without the link) leaves identity unresolvable, which is the bug itself.
• Docs: learn-cron-dashboard.md (slot-creation timing) and session-control.md (minting site) updated in the same commit — both documented the injection as the only creator.

Tests

test/test_cron_first_run_tab.py (new, 8 tests):

• first run mints tab + link + dashboard-surface registry row before any result
• caller_slot_key resolves cron:<id> on the first run (with an unresolvable-before control)
• hydration moves with the link, and a later injection does not re-hydrate (no duplicated history) while still appending the result pair
• the executor's silent/dedup gate expression (mirrored verbatim) flips False→True at pre-create
• hide_in_chat=True mints nothing (contract control)
• persistent_session=False mints nothing (contract control)
• second call short-circuits without a transcript read (no repeated whole-transcript parse)
• empty history on a genuinely new job is benign

Fails-before: at base, the behavioral probe above convicts both the defect and the run-2 mask; the committed suite's collection fails at base on the new helper import. After: 8 passed. Neighbor suites (test_dashboard_cron_to_chat, test_cron_channel_delivery, test_session_control, test_cron_session_control, test_cron): 404 passed.

Manual verification

N/A — unit coverage sufficient: the mock state pins get_slot/has_slot as real functions over one dict (a bare MagicMock would auto-return truthy mocks and make every assertion unfalsifiable), and the silent-path test exercises the executor's own gate expression shape.

Screenshots / video

Why no screenshot: backend-only diff (two Python modules, one Python test, two docs) — zero frontend files, no rendered surface changes. The visible consequence (a tab appearing at run start instead of at delivery) is a timing change to an existing surface, decided and documented in the issue's own terms.

Related Issues

Fixes #8336

Pattern harvest

Rule candidate: review-prompt
Pattern: "identity/registry row minted only by a delivery path that runs after the turn — any consumer that needs it DURING execution fails on the first run, and the previous run's leftover masks the defect from run 2 onward; check every get_or_create_* that only runs post-turn."

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

Contribution License Agreement

Per the template placeholder (CLA text pending): offered under the same terms as my prior merged contributions to this repository (#8835).

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

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

The claims check out end-to-end: caller_slot_key (session_control.py:383) walks live slot links so a missing slot yields ""; base inject_cron_result_to_dashboard was indeed the only cron-{id} creator; the insertion point sits after the message-path fire-time gate (gateway.py:4342) and before dispatch; and the run_never_started arm/clear contract in the new wrapper exactly mirrors the proven _await_cron_fire_time_gate pattern, matching cron.py's delete-site expression (cron.py:4059) and _execute's pre-callback clear (cron.py:3939). The TOCTOU across the prefetch await resolves benignly because _bind_cron_slot re-checks the link. Eligibility gating inside the helper preserves the documented hide_in_chat/ephemeral fail-closed contract, script/command jobs never reach the site, alternatives were weighed explicitly, and both owning specs are updated in the same commit.

Design-Verdict: PASS

Root-cause fix at the right seam: one shared bind core for both creator paths keeps the link/hydration invariant single-writer, with failure semantics matching the proven fire-time-gate contract.

[DESIGN-REVIEWED] 847defd

[github-actions]

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

Premise-level review of 847defd6c64ae1412fc90d8c8bf0fe9a2fb6bbcd 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.

All claims verified against the base. One finding: the new docstring's "only writer" claim is contradicted by a second hand-rolled cron-slot bind in handlers/cron.py:1411-1414, and the guard wrapper plus four of the twelve tests are absent from the description. Composing the review now.

First-Principles-Verdict: CONCERNS

Sound cause-level fix, but the "only writer of linked_session_key for a cron slot" claim ships false — handlers/cron.py:1413 is a second, hand-rolled writer.

What this change ships

Intent: let a brand-new cron job's first run be recognized by the dashboard (identity + routing) while it runs — a FIX.

1. First run can call session-control verbs; caller_unidentified refusal gone — justified, cause-level (verified: caller_slot_key walks live slots, session_control.py:383).
2. The job's chat tab appears at run start instead of at first delivery — justified, declared timing change.
3. A first run that fails midway now leaves an empty tab — declared trade-off, decided in the diff.
4. A silent job's first result now reaches its tab — justified (3 has_slot(f"cron-{job.id}") gates counted: gateway.py:4753, 4778, 4834).
5. Sub-agent/widget/question/approval routing works during a first run — justified, same registry row.
6. A pre-create failure no longer kills the run (logged, run proceeds tab-less) — undeclared, justified.
7. Cancellation during pre-create preserves a delete_after_run one-shot — undeclared, derived (cron.py:3939 clears the marker pre-callback; delete site cron.py:4059 reads it; mirrors the fire-time gate, gateway.py:902–948).
8. One shared bind core for both creator paths (_bind_cron_slot, 2 consumers) — justified, but see Watch.
9. Both specs updated same-commit — mandated by AGENTS.md.

Watch

• The new docstring states "the only writer of linked_session_key for a cron slot is _bind_cron_slot" — grep linked_session_key = counts 2 cron-slot writers: the deleted-job branch of api_cron_to_chat (handlers/cron.py:1411–1414) hand-rolls the same get_or_create + link + hydrate. That branch has no CronJob, so consolidation is genuinely out of shape — but the docstring's absolute claim is false as shipped; weaken it to "the only run-path writer."
• Description says "test/test_cron_first_run_tab.py (new, 8 tests)" and never mentions _pre_create_cron_slot; the file ships 12 tests including the 4-test TestPreCreateGuard. Items 6–7 are justified but arrived after the description was written — declare them.

[FIRST-PRINCIPLES-REVIEWED] 847defd

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] 847defd

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

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

Review details

FINDING -- src/kiro_crew/slack/gateway.py:975 -- Silent/deduplicated runs require has_slot(...), so after pre-create failure "delivery's own bind still creates the tab afterwards" is false -> Fix: qualify this fallback as normal delivery only.
[GPT-REVIEWED] 847defd

[javenciu]

Prior-art note for reviewers: #8441, an earlier attempt at #8336, was self-closed with the rationale that session-control policy refuses cron callers on every run — which would make a first-run tab pointless. That rationale does not hold at current main: src/kiro_crew/dashboard/session_control.py admits cron callers via the _cron_caller(caller_key) carve-out (defined at :137 on the cron- slot-key prefix, used in the refusal expressions at :193/:243), so a cron run whose slot exists CAN use session-control verbs during the run. Pre-creating the tab at run start is what makes that identity exist for run 1; the tests in the first commit pin the end-to-end path.

[bolichen97]

Tech Lead review — approved.

Caller identity at run start is correct and not spoofable. Both the slot name (cron-{job.id}) and the identity it carries (linked_session_key = "cron:{job.id}") are computed inside the helper from the host-held CronJob.id; no job-controlled string reaches either. The only job-supplied value that reaches the slot is job.name → slot.title, display-only and redacted in the same order as before (_safe_job_name preserves URL-pass-then-credential-pass). And caller_slot_key resolves by matching the MCP process's authenticated session key against live slot links rather than accepting a key as an argument, so a job cannot present another job's cron:<id>.

No race can misassign it. _bind_cron_slot is fully synchronous — get_or_create → title → link+hydrate → _sync_dashboard_slots, no await between them — so on the single event loop it is atomic against every other coroutine. The one await in ensure_cron_slot is prefetch_cron_history, before the bind; a second caller that slipped past the same get_slot check re-enters _bind_cron_slot, finds linked_session_key already set, and no-ops. Worst case is a redundant transcript read, never a duplicated hydration or a wrong identity. Placement after the fire-time gate and before dispatch leaves no window where a dispatched run lacks the slot delivery would later have created, and the run_never_started arm/clear correctly mirrors _await_cron_fire_time_gate: CancelledError is a BaseException, escapes the except Exception, and leaves the marker armed so a cancelled pre-create retains a delete_after_run one-shot. For an ineligible job ensure_cron_slot returns without awaiting, so the marker is never observable.

Scope is tight and correct for a fix: — 5 files, backend + two owning specs, no frontend, no workflow files, one new test module.

Two advisory findings verified as factually correct, neither behavioral:

1. First Principles is right that the new inject_cron_result_to_dashboard docstring's "the only writer of linked_session_key for a cron slot is _bind_cron_slot" ships false — handlers/cron.py:1413 (the deleted-job branch of api_cron_to_chat) is a second hand-rolled writer. That branch has no CronJob so consolidation is genuinely out of shape; the claim should read "the only run-path writer."
2. GPT is right that _pre_create_cron_slot's "delivery's own bind still creates the tab afterwards" is false for the silent/dedup paths, which gate on has_slot(f"cron-{job.id}") at gateway.py:4752/4777/4833 — for a silent job a failed pre-create means the first result never reaches a tab. That is the pre-existing behavior, so no regression, but the comment states the opposite of the case it exists to justify.

Both are comment text, not code, and both lanes rated them non-blocking. I am not holding a verified root-cause fix on two docstring sentences — and I cannot amend them here without becoming last-pusher and voiding this approval. Filing them as a follow-up instead.

Merging.