fix(cron): give script crons a session identity for state-mutating MCP calls

[bolichen97]

Problem / Motivation

A script cron that calls a state-mutating MCP tool (ctx.call_tool("kirocrew-cron", "cron_trigger", ...), learn_add, artifact_save, ...) does not raise — the call reaches the handler and comes back with the fail-closed refusal as a plain string:

Error: cannot determine which session is calling, so this write is refused. ... No identity channel: the signed pid mapping for this session did not verify.

Read-only calls work, so the job looks healthy: a script that swallows the string reports ok on every run while writing nothing.

Why it matters

This is the second half of #6431. That fix (boot_platform in the launcher preamble) made script-cron MCP calls reach the server again; this one makes the server trust them. Without it, cron→cron orchestration from a script cron (a selector waking an investigator via cron_trigger), and every other script-driven write, silently no-ops on every install — Linux included, where the refusal names a "signed pid mapping" the operator has no way to produce.

What changed (motivation → approach → change)

mcp_core._resolve_session_key_strict — the gate behind every mutating tool — accepts exactly three sources: the gateway-injected caller block, KIROCREW_SESSION_KEY, or KIROCREW_HOST_PID plus its HMAC sidecar. A script cron had none of them:

• nothing routes its direct MCP spawns through gatewayd (no caller block);
• run_script_sandboxed never set KIROCREW_SESSION_KEY;
• nobody publishes session_pid_<pid>.sig for its launcher pid, so on Linux the sandbox launcher's KIROCREW_HOST_PID pointed at an unsigned pid (hence the "did not verify" wording), and on macOS/Windows no HOST_PID is exported at all.

Approach: use the channel the gateway already gives every agent subprocess. acp/client.py injects KIROCREW_SESSION_KEY=<session> into agent processes — agent crons included, where the key is cron:<job id>. Script crons now get the same:

• run_script_sandboxed sets KIROCREW_SESSION_KEY=cron:<job id> in the child env. This is the key ScriptContext already presents to the gateway over HTTP (X-Session-Key / caller_session), so ownership and audit see one principal per job regardless of surface.
• McpToolClient takes an optional session_key and hard-assigns it on the server spawn env AFTER the inherited-env and spec-env overlays. ScriptContext.call_tool passes cron:<job id>. Same reason ScriptContext.notify hard-assigns caller_session: the bridge's env is the script child's own os.environ, which user code can rewrite before calling ctx.call_tool. The CLI preview path constructs the bridge bare and is unchanged.

Why env rather than publishing a signed sidecar for proc.pid: the sidecar channel exists for the kiro backend's session-unbound AcpRuntime, where one process multiplexes N sessions and an env var is wrong by construction. A script cron is one process per job, so the env var is the correct channel — it is what agent crons use, it works on macOS/Windows (no launcher exports HOST_PID there), and it leaves no per-run session_pid_* files behind. Provenance is identical to the ACP path: gateway-authored env on a direct child.

Side effect, intended: cloud.aws.assert_human_action keys on the same var, so destructive cloud verbs from a script cron are now refused like they are from an agent session.

Out of scope, deliberately: _check_cron_job_ownership still requires the caller's key to equal the target job's owner. With this change a script cron is identified as cron:<its own id>, which is not the owner of any job it did not create, so cron_trigger on a different job is still refused (as it is for agent crons). Whether cross-job orchestration should get a scoped exemption is a design question, not an identity bug; happy to open an issue if maintainers want it tracked.

Specs

docs/system-specs/modules/learn-cron-dashboard.md is the durable home for the identity contract: script crons run as cron:<job id>, the two hops that deliver it, why the omission was silent (strict resolution fails closed for writes only), the two consequences named above, and the general rule this is one instance of — a spawn path that hands a child MCP access must hand it an identity.

The launcher is a second injector of KIROCREW_SESSION_KEY, so docs/system-specs/modules/computer-use.md and the mcp_computer shim stop calling the ACP spawn path the only one. The claim those passages actually rest on is unchanged and now stated directly: both accepted env sources come from a launcher above the process, and a GUI-launched kiro-cli has none, which is why the shim does not gate on identity. That fact was restated in five places this diff had to keep in sync, so it is now stated once in the shim's module docstring and pointed at from the two comments and the test docstring that repeated it — the next injector syncs one copy. While there: the same docstring described the audit trail as recording an empty key, which the UNRESOLVED_SESSION_PREFIX constant thirty lines below it exists to prevent, so it now names the placeholder the code actually forwards.

Deliberately not in this PR, both raised as follow-ups rather than fixed here: ScriptContext.call_tool still audits "ok" on a refusal that arrives as an ordinary result string, so a future identity or ownership gap would be as quiet as this one was; and the cross-job cron_trigger ownership question below.

Tests

test/test_cron_script_identity.py (new, 11 tests, mutation-verified: 10/11 fail with the production change reverted):

• launcher env carries KIROCREW_SESSION_KEY=cron:<job>; equals the X-Session-Key ScriptContext._post sends; a forged inherited key is overwritten, not kept
• bridge pins the key over a script-rewritten os.environ and over a spec-env block; passes nothing when given nothing (CLI preview); call_tool passes the job's key
• the real consumers accept it: mcp_core._resolve_session_key_strict() and mcp_cron._authz_session_key() evaluated in the child env return cron:<job>, and the same env minus the key reproduces the refusal ("")

test/test_cron_script_more_coverage.py: three McpToolClient stubs accept the new kwarg.

Full cron suites green locally (419 passed, 1 skipped); flake8 clean.

Manual verification

N/A — the consumer-side tests evaluate the actual strict resolver against the actual child env; the refused string in the report is exactly _resolve_session_key_strict() == "".

Related Issues

Follow-up to #6431 (same launcher, second gate).

Pattern harvest

Rule candidate: review-prompt
Pattern: "gateway-spawned child that calls session-keyed MCP tools without the gateway setting KIROCREW_SESSION_KEY (or publishing its pid sidecar)" — any new spawn path that hands a child MCP access must also hand it an identity, or the strict resolver fails closed only for writes and the failure is silent.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated — the cron spec gains the script-cron identity contract (see Specs above); the computer-use spec and shim no longer call the ACP spawn path the only injector of KIROCREW_SESSION_KEY
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 1dd61952895460fa0cb8140352d0e1481b99fa60 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design review verified: the identity gap is real (script crons had none of the three sources _resolve_session_key_strict accepts), the fix reuses the existing gateway-authored env channel rather than inventing a new one, ownership semantics are preserved, the spec is updated in the same commit, and the mcp_computer.py doc changes are accounted-for consistency updates (the docstring's "only injector" claim became false). Known gaps (refusal-audited-as-"ok", cross-job trigger ownership) are disclosed and deliberately deferred.

Design-Verdict: PASS

Root-cause fix through the channel agent crons already use; scope, side effects, and deferred gaps are all named and defensible.

[DESIGN-REVIEWED] 1dd6195

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 1dd61952895460fa0cb8140352d0e1481b99fa60 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 1dd6195

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 1dd61952895460fa0cb8140352d0e1481b99fa60: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 1dd61952895460fa0cb8140352d0e1481b99fa60 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 1dd6195

Verdict parsed from the review's SHA-scoped output markers for commit 1dd61952895460fa0cb8140352d0e1481b99fa60.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 1dd61952895460fa0cb8140352d0e1481b99fa60: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 1dd61952895460fa0cb8140352d0e1481b99fa60 — 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 repository. Writing the review now.

First-Principles-Verdict: PASS

A cause-level fix: the one identity channel every launcher already uses now reaches script crons, and every rider is a declared comment/spec sync.

What this change ships

Intent: make a script cron's state-mutating MCP calls actually write instead of silently no-oping — a FIX.

1. Script-cron MCP writes now succeed instead of silently returning a refusal string — justified, cause-level (verified: _resolve_session_key_strict at mcp_core.py:749 accepts exactly the three sources the description names, and a script cron had none)
2. Script child env carries KIROCREW_SESSION_KEY=cron:<job id> — justified; same channel acp/client.py:3647 and members.py:184 already use, so no second spelling
3. MCP server spawn pinned to the job's key over a script-rewritten env — justified by the agent-untrusted boundary, same footing as notify()'s existing hard-assign
4. Destructive cloud verbs from a script cron now refused (cloud/aws.py:61 keys on the same var) — declared side effect, inseparable from the single-principal design
5. McpToolClient gains optional session_key — 2 construction sites counted (cron_script.py:340 passes it, cli_commands.py:1316 bare); not one-consumer generalization
6. Cron and computer-use specs updated — mandated by the AGENTS.md same-commit spec-sync invariant (the fix falsified "injected only by the ACP spawn path")
7. mcp_computer docstring "empty key" → placeholder — rides along, declared; corrects a claim the code (UNRESOLVED_SESSION_PREFIX, mcp_computer.py:183) contradicts; comment-only, zero surface

Watch

The silence that hid this bug — refusals arriving as ordinary result strings that _audit_tool_call records as "ok" (cron_script.py:342) — remains, with one sibling sharing it (cli_commands.py:1317, counted via the two McpToolClient( sites). The description declares it as a deferred follow-up; hold the author to filing it.

[FIRST-PRINCIPLES-REVIEWED] 1dd6195

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #7670 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7670: KEEP. Neighbouring edits in one function with no behavioural interaction. Files: src/kiro_crew/cron_script.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.