Skip to content

feat(onboard): decide when the rules run before asking to mark complete - #465

Merged
theCodeDrift merged 4 commits into
mainfrom
onboard-decide-when-rules-run
Oct 6, 2026
Merged

theCodeDrift merged 4 commits into
mainfrom
onboard-decide-when-rules-run

Conversation

@theCodeDrift

Copy link
Copy Markdown
Member

The onboard recipe went from writing rules straight to the consent-gated --mark-complete question. Nothing asked when the new rules would run, so a project could finish onboarding with rules that never run. When an agent made up the missing question itself, it asked it in the same message as mark-complete, and the user's "yes" could have answered either one.

What changes

  • Onboard recipe (v4 → v5). A new step, decide when the rules run, comes after the rules are written and before mark-complete. The agent tells the user which CI systems and commit-hook tools the repository has, then offers agent ci and agent hooks and does whichever the user picks. Picking neither is fine, and the recipe says so in one line. The mark-complete question must be asked in a message with no other question in it. See Also now lists ci and hooks.
  • detect --json gains two fields, ci and hooks, both lists of { name, evidence } entries shaped like linters. Nothing existing changes. Both are read at the repository root only: CI reads its config from there, and git runs one set of hooks per repository, so a match inside a sub-package is not this repo's setup.
  • New agent hooks topic (internal). It covers running check on staged files in the hook tool the repo already uses (lint-staged, lefthook, pre-commit, husky, simple-git-hooks). It also covers:
    • exiting early when nothing is staged, because check with no paths scans the whole project
    • running test plus a full check when a file under .taskless/rules/ changes
    • running the repo's pinned dev-dependency binary instead of a download-and-run launcher.
  • ci (v3 → v4), check (v5 → v6), detect (v1 → v2) now point to the new field and topic. The installed skill's topic table gets a hooks row.

Where this differs from the issue's suggestions

  • --no-stash has a downside, and the recipe states it. lint-staged documents that the flag implies --no-hide-partially-staged. A partially staged file is then checked as it is in the working tree, not as staged. The recipe explains the trade-off and leaves the choice to the user.
  • A hook does not run static rules only. When the developer is logged in, check verifies the rules with the service on every commit and runs the runtime rules that pass. The recipe says this, offers --anonymous if that is too slow, and forbids --dangerously-run-scripts.

Verification

  • pnpm typecheck and pnpm lint pass, and the full CLI suite passes (114 files, 2035 tests).
  • New tests: detect.test.ts covers every CI system and hook tool, root-only matching, and empty arrays. onboard.test.ts covers step order, the decline path, the question asked alone, and See Also.
  • Pre-archive scenario check: every scenario in cli-onboard, cli-detect and cli-agent survives the archive, plus four new ones in each.

Fixes #441

@theCodeDrift

Copy link
Copy Markdown
Member Author

@claude /review

@claude

claude Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Claude finished @theCodeDrift's task in 19m 23s —— View job


Review complete

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contained no review threads, no review summaries, and no top-level comments other than the trigger comment itself, so this is the first review of this PR. The whole diff was assessed from scratch; there are no previously-raised items to classify as addressed or still-open.

  • Read prior review data
  • Fetch PR diff and metadata
  • Review onboard/ci/hooks/detect recipe changes
  • Review detect/automation.ts, detect/scan.ts, schema changes
  • Review tests
  • Post findings

Assessment

I read the full diff (gh pr diff) plus the complete contents of packages/cli/src/detect/automation.ts, packages/cli/src/commands/detect.ts, packages/cli/src/prompts/index.ts, packages/cli/src/agent/onboard.md/hooks.md, the recipe-cross-references.test.ts guard, and both test files in full, along with the openspec deltas.

Correctness

  • detectAutomation's root-only matching, directory-presence checks (trailing-/ patterns), and package.json key/dependency lookups are internally consistent and match the detect.test.ts expectations exactly, including evidence ordering (paths → packageJsonKeys → deps) and the sub-package exclusion tests.
  • The new hooks.md content stays clear of the hand-written-invocation guard in recipe-cross-references.test.ts by construction: it always writes <local-taskless> check/test rather than a bare taskless <subcommand>, so no allowlist entry was needed, matching the PR description's claim.
  • hooks is correctly added to INTERNAL_TOPICS (alphabetically placed) and not to the public TOPICS export, consistent with the stated internal classification.
  • onboard.md's new step 8 and renumbered step 9 line up with what onboard.test.ts asserts (step ordering, the "decide when the rules run" offer text, the "neither" decline path, the lone mark-complete question, and the See Also entries).
  • The detect --json output schema and detect.ts human-readable output both handle empty ci/hooks arrays correctly, matching the "empty arrays, not omitted" spec scenario.
  • The archived openspec delta for cli-detect and cli-onboard are additive — every previously-existing scenario is preserved as unchanged context in the diff, with new scenarios appended, so this doesn't fall into the "MODIFIED Requirements replaces the whole requirement" trap called out in this repo's CLAUDE.md.
  • node:fs's globSync/existsSync usage in automation.ts mirrors the pattern already used in scan.ts, and the repo's engines.node >=22.22.0 supports it.
  • The changeset is a single file, correctly scoped to patch (package is still 0.y.z, so new additive fields/topics don't warrant minor per this repo's documented semver policy).

Minor/non-blocking observations (none rise to the level of a required change)

  • hooks.md's manual husky/simple-git-hooks example (git diff --cached --name-only --diff-filter=ACMR) excludes deleted files, while the prose a few lines above says check "skips any that do not exist, so a staged list that includes deleted files can go straight in." Not a bug — just a slightly more conservative choice in that one example — but a reader comparing the two could briefly wonder why deleted files are filtered out there when the text says they don't need to be.
  • packages/cli/src/commands/detect.ts's new CI/hooks loop uses a tuple-array/as const pattern that differs stylistically from the if/else blocks just above it for linters/ruleStyles in the same function. Harmless, but a maintainer skimming the function will notice the shift in style partway through.

No correctness, security, or test-adequacy issues found. Test coverage for the new detect fields and the onboard step ordering is thorough (root-only matching, empty-array case, each CI/hook signal, decline path, lone-question constraint, See Also).

@theCodeDrift

Copy link
Copy Markdown
Member Author

Re: @claude — "Claude finished @theCodeDrift's task in 19m 23s…"
#465 (comment)

Both minor observations are addressed in 1630aca. hooks.md now says the --diff-filter=ACMR in the husky example drops deleted files up front, which check would skip anyway, so a commit that only deletes files never reaches check. detect.ts prints CI and commit hooks as two plain if/else blocks, matching the linters and ruleStyles output above them.

— AI Coding Agent

Onboarding went from materializing rules straight to the consent-gated
mark-complete question, so a project could finish with rules nothing runs.
The onboard recipe now offers CI (agent ci) and a pre-commit hook (the new
agent hooks topic) first, and asks mark-complete in a message of its own.
detect --json reports the CI systems and commit-hook tools configured at
the repository root as two additive fields, ci and hooks.
@theCodeDrift
theCodeDrift force-pushed the onboard-decide-when-rules-run branch from 1630aca to 2cde3be Compare October 6, 2026 05:00
@theCodeDrift
theCodeDrift merged commit 20b191b into main Oct 6, 2026
4 checks passed
@theCodeDrift
theCodeDrift deleted the onboard-decide-when-rules-run branch October 6, 2026 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Onboarding asks to mark complete without deciding when the rules run (CI, pre-commit)

1 participant