diff --git a/docs/claude-mod.md b/docs/claude-mod.md index 0beeb054a..9f394ec17 100644 --- a/docs/claude-mod.md +++ b/docs/claude-mod.md @@ -56,6 +56,27 @@ that file atomically. Session start restores only the launch baseline, so an ove before `/clear` still survives it. The live skill-toggle journey covers the real CLI write; native component tests stub the process calls. +Before each on-to-off change, Isaac sessions with +`ISAAC_SMART_ROUTER_FEEDBACK=1` require a reason of at least 16 characters after +trimming surrounding whitespace. Isaac sets this flag from its +rollout setting or the user's explicit `1`/`0` override. It controls Isaac feedback +availability, so UG's routing presets and on/off toggles leave it unchanged. +The native form keeps routing on until Enter accepts a valid reason, then disables +routing and sends the exact text through `/isaac-feedback --category smart-routing`. +Cancel or Escape leaves routing on and sends nothing. Missing commands or an +unavailable form leave routing on. A feedback submission failure after a valid +reason leaves routing off and reports the failure. Opening or dismissing the form +makes no model requests; only submitting invokes the feedback command. + +The CLI/skill path, including Codex, enforces the same minimum through the session +toggle's `--smart-router-feedback` argument. The skill asks for the reason before +retrying the toggle and submits it through `/isaac-feedback` on success. The CLI +validates the reason; the skill or native form owns submission. + +Validate the form in Isaac, including the 16-character minimum, exact reason +submission, cancellation, repeated toggles and failures. These scenarios and a live +evidence upload are not covered by the automated suite. + The existing Claude skill-toggle integration journey checks recorded inference payloads during its three calculation phases: launch recipe, `DISABLED` after off, and launch recipe after on. Each phase retains completed native parent/child diff --git a/skills/smart-router/SKILL.md b/skills/smart-router/SKILL.md index 439f8d4a8..23974bf24 100644 --- a/skills/smart-router/SKILL.md +++ b/skills/smart-router/SKILL.md @@ -1,9 +1,9 @@ --- name: smart-router description: Enable or disable Unity Gateway subagent model routing for the current smart-routed Claude or Codex session, along with Smart Router Orchestrator when opted in. -allowed-tools: Bash("$UCODE_SMART_ROUTER_PYTHON" -m ucode.cli claude --enable-smart-routing), Bash("$UCODE_SMART_ROUTER_PYTHON" -m ucode.cli claude --disable-smart-routing), Bash("$UCODE_SMART_ROUTER_PYTHON" -m ucode.cli codex --enable-smart-routing), Bash("$UCODE_SMART_ROUTER_PYTHON" -m ucode.cli codex --disable-smart-routing) +allowed-tools: Bash("$UCODE_SMART_ROUTER_PYTHON" -m ucode.cli claude --enable-smart-routing), Bash("$UCODE_SMART_ROUTER_PYTHON" -m ucode.cli claude --disable-smart-routing), Bash("$UCODE_SMART_ROUTER_PYTHON" -m ucode.cli claude --disable-smart-routing --smart-router-feedback *), Bash("$UCODE_SMART_ROUTER_PYTHON" -m ucode.cli codex --enable-smart-routing), Bash("$UCODE_SMART_ROUTER_PYTHON" -m ucode.cli codex --disable-smart-routing), Bash("$UCODE_SMART_ROUTER_PYTHON" -m ucode.cli codex --disable-smart-routing --smart-router-feedback *) metadata: - version: "1.2.0" + version: "1.4.0" --- # Smart Router @@ -21,6 +21,16 @@ In PowerShell, use `& "$env:UCODE_SMART_ROUTER_PYTHON"` in place of If `UCODE_SMART_ROUTER_PYTHON` or `UCODE_SESSION_ENV_FILE` is unset, ask the user to restart through an updated Unity Gateway with smart routing enabled. +If the off command requires feedback, routing is still on. Ask why the user is +turning Smart Router off. Require at least 16 characters after trimming surrounding +whitespace; ask again if the reason is shorter. Never invent or pad their reason. +If they cancel, stop without disabling routing. Once they provide a valid reason, +repeat the off command with `--smart-router-feedback ''` appended, quoting +the exact text as one shell argument. After a successful toggle, submit that text +through `/isaac-feedback --category smart-routing `. Report a submission +failure without re-enabling routing. This requirement applies when Isaac sets +`ISAAC_SMART_ROUTER_FEEDBACK=1`; do not change that flag to bypass it. + With no argument, explain that only `on` and `off` are accepted. Do not edit the state file. This affects subsequent subagent model selection in the current session, not the root model or first prompt. In sessions launched with `ENABLE_SMART_ROUTER_ORCHESTRATOR=1`, it also controls Smart Router diff --git a/src/ucode/agents/claude_mods/feedback.ts b/src/ucode/agents/claude_mods/feedback.ts new file mode 100644 index 000000000..bf36ea98f --- /dev/null +++ b/src/ucode/agents/claude_mods/feedback.ts @@ -0,0 +1,53 @@ +import type { On } from 'claude-code'; + +export const FEEDBACK_PANE = 'smart-router-feedback'; + +export type FeedbackState = { + submitted: boolean; + reason: string; + disableRouting?: (reason: string) => Promise; +}; + +export function registerFeedback(on: On, state: FeedbackState): void { + on('ui.render', { component: 'Pane' }, ($, e, next) => { + if (e.requestId !== FEEDBACK_PANE || e.surface === 'mobile') return next(e); + const { Box, Text, Input, Button } = $.ui.resolve(e); + return Box({ + flexDirection: 'column', + children: [ + Text({ children: ['Why are you turning Smart Router off?'] }), + Text({ children: ['Enter at least 16 characters. Your reason and session evidence go to /isaac-feedback.'] }), + Input({ + key: 'reason', + label: 'Reason', + value: state.reason, + autoFocus: true, + submitLabel: 'Send feedback and turn off', + onInput: (value) => { state.reason = value; }, + onSubmit: async (value) => { + if (state.submitted || !state.disableRouting) return; + if (Array.from(value.trim()).length < 16) { + $.ui.toast('Enter at least 16 characters, excluding surrounding whitespace.'); + return; + } + state.submitted = true; + try { + await state.disableRouting(value); + } catch { + state.submitted = false; + $.ui.toast('Could not update Smart Router. Smart Router is still on; retry or restart through UG.'); + return; + } + try { + await $.ui.close({ id: FEEDBACK_PANE }); + await $.command.run({ command: 'isaac-feedback', args: `--category smart-routing ${value}` }); + } catch { + $.ui.toast('Smart Router is off, but feedback could not open. Retry with /isaac-feedback --category smart-routing.'); + } + }, + }), + Button({ key: 'cancel', label: 'Cancel (keep routing on)', onPress: () => $.ui.close({ id: FEEDBACK_PANE }) }), + ], + }); + }); +} diff --git a/src/ucode/agents/claude_mods/register.ts b/src/ucode/agents/claude_mods/register.ts index 277b5be10..e2072149c 100644 --- a/src/ucode/agents/claude_mods/register.ts +++ b/src/ucode/agents/claude_mods/register.ts @@ -10,6 +10,7 @@ import { RECIPE_FIELD, } from './constants.ts'; import { mergeExtraBody } from './extra-body.ts'; +import { FEEDBACK_PANE, registerFeedback, type FeedbackState } from './feedback.ts'; type Environment = Record; @@ -32,7 +33,7 @@ async function writeEnvironment($: EngineInterface, env: Environment): Promise { +async function writeSessionControls($: EngineInterface, enabled: boolean, reason?: string): Promise { // Claude's capability scanner requires literal environment-variable names. const python = await $.env.get('UCODE_SMART_ROUTER_PYTHON'); const sessionFile = await $.env.get('UCODE_SESSION_ENV_FILE'); @@ -40,7 +41,9 @@ async function writeSessionControls($: EngineInterface, enabled: boolean): Promi throw new Error('Smart Router session controls are unavailable for this session.'); } const flag = enabled ? ENABLE_ROUTING_FLAG : DISABLE_ROUTING_FLAG; - const result = await $.process.run([python, '-m', CLI_MODULE, CLI_AGENT, flag]); + const args = [python, '-m', CLI_MODULE, CLI_AGENT, flag]; + if (reason !== undefined) args.push('--smart-router-feedback', reason); + const result = await $.process.run(args); if (result.exitCode !== 0) { throw new Error(`Smart Router controls exited ${result.exitCode}: ${result.stderr.trim()}`); } @@ -51,6 +54,7 @@ async function setRouting( options: PluginOptions, enabled: boolean, syncSessionControls: boolean, + reason?: string, ): Promise { const original = (key: string): string | undefined => (options.unset as readonly string[]).includes(key) ? undefined : options[key] as string; @@ -72,7 +76,7 @@ async function setRouting( // An explicit toggle must also replace the CLI/skill's saved session overrides, // which the routing hooks apply on top of the environment and would otherwise win. if (syncSessionControls) { - await writeSessionControls($, enabled); + await writeSessionControls($, enabled, reason); } } catch (error) { // Restore earlier successful writes if a later environment write is denied. @@ -83,6 +87,7 @@ async function setRouting( export function register(on: On, options: PluginOptions): void { + const feedback: FeedbackState = { submitted: false, reason: '' }; const launchedEnabled = options.ENABLE_SMART_ROUTING_V2 === '1' || options.ENABLE_SMART_ROUTING_SUBAGENT_ONLY === '1'; @@ -100,15 +105,43 @@ export function register(on: On, options: PluginOptions): void { const action = e.args.trim(); if (action !== 'on' && action !== 'off') return {text: 'Usage: /smart-router on|off'}; if (!launchedEnabled) return {text: 'Smart Router was not enabled for this session.'}; + if (action === 'off' && await $.env.get('ISAAC_SMART_ROUTER_FEEDBACK') === '1') { + try { + const current = await readEnvironment($); + if (current.ENABLE_SMART_ROUTING_V2 !== '1' && current.ENABLE_SMART_ROUTING_SUBAGENT_ONLY !== '1') { + return {text: 'Smart Router is off'}; + } + if ((await $.command.list()).some((command) => command.name === 'isaac-feedback')) { + feedback.submitted = false; + feedback.reason = ''; + feedback.disableRouting = async (reason) => { + await setRouting($, options, false, true, reason); + showStatusBand(); + $.ui.invalidate('ui.render'); + }; + const pane = await $.ui.open({ + id: FEEDBACK_PANE, title: 'Turn Smart Router off', focus: true, + closeOnEscape: true, rows: 6, columns: 56, + }); + if (pane.isPlaced) return {text: 'A reason of at least 16 characters is required to turn Smart Router off.'}; + // Do not leave a delayed question waiting for an unrelated task. + await $.ui.close({ id: FEEDBACK_PANE }); + } + } catch { + // No reason was collected, so leave routing unchanged. + } + return {text: 'Could not open the feedback form. Smart Router is still on; retry /smart-router off in an Isaac session with /isaac-feedback available.'}; + } try { await setRouting($, options, action === 'on', true); showStatusBand(); $.ui.invalidate('ui.render'); - return {text: `Smart Router is ${action}`}; } catch { return {text: 'Could not update Smart Router. Check the recipe, CLAUDE_CODE_EXTRA_BODY, and environment permissions; retry or restart through UG.'}; } + return {text: `Smart Router is ${action}`}; }); + registerFeedback(on, feedback); registerStatusBand(on); registerSubagentRouting(on, options); } diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 624c18524..84461ed3b 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -2725,13 +2725,24 @@ def _smart_routing_v2_flag(enabled: bool | None, *, agent: str) -> Iterator[None smart_routing_v2.restore_smart_routing_env(previous) -def _toggle_current_smart_routing_session(enabled: bool | None) -> bool: +def _toggle_current_smart_routing_session(enabled: bool | None, reason: str | None = None) -> bool: if enabled is None: return False try: session_env_path() except RuntimeError: return False + if ( + not enabled + and os.environ.get("ISAAC_SMART_ROUTER_FEEDBACK") == "1" + and len((reason or "").strip()) < 16 + ): + print_err( + "Turning Smart Router off requires a reason of at least 16 characters, excluding " + "surrounding whitespace. Ask the user why, then pass their reason with " + "--smart-router-feedback and submit it through /isaac-feedback --category smart-routing." + ) + raise typer.Exit(2) try: set_session_environment(dict.fromkeys(SMART_ROUTING_ENV_KEYS, "0") if not enabled else {}) except RuntimeError as exc: @@ -3732,9 +3743,13 @@ def codex_cmd( help="Enable or disable AI Gateway model routing for this Codex launch or session.", ), ] = None, + smart_router_feedback: Annotated[ + str | None, + typer.Option("--smart-router-feedback", hidden=True), + ] = None, ) -> None: """Launch Codex via Databricks.""" - if _toggle_current_smart_routing_session(enable_smart_routing_flag): + if _toggle_current_smart_routing_session(enable_smart_routing_flag, smart_router_feedback): return try: custom_oauth = _custom_oauth_config(client_id, redirect_url, scopes) @@ -3820,9 +3835,13 @@ def claude_cmd( help="Enable or disable AI Gateway model routing for this Claude Code launch or session.", ), ] = None, + smart_router_feedback: Annotated[ + str | None, + typer.Option("--smart-router-feedback", hidden=True), + ] = None, ) -> None: """Launch Claude Code via Databricks.""" - if _toggle_current_smart_routing_session(enable_smart_routing_flag): + if _toggle_current_smart_routing_session(enable_smart_routing_flag, smart_router_feedback): return try: custom_oauth = _custom_oauth_config(client_id, redirect_url, scopes) diff --git a/src/ucode/smart_routing/v2.py b/src/ucode/smart_routing/v2.py index f568af8e7..7656a77c4 100644 --- a/src/ucode/smart_routing/v2.py +++ b/src/ucode/smart_routing/v2.py @@ -410,6 +410,7 @@ def _write_routed_claude_plugin(plugin_dir: Path, model_ids: list[str]) -> None: "register.ts", "constants.ts", "extra-body.ts", + "feedback.ts", "smart-routing-status.ts", "subagent-routing.ts", ): diff --git a/tests/README.md b/tests/README.md index f785238ca..095d3d7ee 100644 --- a/tests/README.md +++ b/tests/README.md @@ -19,6 +19,10 @@ TypeScript event-forwarding and launch-option tests in CI. The existing Claude skill-toggle integration journey checks the live recipe payload field across on/off/on. See [the mod guide](../docs/claude-mod.md). +The required Isaac feedback form needs manual validation of the 16-character minimum, +cancellation, repeated toggles, and submission failures. The existing routing-toggle +journeys do not cover this form or a live Isaac evidence upload. + The **harness-latest** workflow runs every six hours on `main` and currently covers smart routing only. It resolves npm's `claude-code@latest` diff --git a/tests/integration/README.md b/tests/integration/README.md index 56377f447..8cfcae087 100644 --- a/tests/integration/README.md +++ b/tests/integration/README.md @@ -65,6 +65,8 @@ The Claude mod entry point has separate native component checks in `../native_claude/test_mod.py`. Claude 2.1.290 validates the module and runs its TypeScript event-forwarding and launch-option tests in a dedicated CI job. The existing Claude skill-toggle journey checks the live recipe payload field across on/off/on. +The required Isaac feedback form and evidence submission need a separate manual +Isaac check. The gateway toggle journey does not cover the form. The existing unit tests keep their fixtures. Integration has an independent pytest configuration and uses `--confcutdir` so those fixtures cannot leak in. diff --git a/tests/test_claude_mod.py b/tests/test_claude_mod.py index 5e924846a..b62ab61cc 100644 --- a/tests/test_claude_mod.py +++ b/tests/test_claude_mod.py @@ -19,6 +19,7 @@ def test_plugin_contains_mod_entry_point(tmp_path): "register.ts", "constants.ts", "extra-body.ts", + "feedback.ts", "smart-routing-status.ts", "subagent-routing.ts", ): @@ -26,6 +27,7 @@ def test_plugin_contains_mod_entry_point(tmp_path): assert sorted(path.name for path in (tmp_path / "hooks").iterdir()) == [ "constants.ts", "extra-body.ts", + "feedback.ts", "hooks.json", "register.ts", "smart-routing-status.ts",