diff --git a/docs/system-specs/modules/taskrunner.md b/docs/system-specs/modules/taskrunner.md index eae3d6e5433..69ffb83ab61 100644 --- a/docs/system-specs/modules/taskrunner.md +++ b/docs/system-specs/modules/taskrunner.md @@ -296,6 +296,56 @@ On gateway restart, any task with `status == "running"` is automatically transit - `cli_server.py` constructs the standalone `kirocrew run TASK.md` runner without an approval callback. Use `force_approval`, not `requires_approval`, for an action that must not execute unattended. - The dashboard supplies the callback and renders Approve/Deny controls in the project detail view. +### Spec-declared approval mode + +A spec may DECLARE its intended approval mode in leading YAML frontmatter, so a +trusted plan carries that intent in version control: + +```markdown +--- +approval: auto +--- +# Task: refactor the widget +``` + +**A declaration is not a grant.** The server derives auto-approval from the +`auto_approve` field of the launching human's request body and from nothing else +— `_gate_auto_approve` is still the only decision point, and the declaration is +never OR-ed into it. So a spec obtained from an untrusted source (repo, chat, +download) cannot disable the launching human's approval prompts, which is what a +content-derived grant did: the provenance gate checks *who launched*, not +*whether that human consented to unattended execution*, so a dashboard launch of +someone else's spec passed the gate on the launcher's own standing. + +What the declaration does instead is reach the person who owns the checkbox: + +- `task_planner.spec_declares_auto()` reports it. `True` only for a literal + `auto` (case-insensitive) as a top-level frontmatter key; the grammar is + `frontmatter.TASK_SPEC`. +- `POST /api/taskrunner/plan` returns it as read-only `declared_approval`, at no + extra disk read — `spec_content` is already in memory at plan time. +- The project detail view renders it beside the existing auto-approve checkbox + and leaves that box **unchecked**. The trust decision stays a deliberate click + at Execute time, matching the existing rule that a planned or resumed run + shows unchecked because its live grant was torn down. + +Because the reading is advisory rather than authorizing, it needs none of the +hardening a content-derived grant needed. Two consequences worth stating: + +- Only frontmatter counts. A bare top-of-file `approval:` line is not a + declaration, so there is no window of arbitrary document text to scan and no + code-fence or HTML-markup class to defend against. +- **Duplicate keys fail closed.** `parse_frontmatter` returns a dict, so two + `approval:` keys would otherwise be resolved silently by position — and + whichever declaration a human reads first need not be the one the parser + honored. `TASK_SPEC` sets `reject_duplicate_keys`, so two or more occurrences + report "not declared". + +Operators who want an unattended *background* source still use the separate +explicit opt-in `hooks.auto_approve_sources` (e.g. add `"taskrunner"`), which is +consent scoped to a source rather than to a file's contents. `force_approval` +gates block regardless. + ## Parallel Execution Parallel groups are throttled to prevent resource exhaustion from simultaneous kiro-cli cold starts. Each kiro-cli cold start spawns MCP server child processes, so concurrent tasks multiply startup pressure. diff --git a/src/kiro_crew/dashboard/handlers/taskrunner.py b/src/kiro_crew/dashboard/handlers/taskrunner.py index 0c3f7a0441a..b143146d32d 100644 --- a/src/kiro_crew/dashboard/handlers/taskrunner.py +++ b/src/kiro_crew/dashboard/handlers/taskrunner.py @@ -12,7 +12,7 @@ from kiro_crew.dashboard.state import DashboardState from kiro_crew.security import is_sensitive_path, redact_credentials, redact_exfiltration_urls -from kiro_crew.task_planner import plan_to_yaml +from kiro_crew.task_planner import plan_to_yaml, spec_declares_auto logger = logging.getLogger(__name__) @@ -565,6 +565,15 @@ async def api_taskrunner_plan(request: web.Request) -> web.Response: [s.index for s in group] for group in state.task_runner._group_parallel_tasks(run.tasks, set()) ], + # READ-ONLY report of what the spec DECLARED, so the reviewing human + # sees it before pressing Execute and can grant unattended execution + # through their OWN request body (the auto-approve checkbox). The + # server never reads this back: `/execute` derives auto-approve from + # `auto_approve` alone, so a spec cannot grant itself anything. + # + # Costs no disk read — `spec_content` was captured in memory at plan + # time and is the same snapshot the run executes from. + "declared_approval": spec_declares_auto(run.spec_content or ""), } ) diff --git a/src/kiro_crew/frontmatter.py b/src/kiro_crew/frontmatter.py index c89d96f7b2d..7021a884230 100644 --- a/src/kiro_crew/frontmatter.py +++ b/src/kiro_crew/frontmatter.py @@ -1,6 +1,6 @@ """Single home for hand-rolled SKILL.md frontmatter parsing. -Four backend callers parse ``key: value`` frontmatter from markdown, and each +Five backend callers parse ``key: value`` frontmatter from markdown, and each historically carried its own copy of the scanner. The copies drifted: they disagree on whether the opening fence may carry trailing text or leading whitespace, whether an indented ``key: value`` line is a field or prose, @@ -18,7 +18,16 @@ :data:`SKILL_LOADER` rather than owning a dialect: what the preview shows must match what the skills loader computes after install. -A FIFTH consumer lives outside Python and cannot be reached from this list by +A fifth caller — a task spec's declared approval mode +(``task_planner.spec_declares_auto``) — arrived with its own scanner and was +folded in as :data:`TASK_SPEC` rather than kept separate, which is what +retired that copy's bespoke code-fence and HTML-markup machinery: a delimited +frontmatter region needs neither. It adds no quirk the mirror below must +track: :data:`TASK_SPEC` is a new dialect, and its one new axis +(``reject_duplicate_keys``) is off for every pre-existing dialect, so +:data:`SKILL_LOADER`'s grammar is byte-for-byte what it was. + +A SIXTH consumer lives outside Python and cannot be reached from this list by import: the skill editor's frontmatter splicer (``website/src/components/SkillForm.tsx``) MIRRORS :data:`SKILL_LOADER`'s grammar in TypeScript, because it must never write a value this reader would @@ -90,6 +99,15 @@ class FrontmatterDialect: # True: the first occurrence of a duplicate key wins (single-value lookup # semantics). False: the last occurrence wins (dict-overwrite semantics). first_key_wins: bool = False + # True: a key declared TWICE OR MORE is dropped from the result entirely, + # so a contradiction reads as absent. Where `first_key_wins` picks a winner + # by position, this refuses to pick one at all — for a caller whose field + # is a security-relevant declaration, whichever line a human reads first + # need not be the line a positional rule honors, so the safe reading of two + # conflicting declarations is "nothing was declared" (GPT 5.6, PR #2129). + # Orthogonal to `first_key_wins`: rejection is decided over the raw + # occurrence count, so it holds under either positional rule. + reject_duplicate_keys: bool = False # Resolve a bare block-scalar indicator value (see # BLOCK_SCALAR_INDICATORS) from the blank-or-indented lines that follow # it, via fold_block_scalar. Dialects without this store the indicator @@ -135,6 +153,23 @@ class FrontmatterDialect: resolve_block_scalars=True, ) +# ``task_planner.spec_declares_auto`` — a task spec's declared approval mode. +# Leading whitespace before the opener is tolerated (specs are hand-authored, so +# a stray blank first line must not silently mean "declared nothing"); indented +# keys are prose, so an ``approval:`` nested under another mapping is not a +# top-level declaration; quotes are stripped so ``approval: "auto"`` reads the +# same as bare. Block scalars are deliberately NOT resolved: resolving them would +# let ``approval: |`` fold an indented continuation line into a declaration, and +# for this caller the indicator character is simply not the declared word. +# Duplicate keys are REJECTED rather than resolved by position — see +# `reject_duplicate_keys`. +TASK_SPEC = FrontmatterDialect( + extraction="leading_ws_fence", + indent_policy="reject_indented", + strip_quotes=True, + reject_duplicate_keys=True, +) + def fold_block_scalar(indicator: str, block: list[str]) -> str: """Resolve a YAML block scalar's indented lines into a single value. @@ -266,6 +301,10 @@ def _extract_block( def _parse_block_lines(lines: list[str], dialect: FrontmatterDialect) -> dict[str, str]: """Scan block lines into a field dict under *dialect*'s line rules.""" fields: dict[str, str] = {} + # Keys seen more than once, collected only when the dialect rejects them. + # Recorded during the scan and applied after it, because the second + # occurrence is what makes the FIRST one unusable too. + duplicated: set[str] = set() i = 0 while i < len(lines): line = lines[i] @@ -294,7 +333,13 @@ def _parse_block_lines(lines: list[str], dialect: FrontmatterDialect) -> dict[st value = fold_block_scalar(value, block) elif dialect.strip_quotes: value = value.strip("\"'") + # Checked BEFORE the positional rule below, which would otherwise skip + # the line that proves the key was declared twice. + if dialect.reject_duplicate_keys and key in fields: + duplicated.add(key) if dialect.first_key_wins and key in fields: continue fields[key] = value + for key in duplicated: + del fields[key] return fields diff --git a/src/kiro_crew/task_planner.py b/src/kiro_crew/task_planner.py index cc978412fe5..ade4cdb5805 100644 --- a/src/kiro_crew/task_planner.py +++ b/src/kiro_crew/task_planner.py @@ -9,6 +9,7 @@ from typing import TYPE_CHECKING, Any from kiro_crew.executors import run_in_embed_pool +from kiro_crew.frontmatter import TASK_SPEC, parse_frontmatter from kiro_crew.hooks import TOOL_DENY from kiro_crew.llm_helpers import _extract_json_of_type from kiro_crew.platform.context import redact_log_via_context @@ -52,6 +53,50 @@ def auto_name(spec_content: str, spec_path: str = "") -> str: return "" +# ── Spec-declared approval mode ── + + +def spec_declares_auto(spec_content: str) -> bool: + """Report whether a spec's frontmatter DECLARES ``approval: auto``. + + A spec may state its intended approval mode in a leading YAML frontmatter + fence, so a trusted plan can carry that intent in version control:: + + --- + approval: auto + --- + # Task: refactor the widget + + This REPORTS a declaration and GRANTS nothing. Unattended execution is + granted only by the launching human's own request body — the dashboard's + auto-approve checkbox, routed through ``_gate_auto_approve`` — and the server + never derives that grant from spec bytes. The declaration is surfaced + pre-launch so the human can act on it; a spec obtained from an untrusted + source therefore cannot disable that human's approval prompts on its own + authority (GPT 5.6 / First-Principles / Design review, PR #2129). + + That separation is also what keeps this function small. While a declaration + was a grant, every reading of it was an authorization decision, so the + scanner had to defend the whole surface a directive could hide in — code + fences, HTML comments, wrapper elements, a bounded top-of-file window. + Reporting intent carries no such burden: the reading is advisory, the region + is delimited, and the grammar is :data:`~kiro_crew.frontmatter.TASK_SPEC`. + + ``True`` only for a literal ``auto`` under a top-level ``approval`` key. + Anything else is ``False``: no frontmatter, an unterminated fence, another + value, an indented occurrence, or TWO conflicting ``approval:`` keys + (rejected, never resolved by position). + + The VALUE is case-insensitive; the KEY is not, so ``Approval: auto`` declares + nothing. That asymmetry is the whole module's behavior — no dialect folds key + case (:func:`~kiro_crew.frontmatter.parse_frontmatter` keeps ``Name`` verbatim + for ``SKILL_LOADER`` too) — and it errs toward per-action prompting, so it is + left as-is rather than special-cased here. + """ + declared = parse_frontmatter(spec_content or "", TASK_SPEC).get("approval", "") + return declared.strip().lower() == "auto" + + # ── Parallel Task Grouping ── diff --git a/temp-screenshots/spec-declared-approval/projects-declared-approval-detail.png b/temp-screenshots/spec-declared-approval/projects-declared-approval-detail.png new file mode 100644 index 00000000000..cb622200147 Binary files /dev/null and b/temp-screenshots/spec-declared-approval/projects-declared-approval-detail.png differ diff --git a/temp-screenshots/spec-declared-approval/projects-declared-approval.png b/temp-screenshots/spec-declared-approval/projects-declared-approval.png new file mode 100644 index 00000000000..3aa3dbc75f7 Binary files /dev/null and b/temp-screenshots/spec-declared-approval/projects-declared-approval.png differ diff --git a/test/test_auto_approve.py b/test/test_auto_approve.py index d1b1ca75e9e..34054015041 100644 --- a/test/test_auto_approve.py +++ b/test/test_auto_approve.py @@ -23,6 +23,7 @@ from kiro_crew.dashboard.handlers.taskrunner import ( api_taskrunner_execute_plan, + api_taskrunner_plan, api_taskrunner_start, ) from kiro_crew.hooks import TOOL_AUTO_APPROVE, TOOL_DENY @@ -630,3 +631,166 @@ async def test_a_successful_inline_start_keeps_its_spec_file(self, tmp_path: Pat assert resp.status == 200 payload = json.loads(resp.text) assert Path(payload["spec"]).is_file() + + +# ══════════════════════════════════════════════════════════════════════ +# A spec DECLARES; only the human's request body GRANTS +# ══════════════════════════════════════════════════════════════════════ + + +class TestSpecDeclarationGrantsNothing: + """`approval: auto` in a spec must never produce auto-approval on its own. + + The server derives the grant from `auto_approve` in the request body ALONE. + An earlier revision of this feature OR-ed the parsed declaration into the + grant on both launch paths, which let spec CONTENT stand in for a human's + explicit consent (GPT 5.6 BLOCK, PR #2129); First-Principles and Design + named the same target. These tests pin the OR as absent — on both paths, and + from a dashboard context, where a grant WOULD be honored if one were asked + for. + """ + + DECLARING = "---\napproval: auto\n---\n# Task: t\n## Steps\n1. do\n" + + async def _start_auto_approve(self, tmp_path: Path, body: dict): + runner = MagicMock() + runner._work_dir = str(tmp_path) + runner.start_background = AsyncMock(return_value="tid") + app = web.Application() + app["state"] = SimpleNamespace(task_runner=runner) + req = make_mocked_request("POST", "/api/taskrunner", app=app) + req["app"] = "" # dashboard context: a requested grant WOULD be honored + req.json = AsyncMock(return_value=body) + resp = await api_taskrunner_start(req) + assert resp.status == 200, resp.text + return runner.start_background.call_args + + @pytest.mark.asyncio + async def test_inline_declaring_spec_without_the_flag_gets_nothing( + self, tmp_path: Path + ) -> None: + call = await self._start_auto_approve( + tmp_path, {"spec": "__inline__:" + self.DECLARING, "source": "dashboard"} + ) + assert call.kwargs["auto_approve"] is False + + @pytest.mark.asyncio + async def test_inline_declaring_spec_with_the_flag_false_gets_nothing( + self, tmp_path: Path + ) -> None: + # An explicit `false` is the STRICT default the dashboard now sends: the + # checkbox is unchecked even when the spec declares auto. + call = await self._start_auto_approve( + tmp_path, + { + "spec": "__inline__:" + self.DECLARING, + "source": "dashboard", + "auto_approve": False, + }, + ) + assert call.kwargs["auto_approve"] is False + + @pytest.mark.asyncio + async def test_the_human_flag_is_what_grants(self, tmp_path: Path) -> None: + # Control: the same declaring spec DOES run unattended once the human + # asks for it, so the tests above are measuring the OR's absence and not + # a gate that simply refuses everything. + call = await self._start_auto_approve( + tmp_path, + { + "spec": "__inline__:" + self.DECLARING, + "source": "dashboard", + "auto_approve": True, + }, + ) + assert call.kwargs["auto_approve"] is True + + @pytest.mark.asyncio + async def test_a_file_backed_declaring_spec_executes_from_its_own_path( + self, tmp_path: Path + ) -> None: + # No content-derived grant means no content-derived TOCTOU to close, so + # the approval snapshot is gone: the run executes from the caller's own + # file and the work dir gains no `snap-/` directory to leak. + spec = tmp_path / "declaring.md" + spec.write_text(self.DECLARING, encoding="utf-8") + work = tmp_path / "work" + work.mkdir() + call = await self._start_auto_approve( + str(work), {"spec": str(spec), "source": "dashboard", "auto_approve": True} + ) + assert call.kwargs["auto_approve"] is True + assert call.args[0] == str(spec.resolve()) + assert list(work.iterdir()) == [] + + async def _execute_auto_approve(self, body: dict, spec_content: str): + runner = MagicMock() + runner.execute_plan = AsyncMock(return_value=None) + runner._runs = { + "t1": Project( + spec_path="s.md", spec_content=spec_content, status="planned", task_id="t1" + ) + } + app = web.Application() + app["state"] = SimpleNamespace(task_runner=runner) + req = make_mocked_request( + "POST", "/api/taskrunner/t1/execute", app=app, match_info={"task_id": "t1"}, + headers={"Content-Length": "32"}, + ) + req["app"] = "" + req.json = AsyncMock(return_value=body) + await api_taskrunner_execute_plan(req) + return runner.execute_plan.call_args.kwargs["auto_approve"] + + @pytest.mark.asyncio + async def test_execute_declaring_spec_without_the_flag_gets_nothing(self) -> None: + assert await self._execute_auto_approve({}, self.DECLARING) is False + + @pytest.mark.asyncio + async def test_execute_declaring_spec_with_the_flag_false_gets_nothing(self) -> None: + assert await self._execute_auto_approve({"auto_approve": False}, self.DECLARING) is False + + @pytest.mark.asyncio + async def test_execute_honors_the_human_flag_on_the_same_spec(self) -> None: + assert await self._execute_auto_approve({"auto_approve": True}, self.DECLARING) is True + + +class TestPlanReportsTheDeclarationWithoutActingOnIt: + """`/plan` surfaces `declared_approval` so the human can decide. + + This is the whole replacement for the deleted server-side OR: the + declaration reaches the person who owns the checkbox, and reaches nothing + else. + """ + + async def _plan(self, spec_content: str): + runner = MagicMock() + run = Project( + spec_path="s.md", spec_content=spec_content, status="planned", task_id="t1" + ) + runner.plan = AsyncMock(return_value=run) + runner._group_parallel_tasks = MagicMock(return_value=[]) + app = web.Application() + app["state"] = SimpleNamespace(task_runner=runner) + req = make_mocked_request("POST", "/api/taskrunner/plan", app=app) + req.json = AsyncMock(return_value={"input": "do the thing", "source": "text"}) + resp = await api_taskrunner_plan(req) + assert resp.status == 200, resp.text + return json.loads(resp.text) + + @pytest.mark.asyncio + async def test_a_declaring_spec_is_reported(self) -> None: + payload = await self._plan("---\napproval: auto\n---\n# Task: t\n") + assert payload["declared_approval"] is True + + @pytest.mark.asyncio + async def test_a_silent_spec_is_reported_as_false(self) -> None: + payload = await self._plan("# Task: t\n") + assert payload["declared_approval"] is False + + @pytest.mark.asyncio + async def test_an_empty_spec_content_is_reported_as_false(self) -> None: + # `plan(source="yaml")` leaves spec_content empty; the field must still + # serialize rather than raising on None/"". + payload = await self._plan("") + assert payload["declared_approval"] is False diff --git a/test/test_frontmatter.py b/test/test_frontmatter.py index 01694e52920..e66b64676be 100644 --- a/test/test_frontmatter.py +++ b/test/test_frontmatter.py @@ -23,6 +23,7 @@ ONBOARDING_IMPORT, SKILL_LOADER, SKILL_UPDATE, + TASK_SPEC, FrontmatterDialect, frontmatter_value, parse_frontmatter, @@ -295,14 +296,14 @@ def test_none_and_empty(self) -> None: class TestDialectContracts: - """The three dialects stay distinct — collapsing any two axes silently + """The four dialects stay distinct — collapsing any two axes silently changes some caller's accepted-input surface.""" def test_presets_are_distinct(self) -> None: - presets = [SKILL_LOADER, ONBOARDING_IMPORT, SKILL_UPDATE] + presets = [SKILL_LOADER, ONBOARDING_IMPORT, SKILL_UPDATE, TASK_SPEC] keys = { (p.extraction, p.indent_policy, p.strip_quotes, p.first_key_wins, - p.resolve_block_scalars) + p.resolve_block_scalars, p.reject_duplicate_keys) for p in presets } assert len(keys) == len(presets) @@ -316,6 +317,33 @@ def test_first_key_wins_vs_last(self) -> None: assert frontmatter_value(text, "k", SKILL_UPDATE) == "first" assert parse_frontmatter(text, SKILL_LOADER)["k"] == "second" + def test_duplicate_rejection_is_per_dialect(self) -> None: + # The third position on the duplicate axis: neither occurrence wins, so + # the key is ABSENT. A caller whose field is a security-relevant + # declaration reads absent as "not declared" and denies. + text = "---\nk: first\nk: second\n---\n" + assert "k" not in parse_frontmatter(text, TASK_SPEC) + assert frontmatter_value(text, "k", TASK_SPEC) == "" + # Other keys in the same block are untouched — rejection is per key. + mixed = "---\nk: first\nother: kept\nk: second\n---\n" + assert parse_frontmatter(mixed, TASK_SPEC) == {"other": "kept"} + + def test_duplicate_rejection_is_decided_before_the_positional_rule(self) -> None: + # `first_key_wins` skips the second occurrence, which is the line that + # PROVES the key was declared twice. Counting first is what makes the two + # axes independent rather than order-dependent. + both = FrontmatterDialect( + extraction="column0_fence", + indent_policy="reject_indented", + strip_quotes=True, + first_key_wins=True, + reject_duplicate_keys=True, + ) + assert parse_frontmatter("---\nk: first\nk: second\n---\n", both) == {} + + def test_a_single_key_survives_a_rejecting_dialect(self) -> None: + assert parse_frontmatter("---\nk: only\n---\n", TASK_SPEC) == {"k": "only"} + def test_quote_stripping_is_per_dialect(self) -> None: text = '---\nk: "v"\n---\n' assert parse_frontmatter(text, SKILL_LOADER)["k"] == "v" @@ -334,7 +362,7 @@ def test_quote_strip_never_applies_to_a_resolved_scalar(self) -> None: assert parse_frontmatter(text, SKILL_LOADER)["k"] == '"quoted"' def test_split_returns_text_unchanged_without_block(self) -> None: - for dialect in (SKILL_LOADER, ONBOARDING_IMPORT, SKILL_UPDATE): + for dialect in (SKILL_LOADER, ONBOARDING_IMPORT, SKILL_UPDATE, TASK_SPEC): assert split_frontmatter("plain prose", dialect) == ({}, "plain prose") def test_custom_dialect_axes_compose(self) -> None: diff --git a/test/test_spec_approval_mode.py b/test/test_spec_approval_mode.py new file mode 100644 index 00000000000..cab98c46360 --- /dev/null +++ b/test/test_spec_approval_mode.py @@ -0,0 +1,154 @@ +"""Unit tests for the spec-declared approval mode. + +`task_planner.spec_declares_auto()` reports whether a spec's leading YAML +frontmatter DECLARES `approval: auto`. It grants nothing: the grant comes from +the launching human's request body (`auto_approve`), so these tests are about +what is REPORTED, never about what is honored. The gating tests live in +`test_auto_approve.py`. + +Deny-by-default throughout: anything other than an unambiguous top-level +`approval: auto` reports False. +""" + +from __future__ import annotations + +from kiro_crew.task_planner import spec_declares_auto + + +class TestDeclared: + def test_frontmatter_auto(self): + assert spec_declares_auto("---\napproval: auto\n---\n# Task: do the thing\n") is True + + def test_auto_alongside_other_keys(self): + spec = "---\nname: Widget refactor\napproval: auto\nowner: me\n---\n# Task: x\n" + assert spec_declares_auto(spec) is True + + def test_quoted_value(self): + assert spec_declares_auto('---\napproval: "auto"\n---\n') is True + assert spec_declares_auto("---\napproval: 'auto'\n---\n") is True + + def test_value_is_case_insensitive_but_the_key_is_not(self): + assert spec_declares_auto("---\napproval: AUTO\n---\n") is True + assert spec_declares_auto("---\nApproval: auto\n---\n") is False + + def test_leading_blank_lines_tolerated(self): + # A spec is hand-authored; a stray blank first line must not silently + # read as "declared nothing" (frontmatter.TASK_SPEC: leading_ws_fence). + assert spec_declares_auto("\n\n---\napproval: auto\n---\n") is True + + +class TestDenyByDefault: + def test_empty_and_none_safe(self): + assert spec_declares_auto("") is False + assert spec_declares_auto(None) is False # type: ignore[arg-type] + + def test_no_frontmatter(self): + assert spec_declares_auto("# Task: do the thing\n1. step\n") is False + + def test_other_modes_are_not_auto(self): + # `per-task`/`per-action` are simply not `auto`. Nothing enumerates them + # any more: only the literal `auto` is meaningful, so there is no mode + # table left for a typo to land next to. + assert spec_declares_auto("---\napproval: per-task\n---\n") is False + assert spec_declares_auto("---\napproval: per-action\n---\n") is False + + def test_unknown_value(self): + assert spec_declares_auto("---\napproval: yolo\n---\n") is False + + def test_prose_value_never_matches(self): + assert spec_declares_auto("---\napproval: pending review of the design\n---\n") is False + + def test_substring_is_not_the_value(self): + assert spec_declares_auto("---\napproval: automatic\n---\n") is False + assert spec_declares_auto("---\napproval_mode: auto\n---\n") is False + + def test_indented_key_is_not_top_level(self): + # An indented occurrence belongs to an enclosing mapping or block + # scalar, not to the document's top level. + assert spec_declares_auto("---\n approval: auto\n---\n") is False + assert spec_declares_auto("---\nsteps:\n approval: auto\n---\n") is False + assert spec_declares_auto("---\napproval:\n auto\n---\n") is False + + def test_block_scalar_cannot_fold_into_a_declaration(self): + # TASK_SPEC does not resolve block scalars, so the indicator character + # is the value and the indented continuation stays prose. + assert spec_declares_auto("---\napproval: |\n auto\n---\n") is False + assert spec_declares_auto("---\napproval: >\n auto\n---\n") is False + + def test_unterminated_frontmatter_is_not_frontmatter(self): + assert spec_declares_auto("---\napproval: auto\n# Task: x\n") is False + + def test_body_directive_is_outside_the_region(self): + # Everything after the closing fence is body text, at any depth or + # distance — including a fenced example. + assert spec_declares_auto("---\nname: x\n---\napproval: auto\n") is False + assert spec_declares_auto("---\nname: x\n---\n```\napproval: auto\n```\n") is False + + def test_bare_top_of_file_directive_is_no_longer_honored(self): + # Deliberate narrowing: a declaration must live in frontmatter. The bare + # form required scanning arbitrary document text, which is what forced + # the code-fence and HTML-markup defenses this refactor deleted. + assert spec_declares_auto("approval: auto\n# Task: x\n") is False + assert spec_declares_auto("# Task: x\napproval: auto\n") is False + + def test_bom_prefixed_spec_fails_closed(self): + # A BOM is not whitespace, so this is not frontmatter and the + # declaration is not reported. Failing closed costs a UI hint, never a + # grant — the human's checkbox is unaffected either way. + assert spec_declares_auto("---\napproval: auto\n---\n") is False + + +class TestMarkupCannotSynthesizeADeclaration: + """The whole value must BE the word, so no deletion of characters can + manufacture one out of two harmless fragments.""" + + def test_element_span_between_key_and_value(self): + # GPT 5.6's original finding against a splicing parser: removing the + # element span from this line leaves `approval: auto`, which a splicing + # reader honors even though the author wrote `per-task`. Nothing is + # removed here, so the value is the whole markup-bearing string. + assert spec_declares_auto("---\napproval:per-task auto\n---\n") is False + + def test_html_comment_on_the_line(self): + assert spec_declares_auto("---\napproval: \n---\n") is False + assert spec_declares_auto("---\napproval: auto \n---\n") is False + + def test_trailing_yaml_comment_is_not_a_declaration_of_auto(self): + # Not a YAML parser: a trailing `#` comment is part of the value, so the + # line does not read as a bare `auto`. Fails closed, not open. + assert spec_declares_auto("---\napproval: auto # trusted plan\n---\n") is False + + +class TestConflictingDeclarationsFailClosed: + """Two `approval:` keys report NOT DECLARED. + + `parse_frontmatter` returns a dict, so without this a duplicate would be + resolved silently by position — and whichever declaration a human reads + first need not be the one the parser honored (GPT 5.6, PR #2129). The guard + is the `reject_duplicate_keys` axis on `frontmatter.TASK_SPEC`. + """ + + def test_auto_then_per_action(self): + assert spec_declares_auto("---\napproval: auto\napproval: per-action\n---\n") is False + + def test_per_action_then_auto(self): + # Both orders, because a first-wins rule and a last-wins rule disagree + # about exactly this input and neither is safe. + assert spec_declares_auto("---\napproval: per-action\napproval: auto\n---\n") is False + + def test_auto_twice(self): + # Even a non-contradictory repeat is refused: the rule is over the + # occurrence count, not over whether the values happen to agree. + assert spec_declares_auto("---\napproval: auto\napproval: auto\n---\n") is False + + def test_second_declaration_separated_by_other_keys(self): + spec = "---\napproval: auto\nname: x\nowner: me\napproval: yolo\n---\n" + assert spec_declares_auto(spec) is False + + def test_an_indented_second_occurrence_is_not_a_duplicate(self): + # Indented lines are prose under this dialect, so they never reached the + # field dict and cannot suppress a real declaration. + assert spec_declares_auto("---\napproval: auto\n approval: per-action\n---\n") is True + + def test_a_body_occurrence_is_not_a_duplicate(self): + assert spec_declares_auto("---\napproval: auto\n---\napproval: per-action\n") is True diff --git a/website/src/i18n/locales/bn.json b/website/src/i18n/locales/bn.json index 3eef5d6a9aa..588bb73ad2a 100644 --- a/website/src/i18n/locales/bn.json +++ b/website/src/i18n/locales/bn.json @@ -11104,6 +11104,8 @@ "running": "চলছে", "schedule": "সময়সূচি", "scheduled_as_daily_cron_job": "দৈনিক cron জব হিসেবে সময়সূচি করা হয়েছে", + "spec_declares_auto_approval": "স্পেক approval: auto ঘোষণা করে", + "spec_declares_auto_approval_hint": "এই স্পেক `approval: auto` ঘোষণা করে। এটি কেবল একটি ঘোষণা — আপনি নিজে বক্সে টিক না দিলে টুল কল একটি একটি করেই অনুমোদিত হয়।", "task_runner": "Task Runner", "upload_a_file": "একটি ফাইল আপলোড করুন", "workspace": "ওয়ার্কস্পেস:", diff --git a/website/src/i18n/locales/de.json b/website/src/i18n/locales/de.json index 28564d6e1fe..17736385953 100644 --- a/website/src/i18n/locales/de.json +++ b/website/src/i18n/locales/de.json @@ -11104,6 +11104,8 @@ "running": "Läuft", "schedule": "Einplanen", "scheduled_as_daily_cron_job": "Als täglicher Cron-Job geplant", + "spec_declares_auto_approval": "Spezifikation deklariert approval: auto", + "spec_declares_auto_approval_hint": "Diese Spezifikation deklariert `approval: auto`. Das ist nur eine Deklaration — Tool-Aufrufe werden weiterhin einzeln genehmigt, sofern Sie das Kästchen nicht selbst aktivieren.", "task_runner": "Task Runner", "upload_a_file": "Eine Datei hochladen", "workspace": "Workspace:", diff --git a/website/src/i18n/locales/en-XA.json b/website/src/i18n/locales/en-XA.json index 7ea6d98e5e7..4afe5eec88f 100644 --- a/website/src/i18n/locales/en-XA.json +++ b/website/src/i18n/locales/en-XA.json @@ -10825,6 +10825,8 @@ "running": "[Ŕùññìñğ ···········]", "schedule": "[Şçĥèðùĺè ············]", "scheduled_as_daily_cron_job": "[Şçĥèðùĺèð àş ðàìĺý çŕøñ ĵøƀ ···················]", + "spec_declares_auto_approval": "[Şþèç ðèçĺàŕèş àþþŕøṽàĺ: àùţø ····················]", + "spec_declares_auto_approval_hint": "[Ţĥìş şþèç ðèçĺàŕèş `àþþŕøṽàĺ: àùţø`. Ìţ ìş øñĺý à ðèçĺàŕàţìøñ — ţøøĺ çàĺĺş àŕè şţìĺĺ àþþŕøṽèð øñè àţ à ţìɱè ùñĺèşş ýøù ţìçķ ţĥè ƀøẋ ýøùŕşèĺƒ. ··········································]", "task_runner": "[Ţàşķ Ŕùññèŕ ··········]", "upload_a_file": "[Ùþĺøàð à ƒìĺè ············]", "workspace": "[Ẁøŕķşþàçè: ···············]", diff --git a/website/src/i18n/locales/en.json b/website/src/i18n/locales/en.json index 6c28d6833be..db676e78ca5 100644 --- a/website/src/i18n/locales/en.json +++ b/website/src/i18n/locales/en.json @@ -7238,6 +7238,8 @@ "running": "Running", "schedule": "Schedule", "scheduled_as_daily_cron_job": "Scheduled as daily cron job", + "spec_declares_auto_approval": "Spec declares approval: auto", + "spec_declares_auto_approval_hint": "This spec declares `approval: auto`. It is only a declaration \u2014 tool calls are still approved one at a time unless you tick the box yourself.", "task_runner": "Task Runner", "upload_a_file": "Upload a file", "workspace": "Workspace:", diff --git a/website/src/i18n/locales/es.json b/website/src/i18n/locales/es.json index ce83016db36..bf5035eb013 100644 --- a/website/src/i18n/locales/es.json +++ b/website/src/i18n/locales/es.json @@ -11263,6 +11263,8 @@ "running": "En ejecución", "schedule": "Programar", "scheduled_as_daily_cron_job": "Programado como trabajo cron diario", + "spec_declares_auto_approval": "La especificación declara approval: auto", + "spec_declares_auto_approval_hint": "Esta especificación declara `approval: auto`. Es solo una declaración: las llamadas a herramientas se siguen aprobando una por una a menos que marques la casilla tú mismo.", "task_runner": "Task Runner", "upload_a_file": "Subir un archivo", "workspace": "Espacio de trabajo:", diff --git a/website/src/i18n/locales/fr.json b/website/src/i18n/locales/fr.json index 8cf9438edc8..d524bbd7e6f 100644 --- a/website/src/i18n/locales/fr.json +++ b/website/src/i18n/locales/fr.json @@ -11263,6 +11263,8 @@ "running": "En cours", "schedule": "Planifier", "scheduled_as_daily_cron_job": "Planifié comme tâche cron quotidienne", + "spec_declares_auto_approval": "La spécification déclare approval: auto", + "spec_declares_auto_approval_hint": "Cette spécification déclare `approval: auto`. Ce n'est qu'une déclaration — les appels d'outils sont toujours approuvés un par un, sauf si vous cochez la case vous-même.", "task_runner": "Task Runner", "upload_a_file": "Importer un fichier", "workspace": "Espace de travail :", diff --git a/website/src/i18n/locales/hi.json b/website/src/i18n/locales/hi.json index 0c876087ee8..a31d3ddbced 100644 --- a/website/src/i18n/locales/hi.json +++ b/website/src/i18n/locales/hi.json @@ -11104,6 +11104,8 @@ "running": "चल रहा है", "schedule": "शेड्यूल", "scheduled_as_daily_cron_job": "दैनिक cron जॉब के रूप में शेड्यूल किया गया", + "spec_declares_auto_approval": "स्पेक approval: auto घोषित करता है", + "spec_declares_auto_approval_hint": "यह स्पेक `approval: auto` घोषित करता है। यह केवल एक घोषणा है — जब तक तुम खुद बॉक्स पर टिक नहीं करते, टूल कॉल एक-एक करके ही स्वीकृत होते हैं।", "task_runner": "Task Runner", "upload_a_file": "फ़ाइल अपलोड करें", "workspace": "वर्कस्पेस:", diff --git a/website/src/i18n/locales/it.json b/website/src/i18n/locales/it.json index b21c867279a..4d9ddac691c 100644 --- a/website/src/i18n/locales/it.json +++ b/website/src/i18n/locales/it.json @@ -11263,6 +11263,8 @@ "running": "In esecuzione", "schedule": "Programma", "scheduled_as_daily_cron_job": "Pianificato come job cron giornaliero", + "spec_declares_auto_approval": "La specifica dichiara approval: auto", + "spec_declares_auto_approval_hint": "Questa specifica dichiara `approval: auto`. È solo una dichiarazione: le chiamate agli strumenti vengono ancora approvate una alla volta, a meno che non selezioni tu stesso la casella.", "task_runner": "Task Runner", "upload_a_file": "Carica un file", "workspace": "Workspace:", diff --git a/website/src/i18n/locales/ja.json b/website/src/i18n/locales/ja.json index 8f8a13faa98..56343324eac 100644 --- a/website/src/i18n/locales/ja.json +++ b/website/src/i18n/locales/ja.json @@ -10945,6 +10945,8 @@ "running": "実行中", "schedule": "スケジュール", "scheduled_as_daily_cron_job": "日次cronジョブとしてスケジュール済み", + "spec_declares_auto_approval": "仕様が approval: auto を宣言", + "spec_declares_auto_approval_hint": "この仕様は `approval: auto` を宣言しています。これは宣言にすぎません。自分でチェックボックスをオンにしない限り、ツール呼び出しは 1 件ずつ承認されます。", "task_runner": "Task Runner", "upload_a_file": "ファイルをアップロード", "workspace": "ワークスペース：", diff --git a/website/src/i18n/locales/ko.json b/website/src/i18n/locales/ko.json index e8ba86e6dad..58025fdac13 100644 --- a/website/src/i18n/locales/ko.json +++ b/website/src/i18n/locales/ko.json @@ -10945,6 +10945,8 @@ "running": "실행 중", "schedule": "정기 실행", "scheduled_as_daily_cron_job": "매일 실행되는 cron 작업으로 등록했습니다", + "spec_declares_auto_approval": "명세가 approval: auto를 선언함", + "spec_declares_auto_approval_hint": "이 명세는 `approval: auto`를 선언합니다. 선언일 뿐이며, 직접 확인란을 선택하지 않으면 도구 호출은 계속 하나씩 승인됩니다.", "task_runner": "Task Runner", "upload_a_file": "파일 업로드", "workspace": "워크스페이스:", diff --git a/website/src/i18n/locales/pt.json b/website/src/i18n/locales/pt.json index a92b1797eb9..39ffd7a54f4 100644 --- a/website/src/i18n/locales/pt.json +++ b/website/src/i18n/locales/pt.json @@ -11263,6 +11263,8 @@ "running": "Em execução", "schedule": "Agendar", "scheduled_as_daily_cron_job": "Agendado como job de cron diário", + "spec_declares_auto_approval": "A especificação declara approval: auto", + "spec_declares_auto_approval_hint": "Esta especificação declara `approval: auto`. É apenas uma declaração — as chamadas de ferramentas continuam a ser aprovadas uma a uma, a menos que você mesmo marque a caixa.", "task_runner": "Task Runner", "upload_a_file": "Enviar um arquivo", "workspace": "Workspace:", diff --git a/website/src/i18n/locales/ru.json b/website/src/i18n/locales/ru.json index 7ba11c34b73..bd15f48a3e6 100644 --- a/website/src/i18n/locales/ru.json +++ b/website/src/i18n/locales/ru.json @@ -11422,6 +11422,8 @@ "running": "Выполняется", "schedule": "Запланировать", "scheduled_as_daily_cron_job": "Запланировано как ежедневное cron-задание", + "spec_declares_auto_approval": "Спецификация объявляет approval: auto", + "spec_declares_auto_approval_hint": "Эта спецификация объявляет `approval: auto`. Это лишь объявление — вызовы инструментов по-прежнему утверждаются по одному, если вы сами не установите флажок.", "task_runner": "Task Runner", "upload_a_file": "Загрузить файл", "workspace": "Рабочая область:", diff --git a/website/src/i18n/locales/zh-CN.json b/website/src/i18n/locales/zh-CN.json index 9bcf4d1b3f2..739bd32dccc 100644 --- a/website/src/i18n/locales/zh-CN.json +++ b/website/src/i18n/locales/zh-CN.json @@ -10945,6 +10945,8 @@ "running": "运行中", "schedule": "计划", "scheduled_as_daily_cron_job": "已排期为每日定时任务", + "spec_declares_auto_approval": "规范声明 approval: auto", + "spec_declares_auto_approval_hint": "此规范声明了 `approval: auto`。这只是一个声明——除非你自己勾选该复选框，工具调用仍会逐个请求批准。", "task_runner": "Task Runner", "upload_a_file": "上传文件", "workspace": "工作区：", diff --git a/website/src/pages/ProjectsPage.tsx b/website/src/pages/ProjectsPage.tsx index cd3b8861ede..14de0ba6f7a 100644 --- a/website/src/pages/ProjectsPage.tsx +++ b/website/src/pages/ProjectsPage.tsx @@ -87,6 +87,16 @@ export default function ProjectsPage() { // ref captured at click time (see `pendingAutoApproveRef`) so the sync effect // cannot clobber it, and it never leaks across runs. const [composeAutoApprove, setComposeAutoApprove] = useState(false) + // What the planned run's spec DECLARED (`approval: auto` in its frontmatter), + // as reported by /plan. Read-only: it labels the checkbox below and never sets + // it — the server derives the grant from `auto_approve` alone, so pre-checking + // here would only re-open the content-derived grant in the browser. + // + // Keyed by task_id so a stale declaration cannot describe a different run. + // Scope is this page's session: /plan is the only carrier of the field, so + // selecting an already-planned run after a reload shows no label — a missing + // hint, never a wrong one. + const [declaredApproval, setDeclaredApproval] = useState<{ taskId: string; declared: boolean } | null>(null) const [refineStatus, setRefineStatus] = useState('idle') const [refineError, setRefineError] = useState('') const mountedRef = useRef(true) @@ -250,6 +260,9 @@ export default function ProjectsPage() { try { const r = await api.planTask(input, source, spec, agent, workspaceDir) if (r.ok) { + // Record the spec's declaration for the label beside the Execute + // checkbox. `=== true` because the field is absent on an older gateway. + setDeclaredApproval(r.task_id ? { taskId: r.task_id, declared: r.declared_approval === true } : null) if (autoRun && r.task_id) { autoRunRef.current = r.task_id // Guard the ref-write with the SAME planTask-success branch as @@ -588,6 +601,11 @@ export default function ProjectsPage() { {selectedRun.status === 'planning' ? <> {i18nT('pages.projectsPage.planning')} : selectedRun.running ? <> {i18nT('pages.projectsPage.running')} : selectedRun.status}
{selectedRun.status === 'planned' && <> + {declaredApproval?.taskId === selectedRun.task_id && declaredApproval.declared && ( + + {i18nT('pages.projectsPage.spec_declares_auto_approval')} + + )}