feat(taskrunner): honor a spec-declared approval: auto mode

[SebastianYuSun]

Problem / Motivation

The autonomous task runner (task_run / "run this task" on a spec) prompts for per-action tool approval on essentially every step, and the dashboard's "Trust All" control does not stop it (#2068).

Why it matters

Anyone running a task spec unattended is forced to approve nearly every tool call, and the one control that looks like it should fix this — "Trust All" — has no effect on the runner. The capability that is meant to enable unattended execution effectively does not, so the runner cannot be left to work on its own.

What changed (motivation → approach → change)

• Symptom: every step of a runner task raises an interactive approval prompt, and pressing "Trust All" in the dashboard does not silence them.

• Root cause: "Trust All" sets trust on the interactive chat slot (DashboardState slot _trust). The runner executes each step in its own per-task session (taskrunner:{task_id}:task{N}), not under that slot. In task_executor.execute_task's EVENT_PERMISSION_REQUEST loop, the gateway's _interactive_approval("taskrunner") callback checks the parent slot's _trust, which for the runner's synthetic session is not the slot the user trusted — so every tool call falls through to a prompt. (The old "all open conversations are trusted" fallback that once bridged this was deliberately removed as a privilege escalation: a background job should not inherit a chat's trust.)

• Change: a spec may now declare its intended approval mode in leading YAML frontmatter, so a trusted plan carries that intent in version control:

  ---
  approval: auto
  ---
  # Task: refactor the widget
  

A declaration is not a grant, and that is the whole design. The declaration is reported to the human before they press Execute, as a read-only declared_approval field on the plan response rendered beside the auto-approve checkbox. The checkbox is not pre-checked. Unattended execution is granted only by the launching human's own request body, and both launch paths derive it from auto_approve alone:

auto_approve = await _gate_auto_approve(
    request, body.get("auto_approve") is True, ..., endpoint=...)

An earlier revision of this PR OR-ed the declaration into that grant. Three reviews (GPT 5.6 blocking, First Principles and Design advisory) independently named the same defect: a server that derives a grant from spec content lets a spec obtained from an untrusted source disable that human's approval prompts on its own authority, and the launch-provenance gate does not catch it because it checks who launched, not whether the human consented to unattended. The or is gone.

Removing it removed the reason most of the code existed, so this revision deletes far more than it adds (+546 / −7, and the net insertion count is down from 1226 on the prior head):

deleted	why it could go
parse_spec_approval_mode (240 lines) → spec_declares_auto(text) -> bool	while a declaration was a grant, every reading of it was an authorization decision, so the scanner had to defend every surface a directive could hide in
_markup_rejected_lines, _SPEC_FENCE_RE, _HTML_TAG_RE, the fence state machine	frontmatter is a delimited region, not a prose window, so the markup-splicing hole (approval:<span>per-task</span> auto synthesizing approval: auto) is now structurally absent rather than merely guarded
_approval_head, _write_spec_snapshot, snap-<hex>/, _full_spec	the read/execute TOCTOU they closed was a consequence of authorization flowing from file bytes; with the grant in the request body there is no content-derived decision to pin

Parsing is consolidated rather than kept separate: frontmatter.py gains a fourth dialect, TASK_SPEC. That is what retires this feature's bespoke scanner — the module's docstring requires each frontmatter grammar be expressed exactly once, and a fifth caller keeping its own copy is what the reviews objected to.

TASK_SPEC adds one axis, reject_duplicate_keys: a key declared twice is dropped entirely rather than resolved by position. This is load-bearing, not tidiness — parse_frontmatter otherwise resolves duplicates silently first-key-wins, and for a security-relevant declaration the line a human reads first need not be the line the parser honors.

force_approval gates and hook deny-lists still block regardless.

Scope / honest limitation: this fixes the dashboard launch path. It intentionally does not make the MCP task_run path (the reporter's own workflow) run unattended — that call belongs to a maintainer. For an unattended background source today the explicit opt-in is hooks.auto_approve_sources (e.g. add "taskrunner"), documented in taskrunner.md.

Tests

• test/test_spec_approval_mode.py — 26 tests. spec_declares_auto is frontmatter-only, so a bare non-frontmatter directive, an unterminated fence, an indented occurrence, a duplicate-key contradiction, and the four markup payloads that defeated the old scanner all report False.
• test/test_auto_approve.py — 36 tests, including TestSpecDeclarationGrantsNothing (a spec declaring auto with no request flag gets NO auto-approval on either /start or /execute) and TestPlanReportsTheDeclarationWithoutActingOnIt.
• test/test_frontmatter.py — 118 tests covering the new TASK_SPEC dialect and the duplicate-key rejection.

Full local gates on this head: flake8, isort, black baseline, tsc --noEmit, eslint (0 errors), i18n:check, and the src/i18n/ suite. The 2 new UI strings are translated in all 11 non-English catalogs plus the generated en-XA pseudolocale (catalogParity), and the Hindi string uses the informal register style/hi.md §4 requires.

Manual verification

Captured against a real gateway started through the repo's own offline E2E harness (kiro_crew.testing.harness.spawn_feature_gateway, isolated $KIROCREW_HOME, loopback-bound), driving the real Projects page in Chromium.

The load-bearing detail is the pairing: the spec's declaration is surfaced, and the Auto-approve tool calls checkbox beside it is unchecked. The human still has to grant it.

Disclosed so the evidence is not read as more than it is: the planned run is real (seeded through the real POST /api/taskrunner/from-chat, no model involved), but one HTTP response is stubbed — POST /api/taskrunner/plan, which awaits a real model decomposition that the capture host cannot perform (no OS-level sandbox backend: unshare(CLONE_NEWUSER) returns EPERM). declared_approval rides only on that response. The stub returns the genuinely-seeded run's task_id plus declared_approval: true — the same payload the handler produces for a spec whose frontmatter declares approval: auto. The component, styling, layout and checkbox state in the images are all real.

Related Issues

Refs #2068

Checklist

• Single commit with a Conventional Commits title (feat: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) — docs/system-specs/modules/taskrunner.md
• No secrets, credentials, or internal references in the diff

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

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

Review details

FINDING -- src/kiro_crew/dashboard/handlers/taskrunner.py:576 -- "declared_approval" is only displayed while the checkbox remains unchecked, so approval: auto still prompts per action despite the stated purpose -> Fix: initialize the existing per-run checkbox from a true declaration so Execute routes the request through _gate_auto_approve.
[GPT-REVIEWED] 03532ff

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 03532ff2b34a67615d8ccacbddcf6204988f08b4 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

The shipped code is safe and sound, but the PR description contradicts it — it claims a server-side OR that the diff deliberately does not implement, overstating what the feature does.

Watch

• Description says api_taskrunner_start "OR-s the declared intent with the UI auto_approve flag" and the table says Dashboard + approval: auto "runs unattended." The shipped code does the opposite: declared_approval is a read-only field on /plan, the checkbox stays unchecked, and TestSpecDeclarationGrantsNothing pins the OR as absent. A reviewer approving on the description would believe a security-relevant grant exists that doesn't. Rewrite the description to match the advisory-only design before merge.
• Given the OR was removed, the net capability added is a UI label plus a pointer to the pre-existing hooks.auto_approve_sources. The declaration enables no new unattended execution — the human still ticks the box each run, as they already could. Confirm this thin advisory signal actually satisfies Autonomous task runner prompts for per-action approval and ignores /tools trust-all #2068's ask rather than just documenting the existing opt-in.

Suggestions

• reject_duplicate_keys and the markup/BOM defenses harden a field the code itself calls "advisory rather than authorizing." Fail-closed on a UI hint is defensible but is more machinery than a label needs; fine to keep, not worth expanding.

[DESIGN-REVIEWED] 03532ff

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] 03532ff

[github-actions]

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

Premise-level review of 03532ff2b34a67615d8ccacbddcf6204988f08b4 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.

Findings

The base already ships full auto-approve gating: _gate_auto_approve on both api_taskrunner_start (taskrunner.py:180) and api_taskrunner_execute_plan (taskrunner.py:638), driven by the request body's auto_approve. The diff does not touch either. Its only functional change is adding a read-only declared_approval to /plan plus a UI label — the gating is untouched.

Consumer counts (from the diff; grep of the base found only an unrelated _reject_duplicate_keys in packaging/): spec_declares_auto → 1 (the /plan handler); declared_approval → 1 (ProjectsPage.tsx); reject_duplicate_keys axis → set by 1 dialect (TASK_SPEC).

Framing gap is the headline. Title: "honor a spec-declared approval: auto mode." Description: "api_taskrunner_start OR-s the declared intent with the UI auto_approve flag." Neither is in the diff — and the PR's own docs contradict it: "the declaration is never OR-ed into it." What ships is advisory-only and does not fix #2068 (the description concedes the reporter's MCP task_run path is deliberately left unfixed, and the dashboard checkbox already worked at base).

reject_duplicate_keys is justified as hardening for "a security-relevant declaration," yet the same PR argues the field is advisory and "needs none of the hardening a content-derived grant needed." The worst case a duplicate causes is a wrong advisory label — no grant. That is an axis with no harm to prevent.

I'm writing the verdict now.

First-Principles-Verdict: CONCERNS

The shipped change is an advisory label, not the "honor approval: auto" the title/description claim — gating is untouched and #2068's reporter is explicitly left unfixed.

What this change ships

Intent: let task specs run unattended (fix #2068) — but this is an ADDITION of an advisory-only label; gating is unchanged.

1. New approval: spec-frontmatter convention — advisory, grants nothing; invented and consumed by this PR — circular harm
2. spec_declares_auto() reports the declaration — one consumer
3. declared_approval read-only field on /plan — one consumer (UI)
4. reject_duplicate_keys dialect axis + TASK_SPEC — one dialect sets it; hardening for a field the PR calls non-security
5. UI warning label beside the existing auto-approve checkbox — justified only by item 1
6. 12 locale strings — ride along with the label
7. taskrunner.md section — DERIVED (spec-update invariant)

Watch

• Title says "honor" and description says api_taskrunner_start "OR-s the declared intent" — the diff contains no such code, and the PR's own docs say "the declaration is never OR-ed." Rewrite the framing to "surface an advisory label," or the change reads as fixing a defect it does not touch.
• The whole approval: convention exists only to render one label re-stating spec text the planner already has; its harm is nameable only because this PR creates the key spec authors write.

Subtractions

• Drop the reject_duplicate_keys axis and the duplicate-scan lines in _parse_block_lines (frontmatter.py) — TASK_SPEC is the sole setter (1 consumer) and the field it guards grants nothing; a wrong advisory label is the entire downside.

[FIRST-PRINCIPLES-REVIEWED] 03532ff

[bolichen97]

🤖 Kiro Crew [operator: bolichen97#bb3ad1ca]: Re-assessed after your update — the trust-model decision we raised has been answered by your pushes (launch-provenance gate on both paths, deny-by-default parse, TOCTOU fix, plus tests and docs), so I've removed the needs-author-decision label.

Current state on 67c038c: every check is green except one Windows test shard, and the single failing test (test_issue_radar_crew_runtime.py::TestDisablingTheAppRevokesInline::test_enabling_again_restores_trust_and_the_loop) is outside this PR's diff footprint and failed via SEL-chain lock contention ("held by another writer"), which looks environmental. I've re-run just the failed jobs — no push to your branch. The pipeline will re-assess on its next cycle once the rerun settles.

[bolichen97]

🤖 Kiro Crew [operator: bolichen97#bb3ad1ca]: Update after the rerun — the Windows shard flake cleared and CI is fully green on 67c038c, but the fork review lanes have now posted verdicts on this head and they need your input:

Blocking readiness (design-level — First Principles BLOCK):

• The new parser is a fifth hand-rolled frontmatter scanner living beside frontmatter.py, whose docstring mandates that logic lives there "exactly once" — either consolidate into frontmatter.py or justify the parallel implementation.
• The bare top-of-file spelling and the per-task / per-action modes have zero consumers in the patch (identical outcome to unknown values) — drop them or wire up a real consumer.
• The bounded-read layer is dead code the doc misdescribes — fix or remove.

Blocking (mechanical, on your code):

• Opus 4.8 (confirmed): the inline branch passes the full unbounded body (spec_text_for_approval = content, up to aiohttp's 1 MB) into the O(n²) HTML-strip regex — a crafted body freezes the event loop for tens of seconds. Bound it via _approval_head(...) like the file branch already does.
• GPT 5.6: conflicting approval directives fail open (first match wins) — scan the full selected region and return "" when multiple directives occur. GPT also wants the snapshot write_text offloaded to asyncio.to_thread, though Opus assessed that same candidate as not load-bearing — your call which way to resolve.

Advisory (Design CONCERNS): content-derived trust can silently override the visible UI auto-approve control for specs from untrusted sources — worth a note on pre-launch visibility even if you keep the current gate.

The premise-level items are design decisions I won't make for you, so I've re-added needs-author-decision. When you've addressed them, the pipeline will re-assess on its next cycle.

[github-actions]

UX Review (Fable 5, fork) — 🟡 CONCERNS

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

UX-Verdict: CONCERNS

The one sentence that tells the user this isn't active ("only a declaration… still approved one at a time") lives only in a title tooltip, so touch, keyboard, and screen-reader users see a warn-colored "Spec declares approval: auto" with no resolution.

Watch

• Tooltip-gated meaning. The label spec_declares_auto_approval renders in text-warn beside an unchecked box, but the clarifying clause is in title=…spec_declares_auto_approval_hint. title tooltips don't fire on touch, aren't keyboard-reachable, and are unreliable for AT — so a user on those inputs reads an alarm-colored "Spec declares approval: auto" and can't tell whether unattended approval is now on. Next to a security-relevant control, that risks either false alarm or a reflexive box-check. Frequency: only auto-declaring specs; impact: possible mis-enable of unattended tool approval; persistence: every such plan. Fix: promote the "only a declaration — you still control it" clause to visible helper text rather than a hover-only title.
• Alarm color for a non-actionable state. text-warn signals caution/problem, but the state it marks is explicitly inert (nothing is auto-approved). Consider text-muted so the color doesn't over-signal a risk that isn't present.

Suggestions

• The label repeats the raw frontmatter token approval: auto; a task-language phrasing ("Spec requests unattended approval") reads without knowing the spec syntax, with the literal key kept in the (now-visible) hint.

[UX-REVIEWED] 03532ff

[SebastianYuSun]

@bolichen97 this one is ready for another look, and it is a different design from the one you last reviewed.

The blocking finding was that the server derived the auto-approve grant from spec content (body.get("auto_approve") is True or spec_declares_auto(...)). That or is gone. A spec now only declares its intended mode; the grant comes solely from the launching human's own request body, and the declaration is surfaced next to the auto-approve checkbox at Execute time with the box left unchecked. Screenshots of that are in the description.

Removing the grant removed the reason most of the code existed, so the revision subtracts rather than adds: parse_spec_approval_mode (240 lines) collapses to a boolean spec_declares_auto, and the markup-rejection scanner, the code-fence state machine and the snap-<hex>/ TOCTOU machinery are all deleted. First Principles had asked for two specific things — consolidate the fifth hand-rolled frontmatter scanner into frontmatter.py, and drop the bare top-of-file spelling and the zero-consumer per-task/per-action modes. Both are done: parsing is now a fourth frontmatter.py dialect, TASK_SPEC.

One detail worth flagging since it is easy to miss in review. TASK_SPEC sets a new reject_duplicate_keys axis, because parse_frontmatter otherwise resolves duplicates silently first-key-wins, and for a security-relevant declaration the line a human reads first need not be the line the parser honors. Two conflicting approval: keys therefore read as "nothing was declared".

Two asks when you have a moment. The branch needs another "Approve and run" — PR Readiness had already passed on the previous head 03532ff2b, and pushing the screenshot commit reset it, so CI is re-running from scratch. After that it needs one code-owner review; there are currently zero on the PR.

Full local gates are green on this head: 26 + 36 + 118 tests across the three affected suites, flake8, isort, the black baseline, tsc --noEmit, eslint with zero errors, and i18n:check. The two new UI strings are translated in all eleven non-English catalogs plus the generated en-XA pseudolocale, and the Hindi string uses the informal register style/hi.md §4 asks for.

[bolichen97]

@SebastianYuSun Thanks for this. Two things before it can move.

Already on main. The auto-approve grant path the title says this PR honors is implemented already: _gate_auto_approve in src/kiro_crew/dashboard/handlers/taskrunner.py guards both the start and execute-plan endpoints, and hooks.AutoApproveConfig.auto_approve_sources covers the background-source opt-in. That predates your merge base. Merged #7518 adds an approval_modes governance scope, but it governs the chat approval-mode picker, not the taskrunner plan path.

Still missing, and only this PR provides it: the advisory surface, meaning the TASK_SPEC dialect in src/kiro_crew/frontmatter.py, spec_declares_auto() in src/kiro_crew/task_planner.py, the read-only declared_approval field on the plan endpoint, and the label in website/src/pages/ProjectsPage.tsx. Nothing on main and no other open PR ships that.

Please narrow the PR to that advisory surface and retitle to match, since the diff reports a declaration and grants nothing (your docs say it is never OR-ed in). Before spending the rebase: the branch is about 1050 commits behind with mergeable_state dirty, main's frontmatter.py gained a STEERING_LOADER dialect and ProjectsPage.tsx was restructured around pendingAutoApproveRef, so all four code hunks need re-authoring. Also issue #2068 is closed as completed with no linked commit, so please confirm the label is still wanted. #5987 touches the same module but disjoint functions, so no reconciliation there.

Overlap with #5274 (cc @atomsbaza): both open a new section in docs/system-specs/modules/taskrunner.md at the same anchor, a certain hand-resolved conflict. Substantively, #5274 makes review-fix auto_approve conditional on dashboard provenance through that same _gate_auto_approve, so a spec-declared mode reaching a review-fix run would mint approval from a spec rather than from request provenance. #5274 is far larger and further along: let it land first, then rebase and state in that doc section how the two interact.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.