Skip to content

Fix Factory fallback tool use IDs#942

Merged
gtrrz-victor merged 6 commits intomainfrom
fix-factory-tool-use-hooks
Apr 14, 2026
Merged

Fix Factory fallback tool use IDs#942
gtrrz-victor merged 6 commits intomainfrom
fix-factory-tool-use-hooks

Conversation

@gtrrz-victor
Copy link
Copy Markdown
Contributor

@gtrrz-victor gtrrz-victor commented Apr 13, 2026

Summary

Fix Factory AI Droid fallback tool_use_id handling so repeated Worker runs with identical inputs do not collide while pre/post task hooks still correlate correctly.

What Changed

  • replaced the deterministic fallback-only path with a per-invocation fallback tool_use_id generated during PreToolUse
  • added a small Factory-only pending fallback state file under .entire/tmp so PostToolUse can recover and consume the exact ID for the matching Worker invocation when Factory omits tool_use_id
  • threaded hook parsing context through the Factory lifecycle parser so the fallback state can be resolved relative to the repo tmp dir
  • added focused lifecycle coverage proving repeated identical missing-tool_use_id invocations stay unique, correlate in LIFO order, and clean up temporary state

Verification

  • go test ./cmd/entire/cli/agent/factoryaidroid
  • mise run test:e2e --agent factoryai-droid TestFactoryTaskHooksDoNotFail

fixes: #917, #930, #938

Entire-Checkpoint: aeffc337f24b
Entire-Checkpoint: cd3732d55e3f
@gtrrz-victor gtrrz-victor requested a review from a team as a code owner April 13, 2026 17:08
Copilot AI review requested due to automatic review settings April 13, 2026 17:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens Factory AI Droid worker hook parsing so Entire continues running when Droid omits tool_use_id and/or emits tool_response as either an object or a string, and adds regression coverage to prevent reintroducing these failures.

Changes:

  • Updated Factory PostToolUse parsing to accept tool_response as raw JSON (object or string) and extract agentId robustly.
  • Added a deterministic fallback tool_use_id when Droid omits it.
  • Added unit + Factory-only E2E regression tests to ensure hooks do not surface parsing errors during interactive runs.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
e2e/testutil/assertions.go Adds a helper to assert console.log does not include hook/parsing failure strings.
e2e/tests/factory_hooks_test.go Introduces a Factory-only E2E regression test validating worker hook execution doesn’t produce hook errors.
cmd/entire/cli/agent/factoryaidroid/types.go Extends hook payload types and adds deterministic fallback tool use ID hashing.
cmd/entire/cli/agent/factoryaidroid/lifecycle.go Implements fallback tool_use_id wiring and supports tool_response object-or-string parsing.
cmd/entire/cli/agent/factoryaidroid/lifecycle_test.go Adds focused lifecycle tests for missing tool_use_id and string tool_response parsing.

Entire-Checkpoint: 71e412557efe
@gtrrz-victor gtrrz-victor changed the title Fix Factory worker hook payload parsing Fix Factory fallback tool use IDs Apr 13, 2026
Fix gosec/wrapcheck lint errors in loadFallbackToolUseState and replace
forbidden cli/testutil import with inline git.PlainInit in lifecycle_test.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Entire-Checkpoint: 80de6f75ba55
@gtrrz-victor
Copy link
Copy Markdown
Contributor Author

Follow-up: fix lint and architecture test violations

  • Fixed gosec (G304) and wrapcheck lint errors in loadFallbackToolUseState
  • Replaced forbidden cli/testutil import in lifecycle_test.go with inline git.PlainInit to satisfy architecture boundary test

Avoids string allocation and handles whitespace-padded JSON correctly.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Entire-Checkpoint: 53a994da50ed
@gtrrz-victor
Copy link
Copy Markdown
Contributor Author

Follow-up: bytes-based null check in parseHookToolResponseAgentID

  • Replaced string(raw) == "null" with bytes.TrimSpace + bytes.Equal to avoid allocation and handle whitespace-padded JSON

@gtrrz-victor gtrrz-victor enabled auto-merge April 14, 2026 04:25
@gtrrz-victor gtrrz-victor merged commit a4fc002 into main Apr 14, 2026
9 checks passed
@gtrrz-victor gtrrz-victor deleted the fix-factory-tool-use-hooks branch April 14, 2026 04:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Factory Droid (factoryai-droid) hooks fail with schema mismatch on PreToolUse and PostToolUse — severely impacts Mission workflow

3 participants