fix(conductor): one plan gate, required goal folder, rate-bounded dashboard trust

[iamwhatever]

Problem / Motivation

Three defects observed on the first real run of the kirocrew-conductor agent
against a live goal:

1. It interrogated the user before doing any work. The goal-conductor skill's
   Round 0 said "restate the goal, list the work items, wait for the user", which
   left room for a clarification round ahead of the plan. The run spent its
   opening turns asking questions rather than proposing a plan.
2. It created sessions with no folder. chat_folder_create was step 1 inside
   the per-item dispatch loop, worded "once per goal (skip if it exists)". The run
   skipped it entirely and every dispatched session landed loose at the sidebar's
   top level.
3. Every session-control call stopped for an approval. @kirocrew-dashboard
   was withheld from the conductor agent's allowedTools, so session_create,
   session_send, chat_folder_* and the rest each prompted.

Why it matters

Each one lands on the conductor's core loop rather than an edge:

1. The conductor exists to take a goal off the user's hands. A round of questions
   before the plan inverts that, and every question about something the conductor
   could have assumed is latency on work it could already have started.
2. Sessions with no folder have nothing tying them to the goal they belong to.
   With two or three items per round the user cannot tell which sidebar entries
   are this goal's, and there is no grouping to close the goal out by.
3. The patrol loop is monitor_start-driven: it wakes itself on a nudge
   interval. A cycle that blocks on an approval blocks on a person who is not at
   the keyboard, so the loop stalls rather than patrols — the skill's own "Known
   limits" section had to describe patrol as "attended-unattended" because of it.

What changed (motivation → approach → change)

1. Round 0 is now one plan message with one gate. The wording that permitted a
question round is replaced by an explicit rule: the first reply is the plan, and
anything the conductor can settle itself becomes a stated Assumption the user
can overrule in the same breath (which repo, items per round, which crew, how to
phrase an acceptance condition). A question is permitted only when a wrong guess is
unrecoverable and no default exists — credentials, spend, destroying someone's
work, or a goal too vague to name a single work item — capped at one and folded
into the plan message. A goal message that already authorizes execution ("just do
it", "go ahead", a re-send of a plan already shown) dispatches round 1 in that same
turn instead of re-asking for permission the conductor already holds.

2. The goal folder is a round-level precondition, not a per-item step.
session_create has no folder argument (SESSION_CREATE_SCHEMA at
src/kiro_crew/validation.py:2628 carries only title and agent), so filing is
a second call — which is exactly why an optional-sounding instruction inside the
loop got dropped. That second call is an earlier design choice, not a
constraint: adding a folder field would make filing atomic with creation and
delete this whole choreography. Doing that is wider than this fix, so it is
deferred to #6118 and the instruction layer is hardened in the meantime. It is hoisted above
the loop as a precondition ahead of the first session_create for the goal,
session_create-before-folder is named as a defect rather than discouraged, and a
failed chat_folder_create stops the round instead of dispatching into no folder.
The local GPT review lane then caught the sibling gap: creating the folder is not
sufficient, because the user can delete it mid-round, so a failed
chat_folder_move_session now also stops the item before the seed — seeding
anyway produces a session that is both unfiled and already doing work, strictly
worse than the outcome the precondition prevents.

3. Dashboard verbs are auto-approved BY NAME, under one invariant. The
dashboard server joins the same _may_auto_approve ceiling filter as session,
report and @kirocrew-core, so a governed host still withholds it (with the
existing mcp_auto_approve_withheld SEL audit) and the derived KAS permissions
block picks the change up automatically rather than restating it.

But the grant names verbs, not the server, because the first attempt at this
granted @kirocrew-dashboard whole and that was wrong. allowedTools is the one
path that never reaches hooks.on_tool_call, and the conductor ingests untrusted
text by design — its charter's worked example is "resolve this repo's open issues"
and it holds web_fetch for exactly that. So every auto-approved verb is
reachable by content the conductor read, with no human in the loop on a
nudge-driven cycle. The rule that decides each verb, recorded on
_CONDUCTOR_DASHBOARD_GRANTS so the next reader extends it by rule:

A granted verb must satisfy both halves:

1. It may create something new or read. It may never mutate
   user-visible workspace state that already exists and is not the conductor's
   own — a session's contents or liveness, or the arrangement the person made of
   their sessions and folders.
2. Its worst case, called in a loop, must be bounded by the server — and
   bounded so the resource stays reachable by everyone else.

Half 2 is the one the review process forced, and it is not a restatement of half 1:
an unbounded create does damage without mutating anything. Per-call approval was
the only thing rate-limiting a granted verb, and allowedTools has no argument or
rate matching to replace it, so waiving the prompt removes the only brake on a loop.
The brake has to live in the endpoint, and it comes in two layers.

The primary guard is a per-caller RATE on both creation verbs
(dashboard/create_rate_limit.py): 20 sessions and 10 folders per caller per
5-minute sliding window, refused with a 429. It is deliberately the guard that needs
no durable state. A lifetime quota means nothing across a restart unless every
path that rehydrates a session carries its attribution, and there are 27
get_or_create_slot call sites across 12 modules; a five-minute window is not
meaningfully resettable — a restart buys a caller one window, not a clean slate. At
the session rate, filling MAX_LIVE_SLOTS from empty takes over two hours of
uninterrupted looping, every step visible in the sidebar, which turns a silent
exhaustion into something a person watching the dashboard sees before it lands.

This cannot live in the conductor's own instructions. It reads untrusted text by
design, and content that can drive it into a creation loop can equally override a
"create at most N per round" line in its skill. Self-restraint is a convention; this
is a control, enforced for every caller regardless of which agent is calling.

Two details in that guard are load-bearing, and both are pinned by tests:

• Folders are rate-limited for INTERNAL callers only. The endpoint is
  mixed-path: the browser's own "new folder" control posts to it, and a person
  organizing their chats can legitimately create a dozen in one sitting, so
  throttling them would be a regression with no security value. _audit_origin
  already distinguishes the two — a request without the internal secret is the
  browser.
• The bucket keys on the validated caller name, not the session key. The session
  key would give finer granularity but is partly caller-supplied:
  _refuse_unattributable_caller refuses a dashboard: key naming a dead slot, but
  not a rotating non-dashboard key, so keying on it would let a caller earn a fresh
  budget per spelling. The caller name is checked against _KNOWN_INTERNAL_CALLERS.
  The cost is that internal callers share one folder budget, acceptable when a goal
  needs exactly one.

Behind the rate guard sit two capacity ceilings, both added here because neither
resource had an adequate one:

• chat_folder_create had no bound at all. Every other allocation path in the
  dashboard tests a ceiling; this one did not, so a loop could grow durable on-disk
  state without limit. Now MAX_CHAT_FOLDERS = 500, tested inside
  mutate_folders — len(folders) is only authoritative while the lock is held,
  which is the same reason the parent is re-checked and order recounted there.
• session_create had a global ceiling but no distribution. MAX_LIVE_SLOTS = 500 bounds the total, so one caller could hold all 500 and every later create —
  the person opening a new chat tab included — got the 429. Now
  MAX_SLOTS_PER_CREATOR = 50 bounds what a single caller holds, with slots created
  through the verb attributed to their caller (_created_by); a person's own tab and
  a fork are attributed to nobody and stay bounded by the global ceiling alone, so
  ordinary human use never consumes an automated caller's share. That attribution is
  in-memory and resets on restart, which is precisely why the rate guard — not this
  ceiling — is the control that has to hold adversarially.

Half 1 names that resource class on purpose rather than saying "any pre-existing
resource". A create always writes some shared bookkeeping — the slot table, the
folder index, the session-pulse counter — so a literal reading would forbid every
create and decide nothing. What it protects is state the person arranged and would
have to rebuild by hand.

Verb	Verdict	Why
chat_folder_tree	granted	read of the caller's visible tree
chat_folder_create	granted	new folder; _refuse_tree_shaping_if_unverifiable refuses an unverifiable caller, and bounded by the new MAX_CHAT_FOLDERS
session_create	granted	new session, bounded globally by MAX_LIVE_SLOTS and per caller by the new MAX_SLOTS_PER_CREATOR. One recorded side effect: it mints with origin=SlotOrigin.USER, which is what keeps the session correctly private, and the session-pulse counter increments on that tag — so conductor sessions count toward the survey's "10 genuine user chats" window. A pre-existing conflation in the counter (it miscounts for every caller of the verb, including a hand-approved one), deferred as #6139
session_read_message	granted	read-only, and the verb an unattended patrol cycle actually needs
chat_folder_move_session	withheld	writes another session's folder_id — the PATCH targets the session named in the arguments; the strict caller key is only the authority header. mcp_dashboard calls it "the one tool here that writes to a session OTHER than the caller's"
chat_folder_move	withheld	reparents an existing tree; no conductor step needs it
session_send	withheld	runs text as a peer's user-role turn under that target's grants
session_stop	withheld	ends a peer's in-flight turn and discards its work

Every withheld verb stays mounted (@kirocrew-dashboard is still in tools)
— it just passes through hooks.on_tool_call like any ungranted tool. The cost is
an approval when a round files a session, seeds a child, or stops one.

Being precise about when that cost lands, because the obvious framing overstates
the fix: on round 1 those prompts follow the human's plan approval, so someone is
present. On a multi-round goal they are not — round N+1 dispatches from the patrol
loop as items complete, so a filing or seeding prompt can land mid-patrol with
nobody there, which is the same stall shape defect 3 describes. So this PR reduces
the unattended-patrol stall rather than ending it: the read-only patrol cycle
(chat_folder_tree, session_read_message) no longer prompts, and #6118 removes the
filing call, but a mid-patrol dispatch still blocks until then.

The same honesty applies to the evaluator. execute_bash stays ungranted, and
the conductor's acceptance check runs accept_eval.py through it, so every patrol
cycle still pays one approval regardless of anything in this diff — the skill's own
cost note says so. Ending the per-cycle stall means giving the evaluator a non-shell
entry point (tracked in #6160), not widening this grant list.

The narrowing is real on both backends, not cosmetic — this was verified before
relying on it, since a per-tool ref that matched nothing would silently grant
nothing: kiro-cli's is_tool_in_allowlist checks @server and then builds
@{server}/{tool} and matches that (tool_permission_checker.rs), and
allowed_tools_to_permissions maps the same entry to an exact KAS server/tool
resource. No kirocrew-dashboard/* wildcard is emitted, so nothing re-grants the
withheld verbs through the KAS path.

execute_bash stays ungranted for a different reason worth keeping distinct:
allowedTools is name-scoped with no argument matching, so trusting the two
bundled scripts (accept_eval.py, ledger_entry.py) cannot be distinguished from
trusting arbitrary shell. There is no per-argument form of that grant the way there
is a per-tool form of the MCP one. The installer's docstring and the skill's cost
note both state what the code now does.

Tests

All in test/test_conductor_agent.py, which already owned this installer's
contract — four of its existing tests failed against the permission change and were
updated rather than worked around:

The rate guard (test/test_create_rate_limit.py, new — 8 tests):

• budget-then-refuse, and refill as the window slides (a refused caller must recover,
  or one burst disables the verb permanently)
• per-caller buckets: one caller exhausting its budget must not refuse another
• separate budgets per verb, so filing a goal's folder cannot consume the session
  budget that same goal needs
• fail-closed on an empty caller key and on an unknown verb, so a future verb wired
  up with a typo'd name is throttled to zero rather than silently unlimited
• test_a_dispatch_round_for_a_real_goal_fits_in_one_window — the limits must not
  bind honest work: one folder plus twelve sessions in a window. Lowering the session
  budget under ~12 fails this, which is the signal the limit has been tightened into
  the legitimate path.
• stale-bucket sweep, since session keys churn for the gateway's lifetime — the
  reason this does not reuse the never-evicting AppRateLimiter

The two ceilings and the endpoint wiring (test/test_chat_folder_cap.py, new, and
additions to test/test_session_control.py):

• test_creation_is_refused_at_the_ceiling / test_the_last_folder_under_the_ceiling_is_still_allowed
  — the refusal and the boundary, so an off-by-one cannot cost the user their last
  folder.
• test_the_ceiling_is_tested_under_the_lock — a concurrent creator fills the tree
  inside the mutate_folders callback; hoisting the check above the lock makes this
  land folder 501.
• test_an_internal_caller_is_rate_limited_at_the_endpoint and
  test_the_browser_is_not_rate_limited — both directions of the mixed-path split.
  Applying the limiter unconditionally makes the second one 429.
• test_a_created_slot_records_the_caller_that_asked_for_it — attribution, without
  which every caller's count stays 0 and the per-creator ceiling is decorative.
• test_one_caller_cannot_consume_everybody_elses_slots — the hog is refused at its
  own share while the global ceiling is nowhere near full, and a second caller still
  succeeds.
• test_slots_nobody_asked_for_are_charged_to_nobody — pins both directions: a
  person's tab must not burn an automated caller's budget, and an empty creator key
  must not match every unattributed slot.

The grant surface (test/test_conductor_agent.py, 29 tests):

• test_only_create_and_read_verbs_are_auto_approved — pins the invariant by
  literal verb name in both directions: the four withheld verbs absent, the four
  granted present. Deliberately not asserted against _CONDUCTOR_DASHBOARD_GRANTS
  itself, which would be tautological and could never catch a widening.
• test_no_dashboard_verb_is_granted_outside_the_named_set — asserts the granted
  dashboard set is exactly those four and that the bare server never appears.
  This is the guard the per-verb lists cannot give: a NEW tool added to the
  dashboard server, or a stray tuple entry, fails here instead of being
  auto-approved without anyone ruling on it.
• test_no_write_tool_and_shell_not_preapproved — no fs_write, and
  execute_bash mounted but never granted.
• test_grants_pass_through_the_governance_ceiling — a ceiling opinion on
  @kirocrew-core strips that grant while leaving the ref mounted; the expected
  list carries the four per-verb dashboard grants.
• test_kas_permissions_are_derived_from_the_filtered_grants — the dashboard
  grants project to EXACT server/tool resources, plus an explicit assertion that
  no kirocrew-dashboard/* wildcard appears. That wildcard is what would silently
  re-grant the withheld verbs on the KAS backend, so it is pinned rather than
  assumed.
• test_a_fully_governed_host_still_emits_the_permissions_key — the {"rules": []}
  branch, whose mere presence is what makes KAS load the spec, kept its coverage
  when a ceiling on @kirocrew-core alone stopped emptying the rule list.
• test_audit_failure_does_not_break_the_install — the spec still lands when the
  SEL sink raises.
• test_skill_gates_the_plan_once_instead_of_interrogating,
  test_skill_requires_the_goal_folder_before_the_first_session,
  test_skill_states_the_real_approval_cost — doc ratchets in the same style as
  the file's existing test_skill_documents_artifacts_as_a_string_map, because for
  a skill it is the instruction, not the code, that regresses. The folder one also
  pins the failed-move stop path; the cost one pins which verbs prompt, so the
  skill and the spec cannot drift apart.

1986 tests pass across the changed surfaces (test_session_control.py,
test_chat_folder_cap.py, test_chat_folder_ownership.py,
test_chat_folder_app_isolation.py, test_chat_folder_store_shape.py,
test_chat_slot_create_folder.py, test_dashboard_chat.py, test_agent.py,
test_kas_permissions.py, test_session_pulse_session_count.py and
test_conductor_agent.py).

Manual verification

N/A for the behavioural half — the fixes are instructions the model follows, and
exercising them means handing a live conductor a real goal, which is the reporter's
next step rather than something a unit test can stand in for.

Verified by hand for the permission half, in the direction that could have made the
narrowing a lie: per-tool refs are honoured on both backends, checked in source
before relying on them (is_tool_in_allowlist in kiro-cli's
tool_permission_checker.rs, _mcp_pattern in acp/kas_permissions.py). There is
no precedent in this repo for a per-tool allowedTools entry — every prior MCP
grant is whole-server — so a ref that matched nothing would have shipped "no grant
at all" while the description claimed a narrowed one.

Also verified the claim the withheld list turns on: chat_folder_move_session
PATCHes /api/chat/slots/{target}/folder where the target comes from
_resolve_chat_slot_key(args["session"], ...), not from the caller.

The installed ~/.kiro/agents/kirocrew-conductor.json was updated by hand to the
same result this installer produces, so the reporter can test on their current
install without waiting for a rebuild.

Local gates green on the pushed head: test/test_conductor_agent.py, isort,
flake8, mypy, check_black_formatting.py, docs_lint.py, check_brand_name.py,
check_harness_parity.py, check_changelog_history.py, check_focus_cue.py,
check_loop_bound_locks.py. Local review ran both profile reviewers against their
extracted CI contracts on each revision.

Related Issues

no linked issue: reported directly from a live conductor run, not filed.

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

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

The single candidate describes the folder rate-limit bucket being keyed on the validated caller name (kirocrew-dashboard) rather than a per-session key, so internal callers share one 10/window budget. This is a deliberate, documented design choice: the code comment explains keying on the session key was rejected because it is partly caller-supplied (_refuse_unattributable_caller only refuses a dashboard: key naming a dead slot), letting a caller rotate keys to earn fresh budgets. The chosen key is checked against _KNOWN_INTERNAL_CALLERS, closing that escape. The "shared budget" cost is explicitly accepted in the comment. The only harm is a temporary 429-with-retry that requires >10 internal folder creates within a 5-minute window — a scenario the discovery pass itself could not ground to a realistic operating point (its own confidence was "low"). This does not meet (a)/(b)/(c) at the required bar: the "observable wrong outcome" is throttling under a rare load pattern, deliberately traded for a closed attribution-escape hole. Dropped.

I re-derived the rest of the diff independently: the _created_by attribution is written synchronously after get_or_create_slot with no suspension point before the creator_slot_count cap test, so the concurrent-create race the comment describes is genuinely closed; the folder cap is tested under mutate_folders' lock; allow_create fails closed on empty key / unknown verb; _audit_origin cannot make the new call crash. No new groundable defect surfaced.

No findings.

[OPUS-REVIEWED] 4cdca99

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

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

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

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

Design-Verdict: CONCERNS

The unattended-patrol fix is partial: every cycle still blocks on the execute_bash evaluator approval, and later-round dispatches still block mid-patrol.

Watch

• The stated harm for fix 3 is "a cycle that blocks on an approval… stalls rather than patrols," yet the updated skill still says "each patrol cycle blocks on one approval for the accept_eval.py invocation plus one per codec call." Granting the dashboard reads removes some prompts, but the loop's acceptance-check step blocks every cycle regardless, so the patrol remains attended in practice — the PR narrows the prompt count without ending the stall it names.
• The claim that filing/seeding approvals "happen right after a human approved the plan" holds only for round 1; on a multi-round goal, round N+1 dispatches from the patrol loop when items complete, so chat_folder_move_session and session_send prompts land mid-patrol with nobody present, stalling exactly the way problem 3 describes until session_create should accept a folder so filing is atomic with creation #6118 (and nothing) removes the seed prompt.

Suggestions

• Ship accept_eval and ledger_entry as MCP tools (the repo's own MCP-first rule already demands this for LLM-facing CLI commands); as read/compute verbs they'd qualify under the stated grant invariant and would actually close the per-cycle stall — the follow-up that completes this PR's problem 3.

[DESIGN-REVIEWED] 4cdca99

[github-actions]

GPT 5.6 Review — ✅ human override accepted

Human judgment by @iamwhatever overrides the GPT 5.6 finding for 4cdca990c67ba7506589aca208bf2e0913f8ea29; the recorded reason is authoritative for this commit.

This comment is updated in place on each push.

The model was not re-run because an authorized human decision supersedes it.

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

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 4cdca990c67ba7506589aca208bf2e0913f8ea29 — 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: _KNOWN_INTERNAL_CALLERS is a one-entry frozenset, the session ledger has its own cap (so no unbounded sibling among the already-granted verbs), reset_for_tests matches an existing repo pattern (7 prior prod files), and the two existing rate limiters genuinely can't do this job as-is (AppRateLimiter never evicts; auth_refresh's buckets are module-private and IP-keyed). Final review follows.

First-Principles-Verdict: PASS

Three reported defects, each fixed at the declared level, every new guard derived from the untrusted-content boundary, and every alternative counted rather than assumed.

What this change ships

Intent: make the conductor's first live-run failures not recur — plan without interrogating, file sessions under the goal, patrol without blocking on approvals. This is a FIX.

1. First reply is the plan; decidables become stated Assumptions — justified (reported defect)
2. A goal that already authorizes execution dispatches immediately, no re-gate — justified
3. Goal folder is a round precondition; failed create stops the round — justified; symptom-level, cause (session_create should accept a folder so filing is atomic with creation #6118) declared and deferred
4. Failed folder-move stops the item before seeding — justified (same cause)
5. Four dashboard read/create verbs stop prompting; move/send/stop still prompt — justified (nudge-driven patrol vs. untrusted-content boundary)
6. Session creation rate-limited per caller, 429 past 20/5min — justified, declared
7. Folder creation rate-limited for internal callers only; browser exempt — justified, declared
8. Global folder ceiling (500) — justified (only unbounded create path, counted by author)
9. Per-creator slot ceiling (50) via new in-memory attribution — justified (distribution, not just total)
10. Three new 429 code values — rides along with 6–9, mandated by the AGENTS.md code-field rule

Watch

• The repo now holds three in-memory rate limiters (grepped: notifications/rate_limit.py token bucket, handlers/auth_refresh.py deque buckets, new create_rate_limit.py). Non-reuse is argued from fundamentals in each case, but the new copy follows auth_refresh's shape while dropping its _REFRESH_RATE_MAX_BUCKETS map bound — benign here (both key spaces are bounded: a one-entry caller frozenset; strict-resolved live slot keys plus sweep), yet the two deque spellings will diverge under maintenance.
• Items 3–4 harden the instruction layer that already failed once (the run dropped "once per goal (skip if it exists)"); the enforcing fix is the deferred folder argument (session_create should accept a folder so filing is atomic with creation #6118). Acceptable as declared — but the skill wording is a convention, not a control, until session_create should accept a folder so filing is atomic with creation #6118 lands.

[FIRST-PRINCIPLES-REVIEWED] 4cdca99

[iamwhatever]

Design Review dispositions

• Auto-approving session_send removes the last call-time human check on WHAT is sent — needs-a-decision.

  The conductor ingests external content by design ... the server-side gates scope which target, not what is sent ... the PR's bounding argument covers identity/target, not input provenance — a human should sign off on that tradeoff explicitly.

  Not rebutted: the finding is correct and it names a real gap in my own argument. Everything
  I offered as the bound — _resolve_session_key_strict(), workspace scoping, the
  ineligible-caller classes, the live-slot cap — constrains which session can be reached.
  None of it constrains the content of a seed derived from an issue body, and the approval
  prompt was the only call-time human check in that chain. What remains is real but weaker:
  agent.session_control still defaults OFF and fails closed, delivery is tagged in the
  target's transcript as sent by another session, and every call is SEL-audited — so the
  chain is visible and disabled by default, but not interrupted.

  Since this is a security tradeoff and not a code question, it is not mine to settle, and I
  am deliberately not filing an issue for it — the question would go unread in a tracker.
  @zejiangg, the ruling I need: accept the grant as-is (patrol runs unattended, content
  provenance is bounded only by the fail-closed switch and the audit trail), or narrow it so
  session_send specifically keeps prompting while the folder/create/read verbs are trusted
  — which restores the content checkpoint at the cost of re-stalling any patrol cycle that
  needs to steer a child. allowedTools is server-granular, so the narrow option means
  granting the individual @kirocrew-dashboard/<tool> refs instead of the server.

• The _may_auto_approve ceiling filter is an install-time snapshot — rebutted (out of proportion here).

  a ceiling tightened after install doesn't reach an already-written spec, and the calls now skip the runtime gate. Pre-existing pattern (same as @kirocrew-core), but this grant raises what rides on it.

  The mechanism criticism holds and I am not disputing it, but it is not introduced by this
  diff: session, report and @kirocrew-core have always been written through the same
  install-time filter, and so is every other allowedTools writer in the repo (the shared
  may_skip_gate_now entry point exists precisely because there are five of them). Closing
  it needs a runtime re-evaluation path for already-written specs — a new mechanism, repo-wide
  in scope, and a decision about what a mid-flight tightening should do to a running session.
  That is not something a three-file conductor fix should introduce, and half-introducing it
  for one grant would leave the inconsistency it is meant to remove. Left as-is deliberately,
  and stated here rather than silently.

• Follow-up: add folder to SESSION_CREATE_SCHEMA — accepted-and-deferred.

  add folder to SESSION_CREATE_SCHEMA so filing is atomic with creation — the PR itself names the missing argument as the root cause, and the mid-round-deleted-folder failure prose exists only to compensate for it.

  Agreed, and filed as session_create should accept a folder so filing is atomic with creation #6118 with the concrete call sites (schema, tool definition, handler,
  create_session's synchronous window) and the instruction-layer prose to delete once it
  lands. Out of scope here for the reason you name — it is a mechanism change, while this PR
  is repairing three reported behaviours.

[iamwhatever]

First Principles dispositions

• The description called a fixable schema gap "unavoidable" — fixed.

  The description calls filing "unavoidably a second call", but the two-call protocol is an earlier choice, not a constraint: SESSION_CREATE_SCHEMA (src/kiro_crew/validation.py:2628, handler src/kiro_crew/mcp_dashboard.py:861) simply lacks a folder field ... the description should not present it as impossible.

  You are right and the word was wrong. "Unavoidable" described the schema as it stands and
  read as though the protocol could not be otherwise, which overstated the case for the
  choreography I then added. The PR body now says the second call is an earlier design
  choice, not a constraint, names the schema line, states that a folder field would delete
  the whole block, and points at session_create should accept a folder so filing is atomic with creation #6118. Prose only — no code change, no new push, head stays
  cb43e4b3d198.

• Subtraction: delete the precondition prose and its doc ratchets once filing is atomic — accepted-and-deferred.

  Defer, then delete: once session_create files atomically, remove the "Before the round" precondition paragraph and the failed-move stop/retry prose from goal-conductor/SKILL.md plus the two doc-ratchet tests pinning them (test_skill_requires_the_goal_folder_before_the_first_session).

  Recorded as the second half of session_create should accept a folder so filing is atomic with creation #6118 rather than left as a note here, so the deletion is
  part of that task instead of something a later reader has to rediscover: the issue names the
  paragraph, the separate move step, both failure-stop paths, and the test to remove — and
  says the replacement test should pin the folder argument being passed at dispatch. Your
  point that the mid-round-deletion race cannot be closed at the instruction layer is the
  reason I am not treating the current prose as a fix: it is a mitigation with a stated hole,
  and session_create should accept a folder so filing is atomic with creation #6118 is what closes it.

  Not done here for the reason you already granted — it is larger than this change, and the
  three reported defects are worth repairing before the mechanism is reworked.

[iamwhatever]

Two GPT-lane concerns, one per rationale. New head dfd6b53b34f1.

• chat_folder_move_session auto-approved (BLOCK-MERGE on cb43e4b3d198) — fixed in dfd6b53b34f1.

  The verb writes another session's folder_id, so auto-approving it lets ingested content refile or unfile a same-workspace session the user filed by hand.

  Verified against the code rather than the surrounding identity check, and the lane was right where a local reviewer was wrong: the PATCH goes to /api/chat/slots/{slot_key}/folder where slot_key comes from _resolve_chat_slot_key(args["session"], ...) — the session named in the arguments. _resolve_session_key_strict() supplies only the authority header, so a strict caller check establishes authority, not target; mcp_dashboard.py says as much itself ("the one tool here that writes to a session OTHER than the caller's"). Withheld, and still mounted so it passes the approval gate.

  Rather than patch the list a third time — two rounds of narrow-then-find-another-verb in one span is the recurrence pattern — the grant list now carries an invariant that decides all eight verbs, plus a test asserting the granted dashboard set is exactly four and never the bare server. chat_folder_move, session_send and session_stop fall out of the same rule.

• session_create advances the session-pulse counter, so the survey's 10-human-chat window can pass with zero human chats — accepted-and-deferred as #6139, with the demanded fix rebutted on proportionality.

  create_session mints SlotOrigin.USER slots -> increment_user_session_count_off_loop advances session_pulse_sessions.json, making the survey's 10-human-chat gate pass with zero human-created chats -> Fix: remove session_create from _CONDUCTOR_DASHBOARD_GRANTS.

  The mechanism is real and I confirmed every link: create_session mints with origin=SlotOrigin.USER (session_control.py:780), and get_or_create_slot increments on exactly minted_new and origin == SlotOrigin.USER (state.py:6817). The counter's own docstring names "the session-control create verb" as a path that reaches it, and test_session_pulse_session_count.py states the intent as "a person starting a dashboard chat". So an agent-created session is counted as a human one. Nothing pinned that as intended — no test asserts the session-control path counts.

  The fix does not follow, for two reasons. It is wrong-layer: SlotOrigin.USER is correct on that slot and is what keeps the session properly private from apps holding slots:user, so the defect is the counter using an ownership tag as a proxy for "a person started a chat" — it miscounts for every caller of the verb, including one a human approves by hand, and predates this PR. And it is disproportional: session_create is the verb this change exists to make usable unattended, so removing it to protect a survey counter defeats the PR while leaving the conflation in place for other callers.

  Not point-fixed here because the real fix carries a default that should not be chosen incidentally — whether the two request-layer paths opt in (a missed site means the survey never fires) or the session-control path opts out (a future USER-origin caller silently over-counts). That is a call for the session-pulse surface's owner; Session-pulse counter counts agent-created sessions as human chats #6139 lays out the three call paths and both directions. Impact meanwhile is survey timing only: no workspace state altered, nothing lost.

  What did change in dfd6b53b34f1: the finding showed the invariant as written overclaimed. It said "never mutate a resource that already exists", but every create writes some shared bookkeeping — the slot table, the folder index, this counter — so read literally it forbids all four granted verbs and decides nothing. It now names the resource class it always meant (user-visible workspace state: a session's contents or liveness, and the arrangement of sessions and folders), and the session_create entry records this side effect with a pointer to Session-pulse counter counts agent-created sessions as human chats #6139 instead of leaving it implicit. All eight verdicts are unchanged by that rewording.

[iamwhatever]

Both concerns fixed in 4cdca990c67b, at a different layer than the one the fixes proposed. Recording the span history first, because it is the reason the layer changed: this is the 4th consecutive blocking finding in _CONDUCTOR_DASHBOARD_GRANTS (whole-server grant -> chat_folder_move_session -> creation-exhaustion -> these two). Each round narrowed or bounded the list and the next round found the next unhandled branch, so this round stopped patching the list and added the missing control instead.

• The global folder ceiling still lets injected content exhaust the user's folder capacity — fixed.

  Malicious external content -> unattended conductor loops the auto-approved create -> 500 durable, person-owned-looking folders fill MAX_CHAT_FOLDERS -> the dashboard user's next legitimate folder creation returns 429.

  Correct, and it was an inconsistency in the previous revision rather than a misreading: sessions got a per-caller share and folders were left a single global cap, so the reservation reasoning was applied to one resource and not its sibling. Rather than add a folder-side reservation (which needs agent-vs-human attribution on a mixed-path endpoint), the fix bounds the rate: dashboard/create_rate_limit.py allows 10 folder creates per caller per 5-minute sliding window, 429 beyond it. A loop cannot reach the ceiling faster than the window allows, and MAX_CHAT_FOLDERS remains behind it as the absolute bound.

  Two properties of that guard are pinned by tests, because getting either wrong would be worse than the finding: it applies to internal callers only (_audit_origin distinguishes them; the browser's own "new folder" control posts to this same endpoint and a person organizing chats can legitimately create a dozen in a sitting), and it buckets on the validated caller name rather than the session key, since _refuse_unattributable_caller refuses a dead dashboard: key but not a rotating non-dashboard one — keying on that would hand a caller a fresh budget per spelling.

• The session quota resets on restart and can still reach the global ceiling — fixed.

  ... restart rehydrates every child with _created_by="" because the attribution is never persisted -> repeat until MAX_LIVE_SLOTS is full ...; multiple conductor slots can likewise consume 50 each in one run.

  Both halves verified: _created_by appears nowhere outside state.py and session_control.py, and the birth-persistence dict writes origin, workspace, agent, project, title and memory_mode but not the attribution. So the lifetime quota is re-earnable, exactly as described.

  The fix is not to persist it. Making a lifetime quota durable means every path that rehydrates a session must carry its attribution, and there are 27 get_or_create_slot call sites across 12 modules, three in the persistence layer — a bound that leaks on whichever branch nobody enumerated, which is the failure mode this span has already produced three times. Instead the guard that must hold adversarially is the one with no durability requirement: a 5-minute window is not meaningfully resettable, because a restart buys a caller one window rather than a clean slate, and it is indifferent to how many conductor slots exist since each is bucketed separately. At 20 session creates per caller per window, filling MAX_LIVE_SLOTS from empty takes over two hours of uninterrupted looping, every step visible in the sidebar. MAX_SLOTS_PER_CREATOR stays as a secondary ceiling; its restart-reset is now a degradation of a backstop rather than of the primary control, and the code says so.

  Worth stating explicitly since it is the obvious alternative: this deliberately is not implemented as the conductor limiting itself. It ingests untrusted text by design, so content that can drive it into a creation loop can equally override a "create at most N per round" line in its skill. Self-restraint is a convention; this is enforced in the endpoint for every caller regardless of which agent calls it or what it was told.

New tests: test/test_create_rate_limit.py (8, including fail-closed on empty key and unknown verb, per-caller isolation, per-verb budgets, the stale-bucket sweep, and test_a_dispatch_round_for_a_real_goal_fits_in_one_window so the limit cannot be tightened into the legitimate path unnoticed) plus endpoint-level coverage of both directions of the internal/browser split.

[iamwhatever]

rebutted — the finding describes a chain that is fully present on main and is not introduced by this diff.

Malicious fetched content -> auto-approved tree/read calls -> auto-approved send_message to a configured shared channel -> private peer transcript disclosed without approval. Fix: Remove session_read_message from _CONDUCTOR_DASHBOARD_GRANTS.

Every link of that chain is already auto-approved before this PR, through a grant this PR does not touch. On origin/main, _install_conductor_agent grants ("session", "report", "@kirocrew-core") — the core server whole — and separately grants web_fetch:

• Untrusted ingest: web_fetch, granted on main (agent.py), and deliberately so — the charter's worked example is reading an issue tracker.
• Transcript read: @kirocrew-core includes search_chat_history and get_chat_session (mcp_tools/sessions.py). get_chat_session returns the full message transcript of another session, resolved by a key from search_chat_history, scoped to the caller's workspace. That is a broader read than session_read_message: it spans the workspace's stored history rather than one live peer's tail.
• Exfiltration: send_message is also a kirocrew-core tool (mcp_core.py), so the outbound half is auto-approved on main too.

So the read+send capability this finding is about predates the diff, and removing session_read_message would not close it — get_chat_session serves the same data class through the same already-granted server. The proposed fix therefore does not achieve what the finding asks for, while removing the one verb the unattended patrol cycle actually needs. That combination is what makes this a rebuttal rather than a deferral.

Two things I am not claiming. I am not claiming the residual is uninteresting: an agent that reads untrusted text, can read workspace transcripts, and can post to a shared channel is a real design property worth someone ruling on deliberately, so it is filed as #6154 rather than left implicit. And I am not claiming session_read_message is free — it is the verb that lets the conductor read the sessions it created, which is precisely why it is the one dashboard read the nudge-driven cycle cannot work without.

On the trust model, since it is the premise the whole grant list rests on: the conductor is an agent a person installs and invokes to run unattended, and its allowedTools set is where that consent is recorded. A conductor that prompts on every read cannot patrol at all — it wakes itself on a nudge and then blocks on an approval nobody is present to give, which is the exact defect this PR was opened to fix. The grant is bounded by the invariant recorded on _CONDUCTOR_DASHBOARD_GRANTS: create-or-read only, never mutate pre-existing user-visible state, and every granted create bounded server-side by rate and by ceiling.

Span record: this is the 5th consecutive blocking finding in _CONDUCTOR_DASHBOARD_GRANTS (whole-server grant -> chat_folder_move_session -> creation-exhaustion -> folder-ceiling + restart-reset -> this). Rounds 1-4 were each accepted and fixed at the layer the finding pointed at, including two new server-side bounds. This round is the first where the named mechanism is not attributable to the diff, so it is answered rather than patched, and a formal override follows.

[iamwhatever]

/ai-review override gpt 4cdca99: The read+send exfiltration chain is complete on origin/main without this diff — the conductor already holds web_fetch plus a whole-server @kirocrew-core grant, which registers get_chat_session (full workspace transcript read, broader than session_read_message) and send_message (outbound); removing session_read_message would not close the chain and would remove the only dashboard read the unattended patrol cycle needs. Pre-existing design property tracked in #6154.

[github-actions]

Human judgment recorded

@iamwhatever marked the gpt AI finding as false positive, not applicable, or explicitly accepted for 4cdca990c67ba7506589aca208bf2e0913f8ea29.

The read+send exfiltration chain is complete on origin/main without this diff — the conductor already holds web_fetch plus a whole-server @kirocrew-core grant, which registers get_chat_session (full workspace transcript read, broader than session_read_message) and send_message (outbound); removing session_read_message would not close the chain and would remove the only dashboard read the unattended patrol cycle needs. Pre-existing design property tracked in #6154.

This decision applies only to this commit. A new push requires a new judgment.

[iamwhatever]

Both concerns are correct, and the second one corrects a claim I made in this PR's own description. Taking them individually.

• The per-cycle execute_bash approval means the patrol is still attended — accepted-and-deferred as #6160.

  The stated harm for fix 3 is "a cycle that blocks on an approval… stalls rather than patrols," yet the updated skill still says "each patrol cycle blocks on one approval for the accept_eval.py invocation" plus one per codec call. Granting the dashboard reads removes some prompts, but the loop's acceptance-check step blocks every cycle regardless, so the patrol remains attended in practice — the PR narrows the prompt count without ending the stall it names.

  This holds exactly as written, and the skill's own cost note is the evidence. What this PR can honestly claim is narrower than "the patrol no longer stalls": the read-only part of the cycle (chat_folder_tree, session_read_message) stops prompting, and that is the part a nudge-driven cycle runs when nothing needs dispatching. The acceptance check is not fixable from the grant list, and the reason is structural rather than a judgment call: allowedTools is name-scoped with no argument matching, so granting execute_bash would trust arbitrary shell, not the two bundled scripts. There is no per-argument form of that grant the way there is a per-tool form of the MCP one — which is why the asymmetry between execute_bash and the dashboard verbs is recorded in the code rather than left as an oversight.

  Your suggestion is the right fix and I have filed it as the follow-up rather than attempting it here: give accept_eval and ledger_entry non-shell entry points, at which point they become individually grantable and qualify under the stated invariant (the check reads and computes; the ledger write creates). Conductor patrol still blocks once per cycle on the execute_bash evaluator #6160 carries that reasoning, the two call sites, and your point that the repo's MCP-first convention already covers this case.

• "Filing and seeding happen right after a human approved the plan" holds only for round 1 — fixed in the PR description (prose only; the diff is unchanged).

  on a multi-round goal, round N+1 dispatches from the patrol loop when items complete, so chat_folder_move_session and session_send prompts land mid-patrol with nobody present, stalling exactly the way problem 3 describes

  You are right and I was overclaiming. That sentence appeared in the PR body and in an earlier disposition comment of mine, and it is only true of the first round; a multi-round goal dispatches from inside the patrol loop as items complete, so a filing or seeding prompt can land with nobody there. That is the same stall shape defect 3 names, so presenting it as "the human is already present" papered over a real residual.

  The body now states the limit explicitly instead — round 1 prompts follow the human's approval, later-round dispatches do not — and says plainly that this PR reduces the unattended-patrol stall rather than ending it. session_create should accept a folder so filing is atomic with creation #6118 removes the filing call; the seeding prompt and the per-cycle evaluator prompt (Conductor patrol still blocks once per cycle on the execute_bash evaluator #6160) are what remain. No code changed for this: the honest description was the thing that was wrong.

[iamwhatever]

Verdict is PASS; answering both Watch items individually since each names something checkable.

• The new limiter drops auth_refresh's _REFRESH_RATE_MAX_BUCKETS bound, and the two deque spellings will diverge — rebutted on the bound, accepted on the divergence.

  the new copy follows auth_refresh's shape while dropping its _REFRESH_RATE_MAX_BUCKETS map bound — benign here (both key spaces are bounded: a one-entry caller frozenset; strict-resolved live slot keys plus sweep), yet the two deque spellings will diverge under maintenance.

  Confirmed the premise: auth_refresh.py does carry _REFRESH_RATE_MAX_BUCKETS = 4096 and fails closed at it (line 159), and create_rate_limit.py has only the age-based sweep. The omission is deliberate, and your own parenthetical is the reason — but it is worth stating why the two cases genuinely differ rather than leaving it as "benign here". auth_refresh keys on client IP: attacker-chosen and unbounded, so a count cap is the only thing standing between it and unbounded map growth, and failing closed at the cap is correct there. This module's key space is structurally bounded: folder buckets key on a validated name from a one-entry frozenset, and session buckets key on a strict-resolved live slot key, so the map cannot exceed roughly MAX_LIVE_SLOTS + 1 entries even before the sweep runs. A count cap would add a fail-closed path that can only fire when something else has already gone wrong, and failing closed on creation is a different (worse) tradeoff than failing closed on a token refresh.

  The divergence point I simply accept — two hand-written deque limiters in one repo will drift, and a third copy is a real maintenance cost that the fundamentals argument does not erase. I have not folded them together here because the shared abstraction would have to reconcile three different key spaces, two different bound strategies and two different failure directions, which is a refactor with its own review rather than a rider on a conductor fix.

• The skill wording is a convention, not a control, until session_create should accept a folder so filing is atomic with creation #6118 lands — accepted-and-deferred, and this is the more important of the two.

  Items 3–4 harden the instruction layer that already failed once (the run dropped "once per goal (skip if it exists)"); the enforcing fix is the deferred folder argument (session_create should accept a folder so filing is atomic with creation #6118).

  Exactly right, and it deserves to be said plainly rather than softened: the folder precondition is prose in a skill file, and prose in a skill file is what already failed on the first live run — the conductor dropped the very line this PR rewrote. Hoisting it to a round-level precondition with a fail-stop makes it harder to skip and gives it a doc-ratchet test, but it remains a convention an agent can drop, not a mechanism that refuses. The enforcing fix is #6118 (a folder argument on session_create, making the filing atomic and deleting both the second call and the prose that describes it). Until that lands, the honest claim for items 3–4 is "the instruction is now unambiguous and tested for presence", not "sessions cannot be created unfiled".

[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 #6237 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 #6237: REBASE. Main's per-verb-name auto-approval landed after this branch was cut and cannot be expressed under one tool name; the mapping is an author-owned security decision, which is why the PR carries the needs-author-decision label. Files: src/kiro_crew/agent.py.

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