Add Factory Droid agent-hook support#907
Conversation
roborev: Combined Review (
|
|
Resolved in Added coverage for Validation run:
|
roborev: Combined Review (
|
|
working on this |
|
Posted by Codex on Wes's behalf. I pushed a refactor/fix pass to this PR, preserving your latest commit Could you please test this against a real Factory Droid install? I do not have Droid available locally. The specific smoke test I want is:
Please report back with the Factory Droid version/context you tested and any mismatch in hook payload shape or output JSON. |
roborev: Combined Review (
|
wesm
changed the title
roborev: Combined Review (
|
roborev: Combined Review (
|
|
looking at this |
Introduce roborev droid-hook run, a Droid-native entrypoint that delegates to the shared internal/agenthook state machine and daemon so Factory Droid behaves on par with the existing Codex/Claude agent-hook integration. - internal/config: add [droid_hook] section mirroring [agent_hook] with Droid defaults (turn=5, commit=0, failed_review=4, Droid-flavored instruction). - internal/droidhook: ResolveOptions reading [droid_hook] + ROBOREV_DROID_HOOK_* env overrides, producing agenthook.Options for the shared daemon. - cmd/roborev: extract shared runHook helper from runAgentHook; add the droid-hook run subcommand; register droid-hook on the root command. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Add roborev droid-hook install and dump, which write and preview the Factory Droid hooks.json entries (PreToolUse/PostToolUse on Execute, matcher-less Stop) mirroring the agent-hook install for Codex/Claude. Factory's hooks.json uses the same JSON shape as Codex/Claude, so the install logic is shared rather than duplicated. - internal/agenthook: export a runner-parameterized install API (InstallSpec, InstallSpecs, PlanSpecs, MarshalJSONConfig, ResolveHookCommandWithRunner, TranslateBinaryNotice) so any integration sharing the JSON hook format can reuse it. The runner string threads through the upsert chain so stale-command detection is scoped per integration (agent-hook run vs droid-hook run are disjoint and never clobber each other). - internal/droidhook: install.go with DroidSpecs, RunInstall/RunDump, user/project scope resolution (~/.factory/hooks.json or .factory/hooks.json), and the droid-hook run command suffix. - cmd/roborev: droid-hook install/dump subcommands with --command, --binary (install only), --config, --scope, --timeout, --dry-run (install only). Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Ship the full roborev skill set for Factory Droid alongside the existing Claude Code and Codex sets, so Droid users get the same review/fix/refine/respond workflows. Droid skills install to ~/.factory/skills (Factory's personal skills location) and are skipped when ~/.factory is absent, so the install stays opt-in for Factory users. The Droid skills are derived from the Codex skills (agent-agnostic: synchronous --wait, no Claude-specific Task tool) with two Factory-specific adaptations: slash invocation (/roborev-X, matching Factory's /skill-name convention) and AGENTS.md (Factory's project-instructions file) instead of the Codex $roborev-X and CLAUDE.md forms. - internal/skills: add AgentDroid spec (.factory config dir, droid embed), embed the 7 droid SKILL.md files, and extend skills_test.go (Droid in agentCases, createMockSkill resolves config dir per agent, IsInstalled coverage, and guards for the Droid adaptations + install location). - cmd/roborev: surface Droid in the skills status/install display, the quickstart skills check, and the post-update skill refresh trigger. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Complete the Factory Droid hook integration with session inspection commands
and user-facing documentation, and validate the full path end-to-end.
- cmd/roborev: add droid-hook status and reset, which delegate to the shared
agenthook daemon (session state is integration-agnostic, keyed by session_id),
plus a subcommand registration test.
- docs: add docs/droid-hook.md (the Factory Droid counterpart to agent-hook.md,
covering the loop, quick start, runtime model, configuration, and session
inspection), and surface droid-hook in README.md, docs/commands.md, and
docs/index.md.
End-to-end validation (built binary, isolated data dir): droid-hook dump emits
the Factory hooks.json shape (PreToolUse/PostToolUse on Execute + Stop);
install --dry-run auto-resolves the binary and reports the target path; run
reaches the shared daemon, records sessions (count increments per Stop, repo
HEAD and branch resolved from cwd), and fails open ({}) when not triggered;
status reports session counters. The block-when-triggered path is covered by
the Phase 1 unit test.
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Allow the shared agenthook recorder to treat Factory Droid Execute events as shell tool events so Droid PreToolUse/PostToolUse hooks seed commit baselines and count commits like Codex/Claude Bash hooks. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Factory Droid was added as a parallel droid-hook command even though its runtime path already reused the agenthook daemon and state machine. Keeping a second unshipped hook surface would make the CLI and package boundaries look more different than the behavior really is. This makes Droid a third agent-hook profile instead: installs write Factory hooks that invoke agent-hook run --agent droid, while Droid-specific config, env vars, scope handling, and the Execute matcher stay explicit under agenthook. Commit tracking now treats Execute as the Droid shell-command tool so commit-threshold reminders can work through the shared state path. Generated with Codex Co-authored-by: Codex <codex@openai.com>
The Droid hook command surface was folded into agent-hook, but some installs from this unmerged branch may already have Factory hooks that invoke droid-hook run. Leaving those commands in hooks.json would keep calling a deleted command on every Droid hook event. Treat the legacy Droid runner as replaceable only when planning the Droid agent-hook install, and tighten runner matching so the broader Codex/Claude agent-hook runner does not claim agent-hook run --agent droid entries. Generated with Codex Co-authored-by: Codex <codex@openai.com>
This reverts commit 9a2b76a.
Droid now uses the shared agent-hook command with a longer runner suffix, agent-hook run --agent droid. The stale-command matcher still used substring matching, so a plain Codex or Claude install could mistake that Droid runner for its own agent-hook run entry and replace it in shared hook files. Match the runner boundary instead of any substring so plain agent-hook installs and Droid profile installs remain disjoint. This intentionally does not add any legacy droid-hook migration behavior. Generated with Codex Co-authored-by: Codex <codex@openai.com>
The runner matcher needs to do two things at once: keep plain agent-hook installs from claiming the Droid profile, and still recognize valid stale hook commands that include supported run flags. Matching only end-of-string or quotes fixed the first case but rejected documented flags such as --turn-threshold and --config. Accept whitespace-delimited runner suffix flags, while treating --agent droid as a different profile when matching the plain agent-hook runner. This keeps stale binary-path replacement working without collapsing Droid and non-Droid hook entries together. Generated with Codex Co-authored-by: Codex <codex@openai.com>
Droid hook installs use the shared agent-hook run command, but the Droid profile is selected by flags rather than a distinct subcommand. Exact matching on agent-hook run --agent droid missed valid commands where flags appeared in a different order or used --agent=droid, leaving stale binary-path hook commands active after reinstall. Classify the parsed run suffix for plain and Droid profiles so stale hook replacement accepts supported flag forms while keeping Codex/Claude installs from claiming Droid entries. This intentionally keeps legacy droid-hook migration behavior out of the branch because that command never shipped. Generated with Codex Co-authored-by: Codex <codex@openai.com>
Add roborev-lookahead-review and roborev-lookahead-review-branch Droid skills to match the Claude/Codex skill sets added in kenn-io#904, resolving CI test failures where Droid was expected to have 9 skills but only had 7. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
The Droid lookahead skills are Droid-only entries, but invoking them failed because review --type only accepted security and design. Treat lookahead as an explicit review type with its own temporal-leakage prompt while reusing the normal review workflow config for agent/model selection. Track supported agents for each embedded skill so roborev skills status does not report Droid-only skills as missing from Claude or Codex installations. Generated with Codex Co-authored-by: Codex <codex@openai.com>
roborev: Combined Review (
|
After rebasing over the commit that introduced the lookahead review type, the Droid branch needs to follow the upstream workflow mapping and embedded skill coverage instead of carrying pre-rebase assumptions. Keep lookahead using its own workflow key, remove duplicate prompt fallback code, and make the skill metadata expectations match the now-shared Claude/Codex/Droid lookahead skills. Generated with Codex Co-authored-by: Codex <codex@openai.com>
Project-scoped Factory Droid hooks live in repo-local .factory/hooks.json, which is executable configuration a PR can modify. Keeping roborev's installer and docs on that path would make a shareable project config look safe when it can run branch-controlled commands in a reviewer's local agent session. Keep the Droid integration user-scoped only, reject project scope for install and dump, and document why repo-local Factory hook commands should not be committed. Validation: added project-scope rejection tests and watched them fail before the implementation. Generated with Codex Co-authored-by: Codex <codex@openai.com>
Droid project scope was rejected, but a caller could still point --config at .factory/hooks.json and create the same repo-local executable hook file. Validate explicit Droid config paths before install or dump so the user-scoped-only rule cannot be bypassed by spelling the project hook path directly. Validation: added --config .factory/hooks.json rejection tests and watched them fail before the implementation. Generated with Codex Co-authored-by: Codex <codex@openai.com>
The project-path guard must not reject the legitimate user-scoped Factory hooks file when a user runs the command from their home directory. Check the resolved user hook path first, then reject only non-user targets that resolve to the current project's .factory/hooks.json. Validation: added cwd-equals-HOME user-scope tests and watched them fail before the implementation. Generated with Codex Co-authored-by: Codex <codex@openai.com>
roborev: Combined Review (
|
Checking only the current directory left project hook writes reachable from repo subdirectories. Resolve the active git repo root and reject that root's .factory/hooks.json for Droid install and dump, while still allowing the resolved user-scoped Factory hooks path. Validation: added subdirectory relative and absolute repo-root config path tests and watched them fail before the implementation. Generated with Codex Co-authored-by: Codex <codex@openai.com>
A caller did not have to run from inside the target repository to write its .factory/hooks.json. Inspect the supplied config path itself: when it points at a git repository root's Factory hook file, reject it unless the path was already recognized as the user-scoped Factory hooks file. Validation: added outside-repo relative and absolute target config path tests and watched them fail before the implementation. Generated with Codex Co-authored-by: Codex <codex@openai.com>
roborev: Combined Review (
|
Check HOME env var before os.UserHomeDir() so tests that override HOME work on Windows, where os.UserHomeDir returns USERPROFILE instead of HOME. This fixes the three Windows-only test failures: TestDefaultDroidHooksPath, TestRunInstallDroidAllowsUserScopeWhenHomeIsCWD, and TestRunDumpDroidAllowsUserScopeWhenHomeIsCWD. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
roborev: Combined Review (
|
Droid hook installs rejected direct project config paths, but an existing symlink could still resolve to a repo-local .factory/hooks.json when the writer followed it. Factory Droid skills also need the same HOME-first config home that hook installs use, otherwise hook prompts can target a Droid setup that lacks the roborev skills. Validate existing symlink targets before Droid hook writes and route Droid skill install, update, status, and installed checks through one agent-aware home resolver. Validation: added symlink-target and HOME-split regression tests and watched them fail before implementation. Generated with Codex Co-authored-by: Codex <codex@openai.com>
roborev: Combined Review (
|
validateDroidHooksPath used filepath.EvalSymlinks on the full hooks.json path, which only works when the file already exists. If a parent directory (e.g., ~/.factory) was a symlink to a repo's .factory directory and hooks.json did not exist yet, validation passed and writeJSONConfig could create a project-scoped Factory hook file via the symlink. Replace evalExistingPath with evalExistingParentPath, which walks up to the longest existing ancestor, resolves symlinks on that portion, and appends the remaining path components. This catches symlinked parents even when the target file has not been created yet. Add tests for both RunInstall and RunDump covering the symlinked-parent scenario. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
|
Resolved in Added tests for both |
roborev: Combined Review (
|
|
looking |
Some Windows and restricted test environments cannot create symlinks. The new symlinked-parent regression tests should behave like the existing symlink coverage by skipping when setup cannot create the fixture, rather than failing before exercising roborev behavior. Generated with Codex Co-authored-by: Codex <codex@openai.com>
|
Resolved in |
roborev: Combined Review (
|
|
working on this |
Droid hook path validation must follow Windows path semantics for project-scope detection. Case-sensitive string checks let mixed-case spellings of .factory/hooks.json bypass the user-scoped-only guard on case-insensitive filesystems. Route Droid path and reserved-name comparisons through a Windows-aware comparison helper and cover mixed-case relative and absolute project config paths. Validation: added mixed-case Droid project-path tests and watched them fail before implementation. Generated with Codex Co-authored-by: Codex <codex@openai.com>
roborev: Combined Review (
|
The fix skills ask agents to summarize review findings back through roborev comment. Passing that dynamic text as a quoted shell argument leaves room for shell expansion if review-derived filenames or text are copied into the summary. Match the safer refine-skill pattern by documenting quoted heredoc input for roborev comment, and keep examples from teaching inline dynamic shell arguments. Validation: added embedded fix-skill regression coverage and watched it fail before implementation. Generated with Codex Co-authored-by: Codex <codex@openai.com>
roborev: Combined Review (
|
|
accepted risk |
Windows CI can check out embedded markdown with CRLF line endings, which made the heredoc regression test fail on prose that intentionally spans two lines. Normalize the embedded skill text in the assertion path so the test verifies the security-relevant template content instead of the platform checkout style. Generated with Codex Co-authored-by: Codex <codex@openai.com>
roborev: Combined Review (
|
2 participants
Summary
roborev agent-hook --agent droidprofile, backed by the shared agent-hook daemon and state machine.~/.factory/hooks.json/.factory/hooks.json, withExecutePreToolUse/PostToolUse hooks and aStophook that invokeroborev agent-hook run --agent droid.[droid_hook]config,ROBOREV_DROID_HOOK_*env overrides, and Droid-friendly continuation instructions while sharing status/reset/daemon behavior with Codex and Claude Code..factory/skillsplus docs and command references.Testing needed
I do not have Factory Droid available locally. Please smoke test the latest PR head in a real Droid environment before merge.