Add broker commit trailer hook support - #1465
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughBroker spawn actions now accept optional commit attestation metadata. Complete metadata reaches spawned workers through environment variables and enables a temporary chained Git hook that appends attribution trailers. ChangesCommit attestation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DispatchAction
participant SpawnAction
participant Spawner
participant Worker
participant GitRepository
DispatchAction->>SpawnAction: provide optional attestation metadata
SpawnAction->>Spawner: map metadata to environment variables
Spawner->>Worker: launch with metadata and temporary hooks path
Worker->>GitRepository: create commit
GitRepository->>Worker: append attribution trailers
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@crates/broker/src/spawner.rs`:
- Around line 244-248: Preserve existing Git configuration in the environment
setup around attestation_env_present, commit_hooks_dir, and
add_broker_hooks_path. Capture the effective core.hooksPath and inherited
GIT_CONFIG_* count before applying the broker override, then chain the captured
hook path and retain inherited indexed configuration entries. Add integration
coverage for repository-configured hooksPath and inherited GIT_CONFIG_*
variables.
- Around line 55-87: Validate attestation.jti, attestation.agent_id, and
attestation.sponsor_id for CR, LF, and NUL characters before exporting them in
the environment-building function, returning unchanged env_vars when any value
is invalid. Apply the same validation in attestation_env_present so the hook is
not enabled when any exported attestation value contains these control
characters.
🪄 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: a8382708-d60d-480b-8e76-ede05e2eb46a
📒 Files selected for processing (4)
crates/broker/src/relaycast/bridge.rscrates/broker/src/spawner.rscrates/broker/src/types.rscrates/broker/src/wrap.rs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/broker/src/spawner.rs">
<violation number="1" location="crates/broker/src/spawner.rs:54">
P2: Configured hooks using `~user/...` (or bare `~`) are still skipped: Git expands these paths, but this chain treats them as repository-relative. Resolve `core.hooksPath` with Git's `--path` mode before selecting the hook directory so all supported tilde forms chain correctly.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| if [ -n "$configured_hooks_path" ]; then | ||
| case "$configured_hooks_path" in | ||
| /*) existing_hooks_dir="$configured_hooks_path" ;; | ||
| "~/"*) existing_hooks_dir="$HOME${configured_hooks_path#\~}" ;; |
There was a problem hiding this comment.
P2: Configured hooks using ~user/... (or bare ~) are still skipped: Git expands these paths, but this chain treats them as repository-relative. Resolve core.hooksPath with Git's --path mode before selecting the hook directory so all supported tilde forms chain correctly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/broker/src/spawner.rs, line 54:
<comment>Configured hooks using `~user/...` (or bare `~`) are still skipped: Git expands these paths, but this chain treats them as repository-relative. Resolve `core.hooksPath` with Git's `--path` mode before selecting the hook directory so all supported tilde forms chain correctly.</comment>
<file context>
@@ -51,6 +51,7 @@ configured_hooks_path="$(git config --get core.hooksPath 2>/dev/null || true)"
if [ -n "$configured_hooks_path" ]; then
case "$configured_hooks_path" in
/*) existing_hooks_dir="$configured_hooks_path" ;;
+ "~/"*) existing_hooks_dir="$HOME${configured_hooks_path#\~}" ;;
*) repo_root="$(git rev-parse --show-toplevel 2>/dev/null)" || exit 0
existing_hooks_dir="$repo_root/$configured_hooks_path" ;;
</file context>
1 participant
Adds optional spawn metadata for commit trailers and preserves repository-local prepare-commit-msg hooks.\n\nTests cover trailer injection, hook chaining, and the metadata-absent no-op path.\n\nReference: https://github.com/AgentWorkforce/sales/issues/27