Skip to content

feat(acp): add BUZZ_GIT_IDENTITY switch for agent commit identity - #8024

Merged
wpfleger96 merged 4 commits into
mainfrom
duncan/git-identity-mode
Oct 1, 2026
Merged

wpfleger96 merged 4 commits into
mainfrom
duncan/git-identity-mode

Conversation

@wpfleger96

@wpfleger96 wpfleger96 commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

🤖 This adds an operator switch for whose identity an agent's commits carry, and removes the reason agents were clearing their git identity env vars. Replaces #6177.

Since #7819, every agent authors and signs commits as its nostr identity through GIT_CONFIG_* env vars, which beat repo and global config in every shell the agent runs. Operators had no way to opt out of that.

BUZZ_GIT_IDENTITY=agent|user. It's read once in GitEnvironment::install. Unset or agent keeps today's behavior. user keeps only relay git auth (the credential helper and keyfile) and installs no identity, no git-sign-nostr and no signing config, so git uses the operator's own config. It also drops inherited GIT_CONFIG_* entries for the agent identity and signing keys, author.*/committer.*, and include.path/includeIf.*.path, so a nested agent in user mode can't pick up its parent agent's identity. Any other value fails startup with an error naming the variable and both allowed values. The resolved mode is forwarded in the managed env to MCP servers, which buzz-agent starts with a cleared env, so a nested buzz-acp launched from an MCP shell keeps the operator's choice instead of falling back to agent.

Known limitation: in user mode, an operator whose own git config signs with git-sign-nostr has commits refused with a key-mismatch error, because the agent shell's BUZZ_PRIVATE_KEY (needed by the buzz CLI) loads ahead of nostr.keyfile and NIP-GS requires the signer to exit when the loaded key doesn't match -u. Nothing is signed with the wrong key. SSH and GPG signing are unaffected.

Desktop reserves GIT_CONFIG*. User env overrides are layered onto the spawn command after the relay credential helper, so a single GIT_CONFIG_COUNT=0 used to silently drop it. GIT_CONFIG and every GIT_CONFIG_* name are now reserved keys.

session_new_forwards_complete_git_block_without_duplicate_names no longer reads the runner's env. Run inside an agent session, it picked up the session's own inherited GIT_CONFIG_* and failed. Agents worked around that by unsetting GIT_CONFIG_COUNT, and any commit made in that shell fell back to the human's git identity and signing key. The test now hides the runner's GIT_CONFIG* and BUZZ_GIT_IDENTITY under ENV_LOCK and restores them afterwards.

Agent instructions. base_prompt.md and the nest AGENTS.md now say the runtime sets the agent's commit identity and signing, and that agents shouldn't override them with user.* config, -c user.*, --author or another signing key. Trailers are added only when the repository or the person the agent works for requires them. If a repository requires a different author, the agent tells the operator, who can switch to BUZZ_GIT_IDENTITY=user, instead of overriding the identity. The nest AGENTS.md scopes this to agents Buzz runs through its stock buzz-acp harness, and notes that custom harnesses get only relay git auth and control their own attribution. NEST_AGENTS_VERSION goes from 5 to 6.

Duncan and others added 2 commits October 1, 2026 13:04
Agents already author and sign as their nostr identity via GIT_CONFIG_*
env vars; operators had no way to opt back into their own git identity.
BUZZ_GIT_IDENTITY=user keeps only relay auth and drops inherited agent
identity so nested agents cannot inherit a parent's. Desktop now reserves
the whole GIT_CONFIG* family so a user override cannot orphan the relay
credential helper. The MCP git-block test no longer reads the runner's
inherited GIT_CONFIG_*, which pushed agents to unset those vars and
commit as the human.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Restoration after install ran as plain statements, so a panic mid-install
left the runner's GIT_CONFIG* hidden. The prompts overstated what git
config guarantees: role config and amend/cherry-pick can keep another
author.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The previous wording imposed one team's trailer convention on every
Buzz user. Trailers now follow the repository or operator, and a repo
requiring a different author is routed to the operator.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96
wpfleger96 deployed to codex-review October 1, 2026 18:28 — with GitHub Actions Active
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Oct 1, 2026
@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

🔐 Codex Security Review

Note: This is an automated, security-focused review generated by Codex.
Use it as a supplement to human review; false positives are possible.

Scope

  • Exact PR diff: 04b040ef6ce1a748fb2418340735cfc04e0fd8f4...d81fb788d3e91ecfaa1b90db3418172f99e4e5cc
  • Model: gpt-5.6-sol

💡 Click "edited" above to see earlier reviews for this PR.


Review Summary

Overall Risk: NONE

No concrete security, correctness, or reliability defects were identified in the authorized PR range.

Findings

No concrete security, correctness, or reliability findings were identified.

Notes

  • Review used read-only source and history inspection; tests and repository scripts were not executed as instructed.

Generated by Codex Security Review |
Requested by: @wpfleger96 |
Workflow run

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Two reproducible P2 blockers in the new opt-out behavior; details and required fixes are inline. Also noted one optional documentation qualification. Please address both runtime findings with process-boundary regressions before merge.

Reviewed base d56ed75424d803cf29b2f1f4f430ee81dc145f92 → head c2968c73ca12f266778d6f61bff0b34a0075eda2. Built the ACP, agent, and MCP binaries at the clean head; exercised real Git commits through native adapter and full task-harness/agent/MCP chains using disposable repositories and a deterministic local model. No live relay or Desktop app was involved. Independent Desktop/config and Git/signing review lanes completed.

Existing Rust unit/lint/Windows CI passes; broad suites were not duplicated locally. Latest snapshot still has Desktop Core running and smoke shard 2 failing on empty-edit-delete.spec.ts:51 (missing alertdialog). That UI failure is separate from these findings; its cause was not investigated.

Comment thread crates/buzz-acp/src/git.rs
Comment thread crates/buzz-acp/src/git.rs
Comment thread desktop/src-tauri/src/managed_agents/nest_agents.md Outdated
buzz-agent clears its MCP child's env, so a nested buzz-acp started from
an MCP shell lost BUZZ_GIT_IDENTITY and fell back to agent mode. The nest
AGENTS.md text now scopes the identity behavior to the stock buzz-acp
harness, since custom harnesses get relay auth only.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@github-actions github-actions Bot removed the codex-security-review-current The posted Codex security review matches its recorded range. label Oct 1, 2026
@wpfleger96
wpfleger96 deployed to codex-review October 1, 2026 19:32 — with GitHub Actions Active
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Oct 1, 2026

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Re-review: the MCP propagation blocker is fixed, and the optional custom-harness documentation correction is addressed. No new defects found in the revision.

One prior P2 remains: operator Nostr signing in user mode. I reproduced the same failure at the new head. The author explicitly deferred it and documented the limitation; that is a proposed scope exception, not a verified fix. I retain this recommendation pending a fix or Wes’s explicit acceptance of that exception. No additional code objections if the exception is accepted.

Reviewed base d56ed75424d803cf29b2f1f4f430ee81dc145f92 → head d81fb788d3e91ecfaa1b90db3418172f99e4e5cc, concentrating on the delta since c2968c73. Rebuilt the three runtime binaries at the clean head. The actual task-harness → agent → MCP → shell → nested harness chain now preserves user; both commits use the operator identity, unsigned. Unset/explicit agent modes still produce verified agent signatures. Native-adapter and inherited-config controls also passed. Independent source/test review of the fix found no new blockers.

Existing Rust CI and all four Desktop smoke shards pass; Desktop Core is still running. Broad suites were not duplicated locally. No live relay or Desktop app was exercised.

@wesbillman
wesbillman dismissed their stale review October 1, 2026 20:25

Carl, automated reviewer via Wes’s account: withdrawing my blocking recommendation. The documented, intentionally deferred Nostr-signing limitation is narrow and fail-safe; recommend treating it as non-blocking. Finding remains valid, not fixed.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Revised recommendation: non-blocking; suitable for human approval at d81fb788d3e91ecfaa1b90db3418172f99e4e5cc. My remaining finding is technically correct, but I overweighted it as a merge blocker. The intentionally deferred, documented Nostr-signing limitation affects a narrow opt-in configuration and fails closed by rejecting the commit; it does not silently sign as the wrong identity. I recommend accepting that limitation for this PR, rather than requiring signer/auth redesign here.

The MCP propagation fix remains verified, the custom-harness wording is corrected, and no other code objections remain. This supersedes my blocking recommendation; the limitation is not fixed, and I am not claiming Wes has separately accepted it. No approval is submitted on Wes’s behalf.

Head is unchanged from the process-level re-review. Current CI snapshot has no failures; Desktop Core is still running, so merge remains subject to required checks.

@wpfleger96
wpfleger96 merged commit d1b7da4 into main Oct 1, 2026
81 checks passed
@wpfleger96
wpfleger96 deleted the duncan/git-identity-mode branch October 1, 2026 21:04
wpfleger96 added a commit that referenced this pull request Oct 1, 2026
* commit '9b083957f^':
  Include thread roots in agent activity events (#8029)
  fix(relay): gate owner-only kinds in shared fan-out access filter (#8006)
  feat(acp): add BUZZ_GIT_IDENTITY switch for agent commit identity (#8024)
  fix(relay): deny channel writes when the channel lookup fails (#8007)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
ArnaudLafosse92100 added a commit to ArnaudLafosse92100/buzz that referenced this pull request Oct 2, 2026
Brings in 31 upstream commits (639593b), including ACP native-steer
frame-writer fixes (block#7568, block#8022), edited-message routing (block#4741), worker
wrapping at launch (block#7985), BUZZ_GIT_IDENTITY (block#8024), thread roots in agent
activity (block#8029) and built-in prompts without sleep polling (block#7992).

Adaptations:
- buzz-acp acp.rs: keep the fork's turn_output module alongside upstream's
  frame_writer module.
- buzz-acp pool.rs: turn_started carries both the fork's
  triggeringRootEventIds and upstream's threadRootEventId.
- buzz-acp queue.rs: drain_channel keeps upstream's withheld-steer reaction
  collection and still clears the cancelled-root tombstones.
- buzz-acp queue.rs: task_root_event_id is now edit-aware, so the 1h
  cancelled-root tombstone also drops edits routed into a stopped tree.
- buzz-acp base_prompt.md: keep the fork's empty-final-answer rule for bare
  acknowledgements, take upstream's handoff wording and no-sleep guidance.
- buzz-acp tests: new `edit` field on fork-only QueuedEvent/BatchEvent tests.
- desktop channels.rs: keep has_active_non_starter_channel guard inside
  upstream's ensure_starter_channels_inner.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Arnoldinh0 <arnaudlafosse92100@gmail.com>

This branch was successfully deployed

1 active deployment
codex-review — d81fb788 Deployed Oct 1, 2026 by wpfleger96 via Run Codex Security Review #6438
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex-security-review-current The posted Codex security review matches its recorded range.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants