diff --git a/.changeset/route-defers-onboarding-questions.md b/.changeset/route-defers-onboarding-questions.md new file mode 100644 index 00000000..e3f074ef --- /dev/null +++ b/.changeset/route-defers-onboarding-questions.md @@ -0,0 +1,5 @@ +--- +"@taskless/cli": patch +--- + +`route` (topic v11) and `onboard` (topic v12) now give one answer to "may I ask the user here?". `route` says that a recipe which limits questions, such as `onboard`, overrides its step 7 choice between a local rule and a generation, its clarifying question when two destinations fit, and its failure fallback's confirmation. `onboard` says what to do at each of those points instead: write the local rule, or skip the candidate and name it beside the report summary. Previously an onboarding agent could read `route` as permission to ask questions that `onboard` forbids. diff --git a/packages/cli/src/agent/onboard.md b/packages/cli/src/agent/onboard.md index f6e8dea0..1629334e 100644 --- a/packages/cli/src/agent/onboard.md +++ b/packages/cli/src/agent/onboard.md @@ -1,4 +1,4 @@ -# Topic: onboard (CLI v%(CLI_VERSION)s / topic v12) +# Topic: onboard (CLI v%(CLI_VERSION)s / topic v13) %(ONE_COMMAND_PER_CALL)s @@ -176,6 +176,25 @@ making the case for running them in CI. many instances of its pattern you saw in the code: the report asks for that estimate. + `route`, and the recipes it sends you to, ask the user in a few + places. During onboarding you ask none of them; step 9 is the only + clarifying question, and only in its case. Decide each yourself: + + - **`route` offers a local rule or a generation** (its step 7): + write the local rule. Generation needs a login, and onboarding + stays logged out. + - **You cannot tell which destination fits** (`route`'s clarifying + question): skip the candidate. Name it in one line when you show + the report summary, as a candidate you left unwritten and why. + - **A local rule fails its cases** (`route`'s failure fallback): do + not offer generation. If the pattern needs runtime evidence, keep it + as a Taskless Cloud candidate as above; otherwise skip it and name + it the same way. + - **A destination recipe asks you to confirm something**, such as + exclusions, an overlapping rule, or which linter owns it: decide + from what you read in the repository. Where you cannot decide, skip + the candidate and name it the same way. + 9. **Clarify only when stuck.** You may ask the user something only when you are unsure what rules to write AND the report cannot be built yet because there are fewer than three working Taskless diff --git a/packages/cli/src/agent/route.md b/packages/cli/src/agent/route.md index f5a3ab26..e372babb 100644 --- a/packages/cli/src/agent/route.md +++ b/packages/cli/src/agent/route.md @@ -1,4 +1,4 @@ -# Topic: route (CLI v%(CLI_VERSION)s / topic v10) +# Topic: route (CLI v%(CLI_VERSION)s / topic v11) %(ONE_COMMAND_PER_CALL)s @@ -16,6 +16,28 @@ answered together. - A working repository the agent can read. - No auth required to route. Login state is an *input* to the decision, not a requirement of making it. +- **Another recipe may have sent you here.** When the recipe you came + from limits what you may ask the user, as `onboard` does, its limits + win over every question below: step 7's choice, the clarifying + question under "When two destinations both fit", and the failure + fallback's confirmation. Do what that recipe says to do instead. + Everywhere else, follow this recipe as written. + + Those limits also cover the recipes this one sends you to, which ask + their own questions: + - `create-sg-rule`: confirms exclusions, asks whether to improve an + overlapping rule, and asks before escalating a failed rule. + - `create-legacy-rule`: asks which linter owns the rule when more than + one could, and before adding a plugin the repo does not have. Its + per-linter pages ask the same, and Ruff's asks before turning on + preview mode. + - `create-runtime-rule`: asks whether the user wants an account. + - `create-remote-rule`: asks about an overlapping rule, asks for the + cases, and confirms before spending a generation. + - `create-vale-rule` asks nothing. + + A recipe that limits questions should say what to do at each of + these. ## Steps @@ -268,7 +290,8 @@ answered together. rule is expressible locally, the user is logged in, **and** `ghOwner` is not `[unknown]`, present both and let them pick: a local rule you write and verify now, or a generation that costs against their plan. - Say which you would choose and why. + Say which you would choose and why. If a recipe that limits questions + sent you here, do not offer the choice; that recipe says which to do. Not logged in, no GitHub owner, or not locally expressible are not choices. Do not pose them as one. @@ -284,9 +307,10 @@ answered together. - **A rule that seems to need two engines is usually two rules.** Split it and route each half, rather than forcing one engine to cover evidence it cannot see. -- **You genuinely cannot tell** → ask the user a clarifying question. - Uncertainty is a reason to ask; it is never a reason to spend a - generation. +- **You genuinely cannot tell** → ask the user a clarifying question, + unless a recipe that limits questions sent you here; then that recipe + decides. Uncertainty is a reason to ask; it is never a reason to spend + a generation. ## Failure fallback (try-verify-escalate) @@ -299,6 +323,9 @@ fails against the user's cases: - Call the service only after the user confirms. On yes, fetch `%(RECIPE_FETCH)screate-remote-rule`. +If a recipe that limits questions sent you here, do not ask for that +confirmation; do what that recipe says instead. + Never fall through silently from a failed local attempt to a service call. A developer who watches a local attempt fail reads it as a Taskless failure, so only attempt local when you reasonably believe it diff --git a/packages/cli/test/legacy-ladder.test.ts b/packages/cli/test/legacy-ladder.test.ts index 3d61c3f4..bf62c3de 100644 --- a/packages/cli/test/legacy-ladder.test.ts +++ b/packages/cli/test/legacy-ladder.test.ts @@ -18,8 +18,8 @@ function tool(name: string): string { describe("route's legacy ladder", () => { const route = recipe("route"); - it("is topic v10", () => { - expect(route).toMatch(/^# Topic: route {5}\(CLI v.+ \/ topic v10\)/m); + it("is topic v11", () => { + expect(route).toMatch(/^# Topic: route {5}\(CLI v.+ \/ topic v11\)/m); }); it("orders shipped rule, repo examples, Taskless static, Taskless runtime", () => { @@ -48,6 +48,54 @@ describe("route's legacy ladder", () => { }); }); +// #675: route's own questions must yield to a recipe that limits asking, +// such as onboard, or reading the two together gives two answers. +describe("route's questions defer to the recipe that sent the agent", () => { + const route = recipe("route"); + const flat = route.replaceAll(/\s+/g, " "); + + it("says a calling recipe's limits win, naming every question", () => { + expect(flat).toContain("**Another recipe may have sent you here.**"); + expect(flat).toContain( + "its limits win over every question below: step 7's choice, the clarifying question under \"When two destinations both fit\", and the failure fallback's confirmation." + ); + }); + + // A limiting recipe that reads only this precondition must learn that the + // destination recipes ask too, or it misses their confirmations. + it("names the questions each destination recipe asks on its own", () => { + expect(flat).toContain( + "Those limits also cover the recipes this one sends you to, which ask their own questions:" + ); + expect(flat).toContain( + "`create-sg-rule`: confirms exclusions, asks whether to improve an overlapping rule, and asks before escalating a failed rule." + ); + expect(flat).toContain( + "`create-legacy-rule`: asks which linter owns the rule when more than one could, and before adding a plugin the repo does not have." + ); + expect(flat).toContain("Ruff's asks before turning on preview mode."); + expect(flat).toContain( + "`create-runtime-rule`: asks whether the user wants an account." + ); + expect(flat).toContain( + "`create-remote-rule`: asks about an overlapping rule, asks for the cases, and confirms before spending a generation." + ); + expect(flat).toContain("`create-vale-rule` asks nothing."); + }); + + it("qualifies step 7's choice, the clarifying question, and the fallback in place", () => { + expect(flat).toContain( + "If a recipe that limits questions sent you here, do not offer the choice" + ); + expect(flat).toContain( + "ask the user a clarifying question, unless a recipe that limits questions sent you here; then that recipe decides." + ); + expect(flat).toContain( + "If a recipe that limits questions sent you here, do not ask for that confirmation; do what that recipe says instead." + ); + }); +}); + describe("the generic legacy recipe", () => { const legacy = recipe("create-legacy-rule"); diff --git a/packages/cli/test/onboard.test.ts b/packages/cli/test/onboard.test.ts index c4f136a5..0166669a 100644 --- a/packages/cli/test/onboard.test.ts +++ b/packages/cli/test/onboard.test.ts @@ -402,8 +402,8 @@ describe("onboard recipe asks one question and finishes on its own", () => { : stdout.slice(start, start + title.length + next); } - it("is topic v12", () => { - expect(stdout).toMatch(/^# Topic: onboard {5}\(CLI v.+ \/ topic v12\)/m); + it("is topic v13", () => { + expect(stdout).toMatch(/^# Topic: onboard {5}\(CLI v.+ \/ topic v13\)/m); }); // v10 (#554): a worktree holds only commits, and `init` commits nothing, so @@ -489,6 +489,25 @@ describe("onboard recipe asks one question and finishes on its own", () => { expect(write).toContain("the report asks for that estimate"); }); + // #675: route and the recipes it sends to still ask in places; onboarding + // must say what to do instead at each one, so the two read as one answer. + it("answers each of route's questions itself instead of asking", () => { + const write = step("Write the rules").replaceAll(/\s+/g, " "); + expect(write).toContain("During onboarding you ask none of them"); + expect(write).toContain( + "**`route` offers a local rule or a generation** (its step 7): write the local rule." + ); + expect(write).toContain( + "**You cannot tell which destination fits** (`route`'s clarifying question): skip the candidate." + ); + expect(write).toContain( + "(`route`'s failure fallback): do not offer generation." + ); + expect(write).toContain( + "**A destination recipe asks you to confirm something**" + ); + }); + it("limits clarifying questions to a refused report", () => { const clarify = step("Clarify only when stuck"); expect(clarify).toContain("fewer than three working Taskless");