fix: guard slot-less set_project; repoint stale boundary tests (#5201)

[bolichen97]

Closes #5201

What broke

main is RED at 410972a: every open PR's merge-ref Backend Tests shard fails on two tests in test/test_driver_session_directives.py::TestChannelApplierBoundary, regardless of the PR's own diff.

PR #3543 (37089b1) deliberately moved set_project out of _DASHBOARD_ONLY_DIRECTIVES (any user-facing surface may retarget its own slot; the gate became the positive _has_user_surface predicate) and updated test/test_mcp_core_set_project.py accordingly — but never touched test_driver_session_directives.py, which still pinned the pre-#3543 invariant with set_project parametrized in the dashboard-only set.

The failures also exposed a real defect, not just a stale expectation: the channel TurnDriver consumer always passes slot=None (messaging/dispatch.py:208). Post-#3543 such a caller passes the user-surface gate (its key IS a channel key) and crashes at slot.project = rp with AttributeError: 'NoneType' object has no attribute 'project', swallowed by the generic handler into an opaque Error applying set_project: ... audited as error. The clear path (slot.project = "") had the same hole.

The fix (minimal, main-red unblock scope)

1. Slot-less guard at the top of _set_project, before ANY mutation (the clear path writes slot state too — fail closed): an explicit refusal naming the condition (... this slot-less channel turn holds no slot. Nothing was changed.).
2. Test repoint: set_project removed from the two dashboard-only boundary tests (suggest_followup/ask_question stay pinned unchanged — that messaging TurnDriver never consumes session-directive markers (set_project/monitor trio silently no-op on standalone transports) #4540 invariant is not weakened); docstrings now record that feat: allow set_project on non-dashboard surfaces (Telegram, Slack) #3543 removed set_project deliberately, so it is not "restored" later.
3. New tests pinning post-feat: allow set_project on non-dashboard surfaces (Telegram, Slack) #3543 semantics: channel key + slot → project applied, realpath'd, history-reset flag set, one push; slot=None (set-path AND clear, no-tab AND open-tab) → the clean refusal string, audited denied, zero pushes, no NoneType leak.

Why raise _DirectiveDenied (audited denied) and not a plain Error: return (audited error)

The task allowed either and asked for the reasoning. The module's own audit contract decides it:

• _DirectiveDenied's docstring names "an unsupported session type" as its case. A slot-less caller is a session/transport-shape condition (TurnDriver holds no slot), not an argument-validation failure like not a directory (which stays a plain return).
• The monitor trio's analogous session-shape refusal ("not supported from this session type") raises _DirectiveDenied, and test_not_supported_paths_audit_denied_never_success pins outcome=denied for exactly that class.
• SEL continuity: before feat: allow set_project on non-dashboard surfaces (Telegram, Slack) #3543 this exact caller shape (slot-less channel calling set_project) was refused by the dashboard-only gate and audited denied. Keeping denied means the same refused attempt does not silently reclassify to error across feat: allow set_project on non-dashboard surfaces (Telegram, Slack) #3543.

Not done here (scope discipline)

• No refactor of the directive dispatch, no unification of the dashboard-only set.
• test/test_mcp_core_set_project.py untouched — feat: allow set_project on non-dashboard surfaces (Telegram, Slack) #3543 updated it deliberately; it remains the authority on the new intent and passes as-is.
• Conflict awareness only (read-only): open PR feat: expose session monitors to agents #5184 also edits session_directive_apply.py, PR refactor: extract shared session-directive forgery-gate predicate (#5122) #5167 edits session_directive.py. This guard is a small localized hunk; landing this first unblocks their merge-ref CI too.

Verified

• Fail-before baseline: both spec-named tests reproduced failing on clean main (AttributeError at slot.project = rp in the traceback).
• After: test_driver_session_directives.py + test_mcp_core_set_project.py + test_session_directive.py = 100 passed.
• Full backend suite: 61777 passed (includes the repo-wide contract/ratchet suites); isort/flake8/mypy clean; changed test file black-clean (the src file is baseline-exempt, pre-existing).
• Pre-push dual model-pinned review: GPT 5.6 Sol PASS (zero findings); Opus 5 PASS + 1 Medium + 4 Low advisories, all five adopted (module-docstring third slot-less class, stale "These three" comment from feat: allow set_project on non-dashboard surfaces (Telegram, Slack) #3543, open-tab × slot-less parametrize, _pending_reset_history_key assertion, removal of needless is_sensitive_path mocks).

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] a58af62

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

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Minimal fail-closed guard at the layer that owns the slot, with audit continuity (denied) preserved across #3543 — the right shape for a main-red unblock.

[DESIGN-REVIEWED] a58af62

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of a58af6267205b7e39946cb12cae8466f23fd3cba — 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 repo: the guard sits in _set_project (session_directive_apply.py:401), the TurnDriver consumer does pass slot=None (messaging/dispatch.py:208), no other applier dereferences slot unguarded (the monitor trio and autonudge_stop use getattr, the dashboard-only pair are gate-refused), and test_mcp_core_set_project.py already pins channel-key+slot application. Final review follows.

First-Principles-Verdict: PASS

Guards the exact slot-dereference #3543 exposed, at the mechanism, with the pre-#3543 denied audit preserved — every item traces to the red-main defect.

What this change ships

Intent: unbreak red main by fixing the crash #3543 left for slot-less channel callers of set_project and repointing the two tests still pinning the pre-#3543 boundary — a FIX.

1. Slot-less channel set_project now gets a clear refusal instead of an opaque swallowed AttributeError — justified (mechanism-level; the gate that used to refuse this caller stopped covering it in feat: allow set_project on non-dashboard surfaces (Telegram, Slack) #3543).
2. That refusal audits denied, the outcome this caller shape produced pre-feat: allow set_project on non-dashboard surfaces (Telegram, Slack) #3543 — justified, declared.
3. Two boundary tests stop expecting set_project to be dashboard-only — justified (the fix's test half).
4. Open-tab boundary test now parametrizes both remaining dashboard-only kinds — justified rider (removing set_project would leave it empty).
5. New test pins channel-with-slot application — overlaps test_mcp_core_set_project.py:455; only the reset-flag/push assertions are new.
6. New test pins the slot-less refusal (set/clear × tab states, audited denied, zero pushes) — justified.
7. Docstrings/comments updated to record the post-feat: allow set_project on non-dashboard surfaces (Telegram, Slack) #3543 boundary — justified (same-commit spec rule).

No unfixed siblings: _USER_SURFACE_DIRECTIVES has one member, and every other applier reads slot via fail-safe getattr or behind the wrapper gate (counted in session_directive_apply.py).

Subtractions

• test_set_project_applies_on_channel_transport_with_slot re-pins what test_set_project_works_on_channel_sessions (test_mcp_core_set_project.py:455, 3 channel keys) already asserts; fold its two genuinely new assertions (_pending_reset_history_key, pushes == 1) into that existing test and drop the duplicate.

[FIRST-PRINCIPLES-REVIEWED] a58af62

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

The discovery pass found no candidates. I've independently verified the changed code: the slot is None guard in _set_project raises _DirectiveDenied before any mutation, which is caught at line 175 and audited exactly once as denied, returning the readable error string. The clear path (which writes slot state) is correctly below the guard, so no mutation occurs on the slot-less path. The user-surface gate at line 146 lets a channel key through (correct — it IS user-facing), and the new guard then handles the slot-less case. No regression, no removed guard, no crash path, and the security invariants (deny-by-default, sensitive-path checks, off-loop FS) remain intact.

No findings.

[OPUS-REVIEWED] a58af62

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

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

[bolichen97]

🤖 Kiro Crew Auto-Pipeline [operator: bolichen97#bb3ad1ca]

Superseded by #5212 (merged 63705e6 while this PR's first CI run was in flight), which fixes the identical defect for the parallel issue #5208 with equivalent semantics: slot-less refusal before any mutation, SEL outcome denied, the same two test repoints, plus its own SEL-truthfulness pin. Verified on main tip: TestChannelApplierBoundary 10 passed — main is green again, so this PR's purpose (the main-red unblock) is already served.

The small deltas this branch carried beyond #5212 (three stale-comment fixes and two extra test pins, all validated by a dual-model review) are filed as #5222 with this branch referenced as a ready-made implementation. Closing rather than rebasing: re-landing a duplicate guard would add a second refusal site for the same condition.