From e2039c037cc9f9cf74c7552164ef7dad0a7b514c Mon Sep 17 00:00:00 2001 From: TECK KEAT WILSON Date: Thu, 10 Sep 2026 10:12:26 +0800 Subject: [PATCH 1/7] feat(ui): make skill-invocation ASCII art default-off (#145) Gate SkillInvocationArt behind a new [ui].skill_invocation_art_enabled config flag (default false), so the decorative picture costs nothing unless a project explicitly opts in. Uses the existing unset-sentinel STATE pattern (see SimpleModeGate) to resolve the flag once per session without conflating "not yet resolved" with "resolved false". Signed-off-by: TECK KEAT WILSON --- schemas/core-config.schema.json | 15 +++++++++++++++ skills/studio/modules/ui/skill-invocation-art.md | 6 +++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/schemas/core-config.schema.json b/schemas/core-config.schema.json index 2fff0f6ec..fd8ef8fbc 100644 --- a/schemas/core-config.schema.json +++ b/schemas/core-config.schema.json @@ -54,6 +54,10 @@ "paths": { "$ref": "#/$defs/paths", "description": "Optional path overrides for non-standard layouts." + }, + "ui": { + "$ref": "#/$defs/ui", + "description": "Optional UI/output preferences for interactive skill and workflow runs." } }, "$defs": { @@ -199,6 +203,17 @@ "description": "Relative path to the studio core directory. Used when the core is not at the default location." } } + }, + "ui": { + "type": "object", + "additionalProperties": false, + "properties": { + "skill_invocation_art_enabled": { + "type": "boolean", + "default": false, + "description": "Whether to draw the decorative ASCII-art picture (plus label) at cf/cf-* skill and workflow entry. Default-off: the picture costs nothing unless explicitly enabled." + } + } } } } diff --git a/skills/studio/modules/ui/skill-invocation-art.md b/skills/studio/modules/ui/skill-invocation-art.md index 83f3473ed..26faba8e9 100644 --- a/skills/studio/modules/ui/skill-invocation-art.md +++ b/skills/studio/modules/ui/skill-invocation-art.md @@ -10,13 +10,17 @@ purpose: Defines SkillInvocationArt — the ASCII-art entry picture rendered at ```pdsl UNIT SkillInvocationArt PURPOSE: Prefix each cf, cf-studio, or cf-* skill entry with one small ASCII-art picture relevant to the skill name, with a plain-text label below, without changing the workflow's control flow. +STATE: + SET SKILL_INVOCATION_ART_ENABLED: true | false | unset (default unset, scope session) WHEN: REQUIRE a cf, cf-studio, or cf-* skill or workflow entry is beginning execution SKIP silently (no picture, no output) WHEN this unit is loaded in a context that does not satisfy the above REQUIRE DO: - RUN SkillInvocationArtGuard + RUN resolve SKILL_INVOCATION_ART_ENABLED from `[ui].skill_invocation_art_enabled` in `{cf-studio-path}/config/core.toml` (false when the key or file is absent) WHEN SKILL_INVOCATION_ART_ENABLED == unset + RUN SkillInvocationArtGuard WHEN SKILL_INVOCATION_ART_ENABLED == true RUN SkillInvocationArtGenerate WHEN SkillInvocationArtGuard passes RULES: + ALWAYS default to disabled and resolve the config flag at most once per session: skip the picture with no further state read unless `[ui].skill_invocation_art_enabled` is explicitly `true` in `{cf-studio-path}/config/core.toml` ALWAYS run this unit once at the start of every cf, cf-studio, or cf-* workflow bootstrap or alias entry, before the workflow's first normal EMIT, EMIT_MENU, WAIT, CONTINUE, INVOKE, DISPATCH, RETURN, or STOP_TURN NEVER alter, delay, or suppress any existing output directive; the picture precedes but does not replace or reorder normal output NEVER replace, delay, reorder, suppress, or alter any load report From 8d071e9b6027d17ce48183de265be21ae416e552 Mon Sep 17 00:00:00 2001 From: TECK KEAT WILSON Date: Thu, 10 Sep 2026 10:12:26 +0800 Subject: [PATCH 2/7] docs(ui): align skill-invocation-art PURPOSE and schema wording with actual gating (#145) Address review nits on PR #166: the PURPOSE line still described the picture as unconditional, and the schema described the flag as "interactive"-scoped even though no such distinction exists in the PDSL gate. Both now match actual behavior. Signed-off-by: TECK KEAT WILSON --- schemas/core-config.schema.json | 2 +- skills/studio/modules/ui/skill-invocation-art.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/schemas/core-config.schema.json b/schemas/core-config.schema.json index fd8ef8fbc..f66467e53 100644 --- a/schemas/core-config.schema.json +++ b/schemas/core-config.schema.json @@ -57,7 +57,7 @@ }, "ui": { "$ref": "#/$defs/ui", - "description": "Optional UI/output preferences for interactive skill and workflow runs." + "description": "Optional UI/output preferences for skill and workflow runs." } }, "$defs": { diff --git a/skills/studio/modules/ui/skill-invocation-art.md b/skills/studio/modules/ui/skill-invocation-art.md index 26faba8e9..79d7f4250 100644 --- a/skills/studio/modules/ui/skill-invocation-art.md +++ b/skills/studio/modules/ui/skill-invocation-art.md @@ -9,7 +9,7 @@ purpose: Defines SkillInvocationArt — the ASCII-art entry picture rendered at ```pdsl UNIT SkillInvocationArt -PURPOSE: Prefix each cf, cf-studio, or cf-* skill entry with one small ASCII-art picture relevant to the skill name, with a plain-text label below, without changing the workflow's control flow. +PURPOSE: When enabled via `[ui].skill_invocation_art_enabled` in `{cf-studio-path}/config/core.toml`, prefix each cf, cf-studio, or cf-* skill entry with one small ASCII-art picture relevant to the skill name, with a plain-text label below, without changing the workflow's control flow. STATE: SET SKILL_INVOCATION_ART_ENABLED: true | false | unset (default unset, scope session) WHEN: From 617c4d38fb91fe5b8f8e77e20bc3183c7970754e Mon Sep 17 00:00:00 2001 From: TECK KEAT WILSON Date: Thu, 10 Sep 2026 11:00:47 +0800 Subject: [PATCH 3/7] docs(ui): scope skill-invocation-art PURPOSE to workflow entries too (#145) CodeRabbit caught that the PURPOSE line said "skill entry" while WHEN/RULES (and the schema) already cover workflow entries too. Signed-off-by: TECK KEAT WILSON --- skills/studio/modules/ui/skill-invocation-art.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/studio/modules/ui/skill-invocation-art.md b/skills/studio/modules/ui/skill-invocation-art.md index 79d7f4250..0b3b106e9 100644 --- a/skills/studio/modules/ui/skill-invocation-art.md +++ b/skills/studio/modules/ui/skill-invocation-art.md @@ -9,7 +9,7 @@ purpose: Defines SkillInvocationArt — the ASCII-art entry picture rendered at ```pdsl UNIT SkillInvocationArt -PURPOSE: When enabled via `[ui].skill_invocation_art_enabled` in `{cf-studio-path}/config/core.toml`, prefix each cf, cf-studio, or cf-* skill entry with one small ASCII-art picture relevant to the skill name, with a plain-text label below, without changing the workflow's control flow. +PURPOSE: When enabled via `[ui].skill_invocation_art_enabled` in `{cf-studio-path}/config/core.toml`, prefix each cf, cf-studio, or cf-* skill or workflow entry with one small ASCII-art picture relevant to the entry name, with a plain-text label below, without changing the workflow's control flow. STATE: SET SKILL_INVOCATION_ART_ENABLED: true | false | unset (default unset, scope session) WHEN: From fb7b6ae72f597d871f87f3e7b0b43225351ce341 Mon Sep 17 00:00:00 2001 From: TECK KEAT WILSON Date: Fri, 11 Sep 2026 14:12:28 +0800 Subject: [PATCH 4/7] feat(pdsl): route blocking EMIT_MENU gates through a harness's native question dialog Studio's blocking menus render as prose, indistinguishable from a finished turn. Adds a per-target ask-tool binding (Claude -> AskUserQuestion) threaded through generated shims via _follow_protocol_lines, and a shape-fit-aware routing rule in pdsl-execution-card.md: native dialog when bound and the menu fits (<=4 fixed options), description-based fallback otherwise, text fallback when the menu can't be represented as fixed choices. Covers all existing EMIT_MENU sites without touching any of them. Refs #142 Signed-off-by: TECK KEAT WILSON --- .../modules/runtime/pdsl-execution-card.md | 21 +++++ .../modules/runtime/required-bootstrap.md | 1 + .../studio/scripts/studio/commands/agents.py | 68 ++++++++++++---- tests/test_subagent_registration.py | 79 +++++++++++++++++++ tests/test_workflow_subagents_dispatch.py | 20 +++++ 5 files changed, 172 insertions(+), 17 deletions(-) diff --git a/skills/studio/modules/runtime/pdsl-execution-card.md b/skills/studio/modules/runtime/pdsl-execution-card.md index da4e3513a..b4686685d 100644 --- a/skills/studio/modules/runtime/pdsl-execution-card.md +++ b/skills/studio/modules/runtime/pdsl-execution-card.md @@ -40,6 +40,27 @@ RULES: handles all unmatched input. ALWAYS require every top-level `OPTIONS` entry to start with a decimal number; aliases or patterns follow the number, not replace it. + ALWAYS, when executing `EMIT_MENU`, first check the menu is native-dialog + shape-compatible: at most 4 top-level `OPTIONS` entries, and no entry + documented as accepting free-text/arbitrary input (a path, a name, "or + describe your own", etc.) rather than choosing among the listed entries. + ALWAYS, for a shape-compatible `EMIT_MENU` where the active `ask_tool_name` + context is a real tool name (not `unset`), invoke that tool instead of + rendering the menu as prose: pass `TITLE` as the question/header text and + each numbered `OPTIONS` entry as one selectable option (its short label as + the option label, its action clause as the option description). + ALWAYS, for a shape-compatible `EMIT_MENU` where `ask_tool_name` is `unset`, + still surface the menu so a harness exposing an equivalent affordance it + recognizes by `ask_tool_description` can match it: state the question, + list the numbered options, and mark it explicitly as a blocking question + the assistant is waiting on — placed as the last content in the turn. + ALWAYS, for a shape-incompatible `EMIT_MENU` (more than 4 options, or any + free-text-accepting entry), render as today's text menu regardless of + `ask_tool_name` — a native dialog's fixed-choice shape cannot represent it + faithfully — but still place it last in the turn and mark it blocking. + NEVER treat a harness with no matching native affordance as an error; + fall back to the same explicitly-marked, end-of-turn text rendering used + for shape-incompatible menus. ALWAYS treat `ON_ERROR` as the named recovery path for matching failures. ALWAYS treat `NOTES` as explanatory only; NOTES do not create executable obligations unless an active rule references them. diff --git a/skills/studio/modules/runtime/required-bootstrap.md b/skills/studio/modules/runtime/required-bootstrap.md index b05cc163b..1b5cdc4a4 100644 --- a/skills/studio/modules/runtime/required-bootstrap.md +++ b/skills/studio/modules/runtime/required-bootstrap.md @@ -32,6 +32,7 @@ RULES: ALWAYS keep template-vars and context-memory loaded so downstream protocols can resolve variables and classify remembered context deterministically ALWAYS activate ContentMemory so downstream content payloads inherit the runtime lifecycle rules from bootstrap ALWAYS activate ResourceContextMemory so downstream workflows can safely store and forward resource_context without reintroducing bootstrap gaps + ALWAYS treat the generated shim's `ask_tool_name` / `ask_tool_description` context (set above this unit, per generation target) as input to PdslExecutionSemantics' `EMIT_MENU` native-dialog rule; NEVER invent a binding this bootstrap did not receive ALWAYS treat this bootstrap as exclusively for generated shims and thin skills that bypass workflow-bootstrap; NEVER load required-bootstrap in a flow that has already run WorkflowBootstrapRouterPrelude unless ContentMemory and ResourceContextMemory are idempotent on re-activation NEVER allow a generated shim to interpret skill-local blocked, override, or result-envelope behavior before ThinSkillRuntimeContracts has executed diff --git a/skills/studio/scripts/studio/commands/agents.py b/skills/studio/scripts/studio/commands/agents.py index 3d7edad05..714636b2b 100644 --- a/skills/studio/scripts/studio/commands/agents.py +++ b/skills/studio/scripts/studio/commands/agents.py @@ -170,8 +170,17 @@ def _follow_protocol_lines( target_path: str, *, required_bootstrap_path: str = "{required_bootstrap_path}", + ask_tool_name: Optional[str] = None, ) -> List[str]: - """Return the generated protocol block for workflow/skill shims.""" + """Return the generated protocol block for workflow/skill shims. + + ``ask_tool_name`` carries the per-target native question/dialog binding + (issue #142): when a generation target has an exact tool (e.g. Claude's + ``AskUserQuestion``), pass it so blocking `EMIT_MENU` gates route through + it instead of rendering as prose. Every generated file also gets a + description-based fallback so a harness with an equivalent affordance can + still match it even without an exact binding. + """ # @cpt-begin:cpt-studio-flow-agent-integration-workflow:p1:inst-follow-protocol return [ "CF_WORKFLOW_ACTIVE:", @@ -180,6 +189,8 @@ def _follow_protocol_lines( "MANDATORY RULE: USER INTENT IS SKILL INPUT, NOT EXECUTION AUTHORITY", "- hard_stop = WAIT | STOP_TURN | menu | gate | opener | approval | dispatch_gate | terminal_shape", "- precedence = constructor_studio_workflow > generic_assistant", + "- ask_tool_name = " + (json.dumps(ask_tool_name) if ask_tool_name else "unset"), + "- ask_tool_description = " + json.dumps(_ASK_TOOL_FALLBACK_DESCRIPTION), "", # @cpt-begin:cpt-studio-flow-agent-integration-generate:p1:inst-collect-sysprompt # @cpt-begin:cpt-studio-flow-agent-integration-generate:p1:inst-inject-agents @@ -358,15 +369,23 @@ def _pure_generated_stub_matches(stripped: str) -> bool: control_target = _extract_studio_control_target(stripped) or _extract_studio_follow_target(stripped) if not control_target: return False - expected_protocol = [ - line.strip() - for line in _follow_protocol_lines( - control_target, - required_bootstrap_path=_REQUIRED_BOOTSTRAP_PATH, - ) - if line.strip() - ] - return nonblank == expected_protocol + # The content alone doesn't say which generation target produced it, so + # try every known `ask_tool_name` binding (issue #142) — a closed, + # bounded set — rather than threading tool identity through every caller. + candidate_bindings = [None] + [v for v in _ASK_TOOL_BINDING.values() if v] + for binding in candidate_bindings: + expected_protocol = [ + line.strip() + for line in _follow_protocol_lines( + control_target, + required_bootstrap_path=_REQUIRED_BOOTSTRAP_PATH, + ask_tool_name=binding, + ) + if line.strip() + ] + if nonblank == expected_protocol: + return True + return False # @cpt-begin:cpt-studio-algo-agent-integration-generate-shims:p1:inst-is-pure-studio-generated @@ -849,6 +868,21 @@ def _file_has_studio_follow_link(path: Path) -> bool: } # @cpt-end:cpt-studio-algo-agent-integration-generate-shims:p1:inst-auto-value-map +# Per-target binding for a blocking `EMIT_MENU` gate's native question/dialog +# affordance (issue #142). Only targets Studio generates a dedicated, +# single-tool file for can carry an exact tool name — everyone else shares one +# byte-identical file across multiple tools (see `_agents_skill_outputs()`) +# and can only rely on the description-based fallback below. +_ASK_TOOL_BINDING: Dict[str, Optional[str]] = { + "claude": "AskUserQuestion", +} +_CLAUDE_ASK_TOOL_NAME = _ASK_TOOL_BINDING.get("claude") + +_ASK_TOOL_FALLBACK_DESCRIPTION = ( + "a tool that presents the user a blocking multiple-choice question with " + "selectable options, distinct from plain text output" +) + # @cpt-begin:cpt-studio-algo-agent-integration-generate-shims:p1:inst-resolve-model-id def _resolve_model_id( @@ -1858,7 +1892,7 @@ def _default_agents_config() -> dict: _GENERATED_MARKER, "", "{custom_content}", - *_follow_protocol_lines("{target_skill_path}"), + *_follow_protocol_lines("{target_skill_path}", ask_tool_name=_CLAUDE_ASK_TOOL_NAME), ], }, { @@ -1874,7 +1908,7 @@ def _default_agents_config() -> dict: "---", _GENERATED_MARKER, "", - *_follow_protocol_lines("{target_path}"), + *_follow_protocol_lines("{target_path}", ask_tool_name=_CLAUDE_ASK_TOOL_NAME), ], }, { @@ -1890,7 +1924,7 @@ def _default_agents_config() -> dict: "---", _GENERATED_MARKER, "", - *_follow_protocol_lines("{target_path}"), + *_follow_protocol_lines("{target_path}", ask_tool_name=_CLAUDE_ASK_TOOL_NAME), ], }, { @@ -1906,7 +1940,7 @@ def _default_agents_config() -> dict: "---", _GENERATED_MARKER, "", - *_follow_protocol_lines("{target_path}"), + *_follow_protocol_lines("{target_path}", ask_tool_name=_CLAUDE_ASK_TOOL_NAME), ], }, { @@ -1922,7 +1956,7 @@ def _default_agents_config() -> dict: "---", _GENERATED_MARKER, "", - *_follow_protocol_lines("{target_path}"), + *_follow_protocol_lines("{target_path}", ask_tool_name=_CLAUDE_ASK_TOOL_NAME), ], }, { @@ -1938,7 +1972,7 @@ def _default_agents_config() -> dict: "---", _GENERATED_MARKER, "", - *_follow_protocol_lines("{target_path}"), + *_follow_protocol_lines("{target_path}", ask_tool_name=_CLAUDE_ASK_TOOL_NAME), ], }, ], @@ -2670,7 +2704,7 @@ def _path_within_any_root(path: Path, roots: Tuple[Path, ...]) -> bool: "---", _GENERATED_MARKER, "", - *_follow_protocol_lines("{target_path}"), + *_follow_protocol_lines("{target_path}", ask_tool_name=_CLAUDE_ASK_TOOL_NAME), ], "openai": _AGENTS_KIT_WORKFLOW_TEMPLATE, "windsurf": _AGENTS_KIT_WORKFLOW_TEMPLATE, diff --git a/tests/test_subagent_registration.py b/tests/test_subagent_registration.py index 7ef3f5edb..e71d07985 100644 --- a/tests/test_subagent_registration.py +++ b/tests/test_subagent_registration.py @@ -1195,6 +1195,85 @@ def test_generated_follow_link_protocol_requires_workflow_execution(self): self.assertIn("ALWAYS stop if any required fragment or rule cannot be followed", joined) self.assertTrue(_is_pure_studio_generated(content, expected_name="cf-analyze")) + def test_ask_tool_binding_defaults_to_unset_with_description_fallback(self): + """Issue #142: every generated shim carries an ask-tool context, even + when no target passes an explicit binding — the description-based + fallback must always be present so a harness can match by intent.""" + from studio.commands.agents import ( + _ASK_TOOL_FALLBACK_DESCRIPTION, + _REQUIRED_BOOTSTRAP_PATH, + _follow_protocol_lines, + _is_pure_studio_generated, + ) + + block = _follow_protocol_lines( + "{cf-studio-path}/.core/workflows/analyze.md", + required_bootstrap_path=_REQUIRED_BOOTSTRAP_PATH, + ) + self.assertIn("- ask_tool_name = unset", block) + self.assertIn( + f"- ask_tool_description = {json.dumps(_ASK_TOOL_FALLBACK_DESCRIPTION)}", + block, + ) + content = ( + "---\nname: cf-analyze\ndescription: analyze\n---\n" + "\n\n" + + "\n".join(block) + + "\n" + ) + self.assertTrue(_is_pure_studio_generated(content, expected_name="cf-analyze")) + + def test_ask_tool_binding_carries_exact_tool_name_for_claude(self): + """Issue #142: Claude gets its own dedicated generated files, so it can + carry an exact native-dialog tool binding (`AskUserQuestion`).""" + from studio.commands.agents import ( + _ASK_TOOL_BINDING, + _REQUIRED_BOOTSTRAP_PATH, + _follow_protocol_lines, + _is_pure_studio_generated, + ) + + block = _follow_protocol_lines( + "{cf-studio-path}/.core/workflows/analyze.md", + required_bootstrap_path=_REQUIRED_BOOTSTRAP_PATH, + ask_tool_name=_ASK_TOOL_BINDING["claude"], + ) + self.assertIn('- ask_tool_name = "AskUserQuestion"', block) + content = ( + "---\nname: cf-analyze\ndescription: analyze\n---\n" + "\n\n" + + "\n".join(block) + + "\n" + ) + self.assertTrue(_is_pure_studio_generated(content, expected_name="cf-analyze")) + + def test_claude_skill_outputs_bind_ask_user_question(self): + """Every Claude-specific generated skill template (the only per-tool + bucket that can name an exact tool) must request AskUserQuestion.""" + from studio.commands.agents import _default_agents_config + + config = _default_agents_config() + outputs = config["agents"]["claude"]["skills"]["outputs"] + self.assertTrue(outputs) + for entry in outputs: + template = "\n".join(entry["template"]) + self.assertIn( + '- ask_tool_name = "AskUserQuestion"', + template, + msg=f"missing ask_tool_name binding in {entry['path']}", + ) + + def test_shared_skill_outputs_have_no_exact_ask_tool_binding(self): + """The shared `.agents/skills/` bucket is byte-identical across + windsurf/cursor/copilot/codex, so it must never bake in an exact + tool name — only the description-based fallback.""" + from studio.commands.agents import _agents_skill_outputs + + for entry in _agents_skill_outputs(): + template = "\n".join(entry["template"]) + self.assertIn("- ask_tool_name = unset", template) + self.assertNotIn("AskUserQuestion", template) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_workflow_subagents_dispatch.py b/tests/test_workflow_subagents_dispatch.py index 394fdb1d4..7cb0ffda2 100644 --- a/tests/test_workflow_subagents_dispatch.py +++ b/tests/test_workflow_subagents_dispatch.py @@ -2642,6 +2642,26 @@ def test_required_bootstrap_activates_content_and_resource_context_memory() -> N assert "RUN ResourceContextMemory" in required_bootstrap +def test_pdsl_execution_card_routes_emit_menu_through_native_ask_tool() -> None: + """Issue #142: blocking EMIT_MENU gates route through a harness's native + question dialog when the generated shim's ask_tool_name binding and the + menu's shape (<=4 fixed-choice options) allow it, else fall back to text.""" + repo_root = Path(__file__).resolve().parents[1] + execution_card = ( + repo_root / "skills" / "studio" / "modules" / "runtime" / "pdsl-execution-card.md" + ).read_text(encoding="utf-8") + normalized = " ".join(execution_card.split()) + + assert "native-dialog shape-compatible" in normalized + assert "at most 4 top-level `OPTIONS` entries" in normalized + assert "no entry documented as accepting free-text" in normalized + assert "`ask_tool_name` context is a real tool name (not `unset`)" in normalized + assert "invoke that tool instead of rendering the menu as prose" in normalized + assert "`ask_tool_description` can match it" in normalized + assert "shape-incompatible `EMIT_MENU`" in normalized + assert "NEVER treat a harness with no matching native affordance as an error" in normalized + + def test_studio_instruction_memory_runs_in_concrete_workflows() -> None: """Concrete workflows load generated/project Studio instructions before work.""" repo_root = Path(__file__).resolve().parents[1] From a60766b59ba4a15437d51aeced081791f3d33b5f Mon Sep 17 00:00:00 2001 From: TECK KEAT WILSON Date: Fri, 11 Sep 2026 15:01:22 +0800 Subject: [PATCH 5/7] fix(pdsl): close review-found gaps in native-dialog EMIT_MENU routing Addresses review feedback on the #142 native-dialog change: - Pre-upgrade generated shims (no ask_tool_name/ask_tool_description lines at all) are recognized as pure generated stubs again, so legacy-cleanup and auto-regeneration keep working across upgrades. - A never-bound ask_tool_name (e.g. workflows entered via WorkflowBootstrapRouterPrelude, which never loads required-bootstrap.md) is now treated identically to an explicit `unset`, closing an undefined third state in the routing rule. - The routing rule now states explicitly it applies to EMIT_MENU paired with WAIT/STOP_TURN, that a native invocation is itself that turn boundary, and that an out-of-band answer/cancellation/dismissal/error from the native tool routes to the menu's own INVALID handler instead of silently advancing past the gate. - Added a regression test tying required-bootstrap.md's ask-tool handoff rule to the exact variable names agents.py emits. Refs #142 Signed-off-by: TECK KEAT WILSON --- .../modules/runtime/pdsl-execution-card.md | 35 +++++++++++----- .../studio/scripts/studio/commands/agents.py | 17 ++++++++ tests/test_subagent_registration.py | 25 ++++++++++++ tests/test_workflow_subagents_dispatch.py | 40 ++++++++++++++++++- 4 files changed, 107 insertions(+), 10 deletions(-) diff --git a/skills/studio/modules/runtime/pdsl-execution-card.md b/skills/studio/modules/runtime/pdsl-execution-card.md index b4686685d..e9a55a56d 100644 --- a/skills/studio/modules/runtime/pdsl-execution-card.md +++ b/skills/studio/modules/runtime/pdsl-execution-card.md @@ -40,20 +40,37 @@ RULES: handles all unmatched input. ALWAYS require every top-level `OPTIONS` entry to start with a decimal number; aliases or patterns follow the number, not replace it. + ALWAYS treat every `EMIT_MENU` in this corpus as a blocking gate paired with + `WAIT`/`STOP_TURN` in the same `DO` block; the native-dialog routing below + applies to that pairing, not to text output that merely lists choices + without waiting on a reply. ALWAYS, when executing `EMIT_MENU`, first check the menu is native-dialog shape-compatible: at most 4 top-level `OPTIONS` entries, and no entry documented as accepting free-text/arbitrary input (a path, a name, "or describe your own", etc.) rather than choosing among the listed entries. + ALWAYS treat an `ask_tool_name` context that was never established (no + generated shim or dispatch prompt set it at all) identically to `unset`; + the distinction between "explicitly no binding" and "never bound" carries + no different behavior. ALWAYS, for a shape-compatible `EMIT_MENU` where the active `ask_tool_name` - context is a real tool name (not `unset`), invoke that tool instead of - rendering the menu as prose: pass `TITLE` as the question/header text and - each numbered `OPTIONS` entry as one selectable option (its short label as - the option label, its action clause as the option description). - ALWAYS, for a shape-compatible `EMIT_MENU` where `ask_tool_name` is `unset`, - still surface the menu so a harness exposing an equivalent affordance it - recognizes by `ask_tool_description` can match it: state the question, - list the numbered options, and mark it explicitly as a blocking question - the assistant is waiting on — placed as the last content in the turn. + context is a real tool name (not `unset` or never established), invoke + that tool instead of rendering the menu as prose: pass `TITLE` as the + question/header text and each numbered `OPTIONS` entry as one selectable + option (its short label as the option label, its action clause as the + option description); that invocation is itself the turn's `WAIT`/ + `STOP_TURN` boundary — NEVER additionally re-render the menu as text or + execute a redundant `STOP_TURN` after it. + ALWAYS treat a native-tool result that selects none of the numbered + `OPTIONS` — an out-of-band/free-text answer, a cancellation, a dismissal, + or a tool error — as unmatched input for the menu's own `INVALID` handler; + NEVER treat any such outcome as silently choosing a default option or + advancing past the gate. + ALWAYS, for a shape-compatible `EMIT_MENU` where `ask_tool_name` is `unset` + or never established, still surface the menu so a harness exposing an + equivalent affordance it recognizes by `ask_tool_description` can match + it: state the question, list the numbered options, and mark it explicitly + as a blocking question the assistant is waiting on — placed as the last + content in the turn. ALWAYS, for a shape-incompatible `EMIT_MENU` (more than 4 options, or any free-text-accepting entry), render as today's text menu regardless of `ask_tool_name` — a native dialog's fixed-choice shape cannot represent it diff --git a/skills/studio/scripts/studio/commands/agents.py b/skills/studio/scripts/studio/commands/agents.py index 714636b2b..ce3d2debf 100644 --- a/skills/studio/scripts/studio/commands/agents.py +++ b/skills/studio/scripts/studio/commands/agents.py @@ -369,6 +369,23 @@ def _pure_generated_stub_matches(stripped: str) -> bool: control_target = _extract_studio_control_target(stripped) or _extract_studio_follow_target(stripped) if not control_target: return False + # A file generated before this repo's `ask_tool_name`/`ask_tool_description` + # context existed (issue #142) carries neither line at all — not "unset", + # simply absent. Without this candidate, every pre-existing generated file + # would stop matching the moment this context was introduced, silently + # breaking legacy-cleanup/regeneration for every prior install. + legacy_protocol = [ + line.strip() + for line in _follow_protocol_lines( + control_target, + required_bootstrap_path=_REQUIRED_BOOTSTRAP_PATH, + ) + if line.strip() + and not line.startswith("- ask_tool_name") + and not line.startswith("- ask_tool_description") + ] + if nonblank == legacy_protocol: + return True # The content alone doesn't say which generation target produced it, so # try every known `ask_tool_name` binding (issue #142) — a closed, # bounded set — rather than threading tool identity through every caller. diff --git a/tests/test_subagent_registration.py b/tests/test_subagent_registration.py index e71d07985..35dd7cb03 100644 --- a/tests/test_subagent_registration.py +++ b/tests/test_subagent_registration.py @@ -1195,6 +1195,31 @@ def test_generated_follow_link_protocol_requires_workflow_execution(self): self.assertIn("ALWAYS stop if any required fragment or rule cannot be followed", joined) self.assertTrue(_is_pure_studio_generated(content, expected_name="cf-analyze")) + def test_pre_142_generated_files_still_recognized_as_pure_after_upgrade(self): + """A file Studio generated before ask_tool_name/ask_tool_description + existed carries neither line at all (not "unset" -- absent). It must + still be recognized as a pure, untouched Studio stub after upgrading, + or legacy-cleanup/regeneration silently stops touching every + pre-existing install's generated files.""" + from studio.commands.agents import ( + _REQUIRED_BOOTSTRAP_PATH, + _follow_protocol_lines, + _pure_generated_stub_matches, + ) + + target = "{cf-studio-path}/.core/workflows/analyze.md" + pre_142_lines = [ + line + for line in _follow_protocol_lines(target, required_bootstrap_path=_REQUIRED_BOOTSTRAP_PATH) + if line.strip() + and not line.startswith("- ask_tool_name") + and not line.startswith("- ask_tool_description") + ] + pre_142_body = "\n".join(pre_142_lines) + + self.assertTrue(_pure_generated_stub_matches(pre_142_body)) + self.assertFalse(_pure_generated_stub_matches(pre_142_body + "\nCUSTOM USER LINE")) + def test_ask_tool_binding_defaults_to_unset_with_description_fallback(self): """Issue #142: every generated shim carries an ask-tool context, even when no target passes an explicit binding — the description-based diff --git a/tests/test_workflow_subagents_dispatch.py b/tests/test_workflow_subagents_dispatch.py index 7cb0ffda2..a1486397d 100644 --- a/tests/test_workflow_subagents_dispatch.py +++ b/tests/test_workflow_subagents_dispatch.py @@ -2655,12 +2655,50 @@ def test_pdsl_execution_card_routes_emit_menu_through_native_ask_tool() -> None: assert "native-dialog shape-compatible" in normalized assert "at most 4 top-level `OPTIONS` entries" in normalized assert "no entry documented as accepting free-text" in normalized - assert "`ask_tool_name` context is a real tool name (not `unset`)" in normalized + assert "`ask_tool_name` context is a real tool name (not `unset` or never established)" in normalized assert "invoke that tool instead of rendering the menu as prose" in normalized assert "`ask_tool_description` can match it" in normalized assert "shape-incompatible `EMIT_MENU`" in normalized assert "NEVER treat a harness with no matching native affordance as an error" in normalized + # Reviewer-identified gaps (issue #142 follow-up): never-bound treated as + # unset, out-of-band/cancelled native answers routed to INVALID, and + # native invocation itself satisfying the turn's WAIT/STOP_TURN boundary. + assert "never established (no" in normalized + assert "identically to `unset`" in normalized + assert "cancellation, a dismissal, or a tool error" in normalized + assert "unmatched input for the menu's own `INVALID` handler" in normalized + assert "that invocation is itself the turn's" in normalized + assert "boundary" in normalized + assert "blocking gate paired with" in normalized + + +def test_required_bootstrap_ask_tool_handoff_names_match_the_generated_lines() -> None: + """Issue #142 follow-up: the bridging rule in required-bootstrap.md must + name the exact variables agents.py actually emits, so a rename/removal on + either side is caught instead of two independently-passing test halves.""" + repo_root = Path(__file__).resolve().parents[1] + required_bootstrap = ( + repo_root / "skills" / "studio" / "modules" / "runtime" / "required-bootstrap.md" + ).read_text(encoding="utf-8") + + assert "`ask_tool_name`" in required_bootstrap + assert "`ask_tool_description`" in required_bootstrap + assert "PdslExecutionSemantics" in required_bootstrap + assert "EMIT_MENU" in required_bootstrap + + from studio.commands.agents import _follow_protocol_lines, _REQUIRED_BOOTSTRAP_PATH + + generated = "\n".join( + _follow_protocol_lines( + "{cf-studio-path}/.core/workflows/analyze.md", + required_bootstrap_path=_REQUIRED_BOOTSTRAP_PATH, + ask_tool_name="AskUserQuestion", + ) + ) + assert "ask_tool_name = " in generated + assert "ask_tool_description = " in generated + def test_studio_instruction_memory_runs_in_concrete_workflows() -> None: """Concrete workflows load generated/project Studio instructions before work.""" From 0ef03d958876065a1dd7ff91a08e290cbe2798fb Mon Sep 17 00:00:00 2001 From: TECK KEAT WILSON Date: Fri, 11 Sep 2026 15:09:45 +0800 Subject: [PATCH 6/7] fix(pdsl): grant AskUserQuestion in allowed-tools, tighten native-dialog mapping Second round of review-driven fixes on #142: - Every Claude template that instructs `ask_tool_name = "AskUserQuestion"` now also grants AskUserQuestion in its `allowed-tools:` frontmatter line (via _CLAUDE_ASK_TOOL_NAME, not a second hardcoded literal). Without this the generated shim told the assistant to call a tool it wasn't permitted to use, making the whole feature a no-op for the one target with a real binding. - Added test coverage for _KIT_WORKFLOW_SKILL_TEMPLATES['claude'], a separate production call site the existing test didn't reach. - Tightened the native-dialog mapping rule: TITLE fills the tool's single question/header field (one EMIT_MENU per invocation, never batched), and each OPTIONS entry's number/alias is retained as its canonical identity so the returned selection resumes the right branch regardless of label rendering/truncation. - Documented which of the four shared-bucket targets are verified-compatible today (none; reserved for a future harness) vs. Claude (confirmed). Refs #142 Signed-off-by: TECK KEAT WILSON --- .../modules/runtime/pdsl-execution-card.md | 17 ++++++---- .../studio/scripts/studio/commands/agents.py | 22 ++++++++----- tests/test_subagent_registration.py | 31 ++++++++++++++++++- 3 files changed, 56 insertions(+), 14 deletions(-) diff --git a/skills/studio/modules/runtime/pdsl-execution-card.md b/skills/studio/modules/runtime/pdsl-execution-card.md index e9a55a56d..606b6421c 100644 --- a/skills/studio/modules/runtime/pdsl-execution-card.md +++ b/skills/studio/modules/runtime/pdsl-execution-card.md @@ -54,12 +54,17 @@ RULES: no different behavior. ALWAYS, for a shape-compatible `EMIT_MENU` where the active `ask_tool_name` context is a real tool name (not `unset` or never established), invoke - that tool instead of rendering the menu as prose: pass `TITLE` as the - question/header text and each numbered `OPTIONS` entry as one selectable - option (its short label as the option label, its action clause as the - option description); that invocation is itself the turn's `WAIT`/ - `STOP_TURN` boundary — NEVER additionally re-render the menu as text or - execute a redundant `STOP_TURN` after it. + that tool instead of rendering the menu as prose, mapped onto that tool's + own question/options schema: `TITLE` fills its single question/header + field (there is one `EMIT_MENU` per invocation, never a multi-question + batch), and each numbered `OPTIONS` entry becomes one selectable option — + label from the entry's short form, description from its action clause — + with the entry's number/alias retained as that option's canonical, + non-displayed identity so the returned selection resumes the exact + numbered branch regardless of how the harness renders or truncates the + displayed label; that invocation is itself the turn's `WAIT`/`STOP_TURN` + boundary — NEVER additionally re-render the menu as text or execute a + redundant `STOP_TURN` after it. ALWAYS treat a native-tool result that selects none of the numbered `OPTIONS` — an out-of-band/free-text answer, a cancellation, a dismissal, or a tool error — as unmatched input for the menu's own `INVALID` handler; diff --git a/skills/studio/scripts/studio/commands/agents.py b/skills/studio/scripts/studio/commands/agents.py index ce3d2debf..8a987860b 100644 --- a/skills/studio/scripts/studio/commands/agents.py +++ b/skills/studio/scripts/studio/commands/agents.py @@ -890,6 +890,11 @@ def _file_has_studio_follow_link(path: Path) -> bool: # single-tool file for can carry an exact tool name — everyone else shares one # byte-identical file across multiple tools (see `_agents_skill_outputs()`) # and can only rely on the description-based fallback below. +# +# Verified-compatible today: Claude Code's `AskUserQuestion`, below. The +# windsurf/cursor/copilot/codex shared bucket carries only the description +# fallback — none of the four is known to expose a matching native affordance +# yet; that path is reserved for a future harness, not confirmed working now. _ASK_TOOL_BINDING: Dict[str, Optional[str]] = { "claude": "AskUserQuestion", } @@ -1904,7 +1909,10 @@ def _default_agents_config() -> dict: _TMPL_DESCRIPTION, "disable-model-invocation: false", "user-invocable: true", - "allowed-tools: Bash, Read, Write, Edit, Glob, Grep, Task, WebFetch", + ( + "allowed-tools: Bash, Read, Write, Edit, Glob, Grep, " + "Task, WebFetch, " + _CLAUDE_ASK_TOOL_NAME + ), "---", _GENERATED_MARKER, "", @@ -1921,7 +1929,7 @@ def _default_agents_config() -> dict: _TMPL_DESCRIPTION, "disable-model-invocation: false", "user-invocable: true", - "allowed-tools: Bash, Read, Write, Edit, Glob, Grep, Task", + "allowed-tools: Bash, Read, Write, Edit, Glob, Grep, Task, " + _CLAUDE_ASK_TOOL_NAME, "---", _GENERATED_MARKER, "", @@ -1937,7 +1945,7 @@ def _default_agents_config() -> dict: _TMPL_DESCRIPTION, "disable-model-invocation: false", "user-invocable: true", - "allowed-tools: Bash, Read, Glob, Grep", + "allowed-tools: Bash, Read, Glob, Grep, " + _CLAUDE_ASK_TOOL_NAME, "---", _GENERATED_MARKER, "", @@ -1953,7 +1961,7 @@ def _default_agents_config() -> dict: _TMPL_DESCRIPTION, "disable-model-invocation: false", "user-invocable: true", - "allowed-tools: Bash, Read, Write, Edit, Glob, Grep", + "allowed-tools: Bash, Read, Write, Edit, Glob, Grep, " + _CLAUDE_ASK_TOOL_NAME, "---", _GENERATED_MARKER, "", @@ -1969,7 +1977,7 @@ def _default_agents_config() -> dict: _TMPL_DESCRIPTION, "disable-model-invocation: false", "user-invocable: true", - "allowed-tools: Bash, Read, Glob, Grep", + "allowed-tools: Bash, Read, Glob, Grep, " + _CLAUDE_ASK_TOOL_NAME, "---", _GENERATED_MARKER, "", @@ -1985,7 +1993,7 @@ def _default_agents_config() -> dict: _TMPL_DESCRIPTION, "disable-model-invocation: false", "user-invocable: true", - "allowed-tools: Bash, Read, Write, Edit, Glob, Grep", + "allowed-tools: Bash, Read, Write, Edit, Glob, Grep, " + _CLAUDE_ASK_TOOL_NAME, "---", _GENERATED_MARKER, "", @@ -2717,7 +2725,7 @@ def _path_within_any_root(path: Path, roots: Tuple[Path, ...]) -> bool: _TMPL_DESCRIPTION, "disable-model-invocation: false", "user-invocable: true", - "allowed-tools: Bash, Read, Write, Edit, Glob, Grep, Task, WebFetch", + "allowed-tools: Bash, Read, Write, Edit, Glob, Grep, Task, WebFetch, " + _CLAUDE_ASK_TOOL_NAME, "---", _GENERATED_MARKER, "", diff --git a/tests/test_subagent_registration.py b/tests/test_subagent_registration.py index 35dd7cb03..b05867597 100644 --- a/tests/test_subagent_registration.py +++ b/tests/test_subagent_registration.py @@ -1274,7 +1274,9 @@ def test_ask_tool_binding_carries_exact_tool_name_for_claude(self): def test_claude_skill_outputs_bind_ask_user_question(self): """Every Claude-specific generated skill template (the only per-tool - bucket that can name an exact tool) must request AskUserQuestion.""" + bucket that can name an exact tool) must request AskUserQuestion, and + must also grant it in `allowed-tools:` -- naming a tool the shim isn't + permitted to call would make the instruction unusable.""" from studio.commands.agents import _default_agents_config config = _default_agents_config() @@ -1287,6 +1289,33 @@ def test_claude_skill_outputs_bind_ask_user_question(self): template, msg=f"missing ask_tool_name binding in {entry['path']}", ) + allowed_tools_line = next( + (line for line in entry["template"] if line.lstrip().startswith("allowed-tools:")), + None, + ) + self.assertIsNotNone(allowed_tools_line, msg=f"no allowed-tools line in {entry['path']}") + self.assertIn( + "AskUserQuestion", + allowed_tools_line, + msg=f"AskUserQuestion instructed but not permitted in {entry['path']}", + ) + + def test_kit_workflow_skill_template_claude_binds_and_permits_ask_user_question(self): + """Issue #142 follow-up: `_KIT_WORKFLOW_SKILL_TEMPLATES['claude']` is a + separate production call site from `_default_agents_config()`'s + outputs -- it needs its own coverage, both for the binding and for + `allowed-tools:` actually granting it.""" + from studio.commands.agents import _KIT_WORKFLOW_SKILL_TEMPLATES + + template = _KIT_WORKFLOW_SKILL_TEMPLATES["claude"] + joined = "\n".join(template) + self.assertIn('- ask_tool_name = "AskUserQuestion"', joined) + allowed_tools_line = next( + (line for line in template if line.lstrip().startswith("allowed-tools:")), + None, + ) + self.assertIsNotNone(allowed_tools_line) + self.assertIn("AskUserQuestion", allowed_tools_line) def test_shared_skill_outputs_have_no_exact_ask_tool_binding(self): """The shared `.agents/skills/` bucket is byte-identical across From a12e883c126ec686dad6a99e8bcfc43d9c96e273 Mon Sep 17 00:00:00 2001 From: TECK KEAT WILSON Date: Fri, 11 Sep 2026 15:37:26 +0800 Subject: [PATCH 7/7] fix(pdsl): make the native-dialog field mapping a concrete abstract contract Third round of ainetx review feedback on #142: the previous "TITLE fills its question/header field" wording was still informal prose with no named request shape, and the two field names ("question" and "header") read as interchangeable when they're actually distinct fields on real tools. Replaced with a fixed abstract contract that's independent of any one tool's literal field names (a single prompt string, an ordered list of options each with a display label, display description, and a retained canonical identity), plus a concrete worked example for the one tool actually bound today (Claude's AskUserQuestion: one questions entry, question/header set from the prompt, options[].label/description from each option). Refs #142 Signed-off-by: TECK KEAT WILSON --- .../modules/runtime/pdsl-execution-card.md | 22 ++++++++++--------- tests/test_workflow_subagents_dispatch.py | 10 ++++++++- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/skills/studio/modules/runtime/pdsl-execution-card.md b/skills/studio/modules/runtime/pdsl-execution-card.md index 606b6421c..196b772c1 100644 --- a/skills/studio/modules/runtime/pdsl-execution-card.md +++ b/skills/studio/modules/runtime/pdsl-execution-card.md @@ -54,17 +54,19 @@ RULES: no different behavior. ALWAYS, for a shape-compatible `EMIT_MENU` where the active `ask_tool_name` context is a real tool name (not `unset` or never established), invoke - that tool instead of rendering the menu as prose, mapped onto that tool's - own question/options schema: `TITLE` fills its single question/header - field (there is one `EMIT_MENU` per invocation, never a multi-question - batch), and each numbered `OPTIONS` entry becomes one selectable option — - label from the entry's short form, description from its action clause — - with the entry's number/alias retained as that option's canonical, - non-displayed identity so the returned selection resumes the exact + that tool instead of rendering the menu as prose, built from this fixed + abstract contract — not the tool's own literal field names, which vary by + harness: one prompt string (from `TITLE`); an ordered list of options, + each carrying a display label (the entry's short form), a display + description (its action clause), and a canonical, non-displayed identity + (the entry's number/alias) so the returned selection resumes the exact numbered branch regardless of how the harness renders or truncates the - displayed label; that invocation is itself the turn's `WAIT`/`STOP_TURN` - boundary — NEVER additionally re-render the menu as text or execute a - redundant `STOP_TURN` after it. + displayed label. For Claude's bound `AskUserQuestion`: one `questions` + entry, its `question`/`header` set to the prompt, `options[].label` and + `options[].description` set from each option's display label/description. + That invocation is itself the turn's `WAIT`/`STOP_TURN` boundary — NEVER + additionally re-render the menu as text or execute a redundant + `STOP_TURN` after it. ALWAYS treat a native-tool result that selects none of the numbered `OPTIONS` — an out-of-band/free-text answer, a cancellation, a dismissal, or a tool error — as unmatched input for the menu's own `INVALID` handler; diff --git a/tests/test_workflow_subagents_dispatch.py b/tests/test_workflow_subagents_dispatch.py index a1486397d..892b41d79 100644 --- a/tests/test_workflow_subagents_dispatch.py +++ b/tests/test_workflow_subagents_dispatch.py @@ -2668,10 +2668,18 @@ def test_pdsl_execution_card_routes_emit_menu_through_native_ask_tool() -> None: assert "identically to `unset`" in normalized assert "cancellation, a dismissal, or a tool error" in normalized assert "unmatched input for the menu's own `INVALID` handler" in normalized - assert "that invocation is itself the turn's" in normalized + assert "invocation is itself the turn's" in normalized assert "boundary" in normalized assert "blocking gate paired with" in normalized + # Concrete field-mapping contract (issue #142 follow-up): an abstract + # prompt/options shape independent of any one tool's literal field names, + # plus a worked example for the one tool actually bound today. + assert "not the tool's own literal field names, which vary by" in normalized + assert "one prompt string (from `TITLE`)" in normalized + assert "canonical, non-displayed identity" in normalized + assert "one `questions` entry, its `question`/`header`" in normalized + def test_required_bootstrap_ask_tool_handoff_names_match_the_generated_lines() -> None: """Issue #142 follow-up: the bridging rule in required-bootstrap.md must