Skip to content

feat(factory): manage Droid settings from dotfiles - #2258

Merged
shunkakinoki merged 3 commits into
mainfrom
codex/factory-droid
Aug 5, 2026
Merged

feat(factory): manage Droid settings from dotfiles#2258
shunkakinoki merged 3 commits into
mainfrom
codex/factory-droid

Conversation

@shunkakinoki

@shunkakinoki shunkakinoki commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Summary

  • manage Factory Droid model selection through models.json hydration
  • install shared security, push, GitHub-settings, secret, and git-ai hooks
  • remove legacy Droid hook entries during activation while preserving unrelated Factory settings

Validation

  • jq JSON validation
  • Bash syntax and ShellCheck
  • fresh and legacy Factory activation merge tests
  • git diff --check
  • make build

Summary by cubic

Manage Factory Droid model and guardrail hooks from dotfiles, merging them into Factory settings without overwriting user preferences. Adds a local Gemma preset, sets it as default, and makes activation idempotent while cleaning legacy entries and writing atomically.

  • New Features
    • Merge managed model and hooks into ~/.factory/settings.json with jq, preserving other settings and removing legacy Droid hooks, duplicate managed hooks, and stale importedClaudeHooks refs.
    • Add a local Gemma model preset and set it as default; values sourced from models.json via config/factory/settings.tpl.json and hydrated by scripts/llm-update.sh.
    • Install shared guardrails: security, block git push, block GitHub settings, secret scan, and git-ai checkpoints on PreToolUse/PostToolUse for write/edit with correct Execute/Edit matchers.
    • Run the merge during Home Manager activation via config/factory/default.nix using activate-settings.sh; idempotent, uses a temp file, and sets chmod 600 for safer writes.

Written for commit c55ea57. Summary will update on new commits.

Review in cubic

@indent-zero

indent-zero Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor
PR Summary

Brings Factory Droid's ~/.factory/settings.json under dotfiles management without clobbering Factory-owned preferences, wiring in the shared Codex/Claude/Cursor guardrail hooks plus a managed local Gemma3:4b Ollama model. Uses a Home Manager activation that jq-merges managed values into whatever Factory has persisted, and extends security.sh's allow-list to recognize Droid's Execute tool. Follow-up commits f69ab1f and c55ea57 harden the activation script.

  • Added config/factory/activate-settings.sh: jq-based merger that dedupes the managed custom model by id and scopes hook cleanup to exact (matcher, command) pairs via is_managed_hook_command, with a is_legacy_droid_command substring fallback for droid-hook.sh.
  • Follow-up f69ab1f: temp file created inside $FACTORY_DIR for an atomic same-fs rename, chmod tightened from 644 to 600, cleanup trap guards against empty TEMP_SETTINGS.
  • Follow-up c55ea57: new is_home_command helper matches both the literal $HOME/… and the env.HOME-expanded form, so cleanup remains idempotent across Factory re-serializations; final reduce step defensively coerces non-array hook values to [] instead of erroring.
  • Rewrote config/factory/default.nix to take {config, lib, pkgs, ...} and register home.activation.factorySettings after writeBoundary.
  • Added config/factory/settings.json + settings.tpl.json declaring the Gemma3:4b Ollama custom model and PreToolUse/PostToolUse hook groups for Execute and Edit|Write|Create|ApplyPatch, including git-ai checkpoint droid pre/post checkpoints.
  • Registered the new template in scripts/llm-update.sh and added gemma-local: gemma3:4b to models.json.
  • Widened config/shared/hooks/security.sh tool_name allow-list to include Execute|execute and updated header comments/README across shared hooks to name Factory Droid.

Issues

All clear! No issues remaining. 🎉

4 issues already resolved
  • chmod 644 "$SETTINGS" makes ~/.factory/settings.json world-readable, which diverges from the sibling config/handy/hydrate.sh pattern that uses install -m 0600; Factory may later persist real API keys for user-added custom models, so 0600 is safer and matches existing conventions. (fixed by commit f69ab1f)
  • The final reduce runs ((.hooks[$entry.key] // []) | map(clean_hook_group)), which only defaults null; if a future Factory build ever stores a hook key as an object or other non-array, jq will throw Cannot iterate over object and the entire home-manager activation aborts — guard with if type == "array" then . else [] end like the earlier with_entries pass does. (fixed by commit c55ea57)
  • TEMP_SETTINGS=$(mktemp) writes to $TMPDIR (usually /tmp), so the final mv -f "$TEMP_SETTINGS" "$SETTINGS" degrades to copy+unlink across filesystems and Factory can observe a partially written ~/.factory/settings.json if it reads concurrently — use mktemp "$FACTORY_DIR/.settings.XXXXXX" (creating $FACTORY_DIR first) to keep the rename atomic. (fixed by commit f69ab1f)
  • Cleanup now uses exact command == "$HOME/…" equality, so any Factory rewrite that expands $HOME (or otherwise reformats the string) will fail every match — including the git-ai checkpoint droid line that used to be caught by a substring — and each home-manager switch will now double both PreToolUse and PostToolUse managed groups (reproduced locally: pre 2→4, post 1→2 after one re-run with $HOME expanded). (fixed by commit c55ea57)

CI Checks

Waiting for CI checks...

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added Factory Droid settings with a local Gemma model and configurable session defaults.
    • Added safeguards for shell commands, file edits, Git pushes, GitHub settings changes, and secret handling.
    • Added automatic generation and activation of Factory Droid settings.
  • Documentation

    • Documented Factory Droid hook matching and shared guardrail support.
  • Bug Fixes

    • Extended security checks to recognize Factory Droid command execution.

Walkthrough

Adds generated Factory Droid settings with a local Gemma model and tool hooks. Home Manager merges these settings into existing Factory configuration. Shared hooks now document and recognize Factory Droid operations.

Changes

Factory Droid settings

Layer / File(s) Summary
Define and generate Factory settings
config/factory/settings.tpl.json, config/factory/settings.json, models.json, scripts/llm-update.sh
Defines the local Gemma model, session defaults, lifecycle hooks, and template-to-output generation mapping.
Merge settings during Home Manager activation
config/factory/activate-settings.sh, config/factory/default.nix
Merges managed settings with existing Factory settings, removes stale managed hooks, and atomically installs the result with fixed permissions.
Extend shared hook matching
config/shared/hooks/README.md, config/shared/hooks/block-gh-settings.sh, config/shared/hooks/block-git-push.sh, config/shared/hooks/secret-guard.sh, config/shared/hooks/security.sh
Documents Factory Droid matchers and adds Factory Droid shell tool names to shared hook support.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant HomeManager
  participant FactoryActivation
  participant ActivateSettings
  participant jq
  participant FactorySettings
  HomeManager->>FactoryActivation: run activation after writeBoundary
  FactoryActivation->>ActivateSettings: pass managed settings and jq
  ActivateSettings->>jq: merge settings and filter managed hooks
  jq-->>ActivateSettings: return staged JSON
  ActivateSettings->>FactorySettings: atomically replace settings.json
Loading

Possibly related PRs

Poem

A rabbit sees settings hop into place,
Gemma models spring from a local space.
Hooks guard commands, edits, and git,
Factory Droid now knows each bit.
jq tucks the config in tight—
Clean, atomic, and just right.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: managing Factory Droid settings from dotfiles.
Description check ✅ Passed The description directly explains model hydration, shared hooks, legacy entry cleanup, activation behavior, and validation.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/factory-droid

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@config/factory/activate-settings.sh`:
- Line 10: Update the temporary-file creation in activate-settings.sh to place
TEMP_SETTINGS inside FACTORY_DIR rather than relying on mktemp’s default
directory. Ensure the temporary path used by the subsequent activation and mv
operations is within the same filesystem as settings.json, preserving atomic
replacement.
- Around line 21-30: Update is_managed_droid_command and clean_hook_group to
remove only exact managed matcher-and-command pairs, rather than every command
under config/shared/hooks/. Preserve unrelated Factory hook groups, including
entries sharing a command path but using different matchers or arguments, while
retaining cleanup of the two managed matcher groups.
- Around line 73-75: Update the settings-file permission in the activation flow
after moving "$TEMP_SETTINGS" to "$SETTINGS" so ~/.factory/settings.json is set
to mode 600 instead of 644, keeping API keys unreadable by other local users.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 913205f7-b23e-48e3-b2c5-4627ebd9f9a6

📥 Commits

Reviewing files that changed from the base of the PR and between 3213b1d and 7cadc46.

📒 Files selected for processing (11)
  • config/factory/activate-settings.sh
  • config/factory/default.nix
  • config/factory/settings.json
  • config/factory/settings.tpl.json
  • config/shared/hooks/README.md
  • config/shared/hooks/block-gh-settings.sh
  • config/shared/hooks/block-git-push.sh
  • config/shared/hooks/secret-guard.sh
  • config/shared/hooks/security.sh
  • models.json
  • scripts/llm-update.sh

Comment thread config/factory/activate-settings.sh Outdated
Comment thread config/factory/activate-settings.sh Outdated
Comment thread config/factory/activate-settings.sh Outdated
Comment thread config/factory/activate-settings.sh Outdated
Comment thread config/factory/activate-settings.sh Outdated
@shunkakinoki

Copy link
Copy Markdown
Owner Author

Addressed in the follow-up commit $(git rev-parse --short HEAD):

  • temporary settings files are created under ~/.factory for same-filesystem atomic replacement
  • managed hook cleanup now matches exact matcher/command pairs, preserving unrelated groups and arguments; legacy droid-hook.sh entries are still removed
  • settings permissions are now mode 600

Validated with Bash syntax, ShellCheck, jq checks, fresh/legacy activation tests, and git diff --check.

@shunkakinoki

Copy link
Copy Markdown
Owner Author

Follow-up commit c55ea572 also addresses the refreshed review findings:

  • non-array existing hook keys are treated as empty before adding managed groups
  • cleanup recognizes both literal $HOME/... commands and the expanded current home path, so activation remains idempotent

Added expanded-path and non-array-hook regression checks; prior mode, atomic-temp, legacy-cleanup, ShellCheck, jq, and syntax checks remain green.

@shunkakinoki
shunkakinoki enabled auto-merge (squash) August 5, 2026 04:06
@shunkakinoki
shunkakinoki merged commit 1d780d8 into main Aug 5, 2026
5 checks passed
@shunkakinoki
shunkakinoki deleted the codex/factory-droid branch August 5, 2026 04:11
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.

1 participant