Merged
Conversation
The init subcommand provides a wizard that sets up runok configuration for user-level and/or project-level scopes. It supports Claude Code integration by parsing existing permission entries from settings.json, converting Bash permissions to runok rules, registering a PreToolUse hook, and optionally removing the migrated permissions. Key features: - --scope user|project to target a specific scope - -y for non-interactive mode with sensible defaults - --force to overwrite existing configuration files - Automatic detection and conversion of Claude Code permissions - Hook registration in .claude/settings.json
Claude Code permission entries use space-separated arguments (e.g. `Bash(npm install *)`), not colon-separated (e.g. `Bash(npm:install:*)`). The `convert_bash_pattern` function was based on a wrong assumption about the format and has been removed. Patterns from settings.json are now used as-is without transformation.
- Replace verbose boilerplate comments with a single yaml-language-server $schema directive, which provides editor autocompletion via JSON Schema - Remove unnecessary "# runok configuration" and example rules comments from the generated config - Use assert_eq! with exact values instead of partial matchers (ends_with, starts_with, field-level JSON checks) across all test files - Add settings.json verification to project_flow_with_claude_code integration test to cover permission removal and hook registration
The hooks array requires objects with {"type": "command", "command": "..."},
not plain strings. The previous format caused `claude doctor` to report
"Expected object, but received string" and broke the user's settings.json.
Also replace partial assertions (is_none/is_object) with exact assert_eq!
on full JSON values in remove_permissions tests.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
The hook format fix in docs (README.md, claude-code.md) will be handled in a separate branch to keep this PR focused on the init command logic. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The init output was hard to read: confirmation prompts blended into surrounding text, the "Skipped non-Bash entries" list was noisy and unhelpful (non-Bash entries being skipped is expected behavior), and the summary didn't show what was actually converted. - Use ColorfulTheme for confirmation prompts so they stand out visually - Print converted rules before prompting so users see what will change - Remove skipped entries output (non-Bash skipping is the default behavior) Co-Authored-By: Claude Opus 4.6 <[email protected]>
Users couldn't tell what would actually change when answering "Remove converted permissions?" or "Register runok hook?" -- the prompts gave no visibility into the resulting settings.json modifications. Show a unified diff of settings.json before each confirmation prompt so users can see the exact changes before deciding. Uses the `similar` crate for proper Myers diff output. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The original settings.json used 3-space indentation while serde_json::to_string_pretty uses 2-space, causing every line to appear as changed in the diff output. Normalize the original content through serde before comparing so only structural changes are shown. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Two separate confirmation prompts ("Remove permissions?" and "Register
hook?") made the flow feel fragmented and didn't clearly communicate
what would happen. Users couldn't see the full picture before deciding.
Show all planned changes as numbered steps with colored unified diffs,
then ask a single "Apply these changes?" prompt. Also add the converted
runok.yml rules to the preview so users see where permissions move to.
Add unit tests for preview_remove_permissions, preview_register_hook,
and normalize_json.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
…rove test coverage wizard.rs was 1230 lines with mixed responsibilities (preview/diff, setup orchestration, public API). Split into three files for readability: - wizard/mod.rs: public API, types, helpers, integration tests - wizard/preview.rs: JSON preview/diff utilities and their tests - wizard/setup.rs: ScopeResult and setup_scope orchestration Also includes fixes from previous session that were not yet committed: - Prompter trait DI to enable per-step test control - remove_permissions now preserves non-Bash entries - preview_register_hook detects legacy hook format - Added parameterized integration tests for mixed permission scenarios
The previous wizard had two issues:
1. When no --scope flag was given, it asked two separate yes/no
questions ("Set up user-level?" then "Set up project-level?")
which was clunky. Now it uses a single select prompt to choose
between User (global) and Project (local).
2. Claude Code integration changes were confirmed per-step (up to 3
separate confirmations), which was tedious. Now all pending changes
are shown as a combined diff with a single "Apply these changes?"
confirmation — accept applies everything, decline skips everything.
Also adds `select` method to the Prompter trait alongside the existing
`confirm`, with implementations for DialoguerPrompter, AutoYesPrompter,
and test SequencePrompter (now uses a Response enum for type-safe
confirm/select queuing).
Project-level config is shared among all developers, so hook registration should be skipped (not everyone uses runok) and permission migration should be opt-in rather than automatic. - Add HookPolicy (Register/Skip) and MigrationPolicy (Always/Ask) - User scope: auto-migrate permissions and register hook - Project scope: ask before migrating (default no), never register hook - Show detection message before diff preview - Update tests for new project scope behavior - Change e2e test to use user scope for Claude Code integration
Claude Code uses `:*` as a prefix-match operator (e.g. `Bash(npm run:*)` matches commands starting with `npm run`). runok has no `:*` syntax so the literal string `npm run:*` would be treated as a pattern with a colon character followed by a wildcard, which is not the intended semantics. Convert `:*` suffix to a space + glob (e.g. `npm run *`).
The --force flag was redundant because the wizard already shows a confirmation prompt before overwriting. Removing it simplifies the UX: `-y` with existing config now overwrites directly. Also changed user scope to ask "Migrate from Claude Code?" instead of auto-migrating, giving users control over migration in all scopes. The migration prompt default is true for user scope (most users want migration) and false for project scope (shared config is more cautious). Fixed the diff preview for runok.yml to show existing file content as the "before" side instead of an empty string.
The migration prompt appeared before the "Detected Claude Code configuration" message, which was confusing because the user was asked about migration without context. Now the detection message is shown first, then the migration prompt follows naturally. Also ensures migration is only asked when Claude Code configuration is actually detected (has migratable rules or hook to register).
The previous commit changed when migration prompts appear (only when Claude Code config is detected) but integration tests still passed because AutoYesPrompter silently returns defaults for any prompt. Replace AutoYesPrompter with SequencePrompter + assert_exhausted in tests that verify Claude Code integration, so the exact number of confirm calls is validated. This catches regressions where prompts are added or removed without updating tests.
runok init's purpose is to create runok.yml. Previously, declining the migration/apply prompt resulted in no runok.yml being created at all, which defeated the purpose of running the command. Now runok.yml is always created: with converted rules if migration is accepted, or as boilerplate if declined.
Integration tests should verify observable behavior (file contents, file existence) not internal details like prompt call counts. Reverted SequencePrompter back to AutoYesPrompter where specific response sequences are not needed.
When Claude Code configuration was detected but the user declined migration (and there were no other changes to apply), runok.yml was still silently created with boilerplate content. This was confusing because the user said "no" but a file was created anyway. Now runok.yml is only created when: - No Claude Code config is detected (normal init flow) - The user approves changes (migration and/or hook)
The conditional logic in setup_scope had been patched ad-hoc, causing inconsistent behavior across different combinations of settings.json existence, Bash permissions, hook state, scope, and user responses. A comprehensive pattern table with 23 valid combinations was defined to specify the exact expected behavior for each case. - Replace ad-hoc integration tests with 23 exhaustive pattern tests covering every valid combination of the 6 condition axes - Fix setup_scope to always ask "Apply these changes?" when inside the "Detected" block, including runok.yml creation in the diff preview - Update unit test for project scope migration-declined case to match the new behavior (boilerplate created when Apply? defaults to yes)
The pattern table comment used "-" for inapplicable fields, which was ambiguous (could mean "no change" or "not shown"). Explicit N/A markers and State/Response/Result column grouping make it clear which prompts are shown and which are skipped for each combination.
…onfig axis The previous test suite missed the "existing runok.yml" axis, which affects whether declined changes should preserve the file or leave it absent. Added EXISTING_CONFIG constant, setup_existing_config helper, and ExpectedConfig::Preserved variant to cover all combinations of the 7 condition axes (settings.json, Bash perms, hook, scope, existing config, migration response, apply response). Co-Authored-By: Claude Opus 4.6 <[email protected]>
The abbreviated "E" was unclear. Use the full name and align all column widths consistently. Co-Authored-By: Claude Opus 4.6 <[email protected]>
When no Claude Code configuration is detected and runok.yml already exists, silently overwriting it was surprising. Add an "Overwrite?" confirmation prompt (default=No) so users must explicitly opt in. Also fix AutoYesPrompter to always return true for confirm(), matching the -y flag semantics (previously it returned the default value, which caused -y to decline overwrite since the default was false). Expand the integration test matrix from 46 to 51 patterns to cover the new Overwrite? yes/no axis for all "no Detected block + existing config" cases. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The `runok init` subcommand was added but documentation was not updated. Add CLI reference page for `runok init`, update Quick Start to recommend it as the primary setup method, add a tip to Claude Code integration page, and update CLI overview and README. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #152 +/- ##
==========================================
- Coverage 89.86% 89.66% -0.20%
==========================================
Files 31 37 +6
Lines 6425 7115 +690
==========================================
+ Hits 5774 6380 +606
- Misses 651 735 +84
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the onboarding experience for Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
On Linux CI, XDG_CONFIG_HOME may be set to a path outside the test's isolated HOME directory, causing config_dir() to write runok.yml to the wrong location. Clear XDG_CONFIG_HOME and XDG_CACHE_HOME in the test command builder so config_dir() falls back to $HOME/.config. Co-Authored-By: Claude Opus 4.6 <[email protected]>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Permission patterns containing single quotes (e.g., `echo 'hello'`) produced invalid YAML when embedded in single-quoted strings. Double the single quotes per YAML spec to prevent injection. Also replace embedded `\n` with `indoc!` in `normalize_json` test. Co-Authored-By: Claude Opus 4.6 <[email protected]>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Replace inline `\n` with `concat!` for readability while preserving the leading 2-space indent that `indoc!` auto-dedent would strip. Co-Authored-By: Claude Opus 4.6 <[email protected]>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…anup, and tests - entry_has_runok_hook now detects string format hooks (plain strings and string arrays inside hooks objects) in addition to the existing object and legacy formats - remove_permissions now cleans both settings.json and settings.local.json, not just settings.json - Parameterize 51 exhaustive integration tests into a single rstest with #[case] variants, reducing duplication while preserving coverage Co-Authored-By: Claude Opus 4.6 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
What
runok initsubcommand with an interactive setup wizardrunok.ymlgeneration in a single flow