feat(repo): host-side repo_publish, approval-gated push (#735) - #778
Conversation
|
Warning Review limit reached
Next review available in: 30 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR adds an approval-gated ChangesRepository publishing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant repo_publish
participant ApprovalRequestQueue
participant perform_effect
participant RepoManager
participant RemoteRepository
Agent->>repo_publish: submit repository and message
repo_publish->>RepoManager: stage checkout HEAD
RepoManager-->>repo_publish: return task branch and commit
repo_publish->>ApprovalRequestQueue: queue repo.publish approval
repo_publish-->>Agent: return pending publish
ApprovalRequestQueue->>perform_effect: deliver approved effect
perform_effect->>RepoManager: push approved commit
RepoManager->>RemoteRepository: push without force
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
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
added
cluster:approvals
oxoxDev
left a comment
oxoxDev
left a comment
There was a problem hiding this comment.
The architectural call is the right one and it is the part that would have been easy to get wrong: not routing this through #243's approve→re-dispatch path, because the task-scoped checkout carrying the agent's commits is purged at turn end, before the re-dispatch fires. Staging HEAD into the mirror immediately and recording an agent: None effect the runtime performs on approval — the email.send shape — means the commits are never lost, nothing is regenerated, and a denied or expired approval leaves the remote untouched. Verifying that against the runtime rather than against the issue text is what found it.
Three more things I checked and liked:
stage_publishis a fetch, not a push. Pulling the checkout'sHEADinto the mirror on a host-owned ref meansreceive-packis never invoked, so #245'spre-receiverefusal hook and its no-push contract test stay untouched rather than being weakened to accommodate a new writer. That is the difference between adding a capability and eroding a guard.- The refusals are structural, not validated. The branch is host-generated from the manager's own company id and a task segment re-validated before the push, so a prompt-injected
--force origin mainhas no code path to a ref it could reach. Confirmed the refspec isrefs/heads/{branch}:refs/heads/{branch}— no leading+, so force is not expressible rather than merely rejected. - The credential stays out of the error surface. The missing-credential path names the binding key, not the token. Worth keeping that discipline as this grows: #614 is the local precedent for a credential reaching a durable record through an error string.
Agent-attributed commits, so a published branch's git log answers which agent wrote it, is the detail that makes this auditable after the fact rather than only gated before it.
0 major. 1 sequencing item, and acting on it saves real work. Approving.
Land #758 first — you are carrying the same grant twice
grants_repo_write_explicit is not in main, is in this branch, and is in #758 — and the two conflict in src/company/mod.rs, which is the same re-export block #758 has just had hand-resolved.
They are independent branches, not a stack: feat/734-repo-write-grant is not an ancestor of feat/735-repo-publish. This branch touches every file #758 touches plus four more, so sub-issue B is carrying a copy of sub-issue A rather than building on it.
Whichever lands second re-conflicts on that block. The cheaper order is clear:
- #758 first — it is the smaller, foundational half, its conflict is already resolved, and it needs only
cargo fmt --allto clear a redRustlane. - Then rebase this branch, at which point the duplicated grant should fall out as already-present and the conflict disappears rather than needing a second hand-resolution.
Merging this one first inverts that: #758 would then conflict harder, on work it originated, for no benefit.
Before merging: base is current and the branch is MERGEABLE — but only against main, not against #758. Worth re-checking mergeable after that lands rather than treating today's green as settled; main has gone red twice this week from branches that were each green against a base lacking the other.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/spec/runtime/repos.md (1)
440-455: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the repository write-tier documentation.
docs/spec/runtime/repos.mdstill claims that no push path exists and that no tool consumesrepo.write. DocumentRepoManager::stage_publish,RepoManager::push_published, andrepo_publish. Addrepo_publishto the agent-surface table. Preserve the accuraterepo.writeandcan_pushfail-closed rules.🤖 Prompt for 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. In `@docs/spec/runtime/repos.md` around lines 440 - 455, Update the “Not in this tier” section to document the existing write path through RepoManager::stage_publish, RepoManager::push_published, and repo_publish instead of claiming no push path or consumers exist. Add repo_publish to the agent-surface table, while preserving the documented repo.write separation and can_push fail-closed behavior.Source: Learnings
🧹 Nitpick comments (5)
src/runtime/cycle.rs (1)
1234-1260: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd focused coverage for the approved-publish arm.
The layer's changed test files cover
RepoManagerstaging and pushing. This dispatch arm carries its own behavior that no listed test exercises: the kind match, therepoandbranchextraction, and theUnimplementedbranch whenrt.repos()isNone. The repository guideline for**/*.rsrequires focused tests with every behavior change.As per coding guidelines: "
**/*.rs: Add focused tests with every behavior change."🤖 Prompt for 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. In `@src/runtime/cycle.rs` around lines 1234 - 1260, Add focused tests for the approved-publish dispatch arm in the surrounding cycle test module, covering REPO_PUBLISH_EFFECT matching, repo and branch payload extraction passed to push_published, and the Unimplemented error when rt.repos() is None. Keep existing RepoManager staging/pushing tests unchanged and use the established runtime/test helpers.Source: Coding guidelines
src/harness/mod.rs (1)
5237-5248: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the declaration check cover
repo_publish.
can_push: Nonekeepsrepo_publishoff this belt, soevery_registered_tool_is_declarednever checks it againstpolicy::consequence::DECLARED. The comment at lines 5226-5231 names this exact failure mode forrepo_checkout.Set
can_push: Some(true)here and addrepo.writeto the grant list at line 5334, then addrepo_publishto the vacuity guard at lines 5344-5354.♻️ Proposed change to wire the publish tool onto the coverage belt
size_bytes: 0, bound_at_millis: 1, - can_push: None, + // Push-capable so the `repo.write` tier wires `repo_publish` + // onto this belt; otherwise the declaration check below never + // looks at it. + can_push: Some(true), }];Then widen the grant list and the vacuity guard:
( - &["workspace", "search", "media", "composio", "repo"][..], + &[ + "workspace", + "search", + "media", + "composio", + "repo", + "repo.write", + ][..], false, true, ),"describe_workflow", "repo_checkout", + "repo_publish",🤖 Prompt for 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. In `@src/harness/mod.rs` around lines 5237 - 5248, Update the RepoBinding fixture to set can_push to Some(true), add repo.write to the grant list, and include repo_publish in the vacuity guard so every_registered_tool_is_declared validates the publish tool against policy::consequence::DECLARED.src/runtime/repo_manager.rs (1)
631-644: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the discarded re-probe error.
Line 640 uses
.ok(), so a probe failure is dropped without a trace. The capability then stays unknown and the write tier stays unwired, but the only operator-visible message is thebuild_agentwarning "no bound repository has a push-capable credential". That message names the wrong cause.Add a debug or warn line on the failure branch so an operator can tell a read-only credential apart from a probe that never answered.
♻️ Proposed change to record the probe failure
match (self.host.as_ref(), token.as_deref()) { (Some(host), Some(tok)) => { let coords = RepoCoordinates { owner: binding.owner.clone(), repo: binding.repo.clone(), }; - host.repo_meta(&coords, tok).await.ok().map(|m| m.can_push) + match host.repo_meta(&coords, tok).await { + Ok(meta) => Some(meta.can_push), + Err(err) => { + tracing::debug!( + company = %self.company, + key, + error = %err, + "[repo] could not probe push capability; it stays unknown \ + (cannot-push) and the next fetch retries" + ); + None + } + } } _ => None, }🤖 Prompt for 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. In `@src/runtime/repo_manager.rs` around lines 631 - 644, Update the reprobed_push logic in RepoManager to preserve the existing successful capability mapping while explicitly logging repo_meta probe failures instead of discarding them with .ok(). Add a debug or warn message on the error branch that distinguishes a failed probe from a confirmed read-only credential.src/harness/repo/test.rs (1)
934-943: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the staged commit, not just the ref's existence.
git_atpanics on a non-zero exit, sorev-parseon a missing ref already fails the test. The!staged.is_empty()assertion at line 994 therefore adds nothing. It also does not prove the branch points at the agent's commit.Return the HEAD SHA from
committed_checkoutand compare against it.src/runtime/repo_manager/test.rsline 1042 uses this stronger form.💚 Proposed change to assert the staged commit
-async fn committed_checkout(ctx: &RepoToolContext, mirror: &Path, key: &str) { +/// Returns the checkout's HEAD SHA, so a caller can assert the staged branch +/// points at the agent's commit rather than merely existing. +async fn committed_checkout(ctx: &RepoToolContext, mirror: &Path, key: &str) -> String { let dest = ctx.workspace.join(CHECKOUT_SUBDIR).join(key); materialize(mirror, &dest, Some("main"), None) .await .expect("materialize"); identify(&dest); std::fs::write(dest.join("FIX.md"), "the fix\n").unwrap(); git_at(&dest, &["add", "FIX.md"]); git_at(&dest, &["commit", "--quiet", "-m", "the fix"]); + git_at(&dest, &["rev-parse", "HEAD"]) }- committed_checkout(&ctx, &mirror, &binding.key).await; + let head = committed_checkout(&ctx, &mirror, &binding.key).await;let staged = git_at(&mirror, &["rev-parse", "refs/heads/oc/acme/card-1"]); - assert!(!staged.is_empty(), "the mirror carries the staged branch"); + assert_eq!( + staged, head, + "the staged branch must point at the agent's commit" + );The no-task test at line 952 does not bind the return value, so it needs no change.
Also applies to: 992-994
🤖 Prompt for 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. In `@src/harness/repo/test.rs` around lines 934 - 943, Update committed_checkout to return the SHA of the commit created after committing FIX.md, then capture that return value in the applicable test and compare the staged branch’s resolved commit against it. Remove the redundant non-empty staged assertion; preserve the no-task test’s unbound return behavior.src/runtime/repo_manager/test.rs (1)
1081-1111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for a binding with no stored credential.
push_publishedrefuses withInvalidRequestwhentoken_forreturnsNone(src/runtime/repo_manager.rs lines 863-867). No test covers that branch. Abind_localbinding starts with no credential, so the case costs one short test.Add it beside this table so a future change cannot make a credential-less binding attempt a push.
💚 Proposed test for the missing-credential refusal
/// A binding with no stored credential cannot publish: the refusal happens /// before git runs, rather than failing inside an unauthenticated push. #[tokio::test] async fn a_push_without_a_stored_credential_is_refused() { let scratch = Scratch::new("no-credential-push"); let url = fixture_remote(&scratch); let (mgr, _secrets) = manager(&scratch); // `bind_local` stores no token at all. mgr.bind_local(&url, "fixture", vec!["main".into()]) .await .unwrap(); let checkout = scratch.join("checkout"); let mirror = mgr.mirror_path("fixture"); checkout_with_commit(&scratch, &mirror, &checkout, "FIX.md", "the fix\n"); let branch = mgr .stage_publish("fixture", &checkout, "task-1") .await .unwrap(); let err = mgr.push_published("fixture", &branch).await.unwrap_err(); assert!( matches!(err, OpenCompanyError::InvalidRequest(_)), "a credential-less binding must be refused before the push: {err:?}" ); }🤖 Prompt for 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. In `@src/runtime/repo_manager/test.rs` around lines 1081 - 1111, Add a neighboring async test for the credential-less binding path, using bind_local without setting a secret, then create a staged publish via checkout_with_commit and stage_publish before calling push_published. Assert it returns OpenCompanyError::InvalidRequest, confirming the push is refused before git runs.
🤖 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 `@src/harness/build.rs`:
- Around line 527-543: Update RepoPublishTool::execute to validate the selected
binding’s can_push value is Some(true) before calling stage_publish, rejecting
read-only bindings before staging or approval creation. Preserve existing
behavior for push-capable bindings, and add a test covering mixed push-capable
and read-only bindings.
In `@src/harness/repo.rs`:
- Around line 998-1045: Update the publish flow around stage_publish and
perform_effect so staging returns the exact commit HEAD, includes that commit
identifier in the approval effect payload, and the later push validates and
sends exactly the approved commit rather than the current mirror ref. Preserve
force-update staging while preventing restaging or descendant changes from
altering an existing approval, and add a focused regression test covering this
scenario.
In `@src/runtime/cycle.rs`:
- Line 1240: Update the REPO_PUBLISH_EFFECT reference in the cycle effect branch
so the default build does not depend on the feature-gated crate::harness::repo
module: either move the constant to an always-compiled module and reference it
there, or gate the branch with #[cfg(feature = "openhuman")].
---
Outside diff comments:
In `@docs/spec/runtime/repos.md`:
- Around line 440-455: Update the “Not in this tier” section to document the
existing write path through RepoManager::stage_publish,
RepoManager::push_published, and repo_publish instead of claiming no push path
or consumers exist. Add repo_publish to the agent-surface table, while
preserving the documented repo.write separation and can_push fail-closed
behavior.
---
Nitpick comments:
In `@src/harness/mod.rs`:
- Around line 5237-5248: Update the RepoBinding fixture to set can_push to
Some(true), add repo.write to the grant list, and include repo_publish in the
vacuity guard so every_registered_tool_is_declared validates the publish tool
against policy::consequence::DECLARED.
In `@src/harness/repo/test.rs`:
- Around line 934-943: Update committed_checkout to return the SHA of the commit
created after committing FIX.md, then capture that return value in the
applicable test and compare the staged branch’s resolved commit against it.
Remove the redundant non-empty staged assertion; preserve the no-task test’s
unbound return behavior.
In `@src/runtime/cycle.rs`:
- Around line 1234-1260: Add focused tests for the approved-publish dispatch arm
in the surrounding cycle test module, covering REPO_PUBLISH_EFFECT matching,
repo and branch payload extraction passed to push_published, and the
Unimplemented error when rt.repos() is None. Keep existing RepoManager
staging/pushing tests unchanged and use the established runtime/test helpers.
In `@src/runtime/repo_manager.rs`:
- Around line 631-644: Update the reprobed_push logic in RepoManager to preserve
the existing successful capability mapping while explicitly logging repo_meta
probe failures instead of discarding them with .ok(). Add a debug or warn
message on the error branch that distinguishes a failed probe from a confirmed
read-only credential.
In `@src/runtime/repo_manager/test.rs`:
- Around line 1081-1111: Add a neighboring async test for the credential-less
binding path, using bind_local without setting a secret, then create a staged
publish via checkout_with_commit and stage_publish before calling
push_published. Assert it returns OpenCompanyError::InvalidRequest, confirming
the push is refused before git runs.
🪄 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: 8f5da330-2820-42e8-805f-005710b5fb62
📒 Files selected for processing (15)
docs/spec/runtime/repos.mdsrc/company/mod.rssrc/company/types.rssrc/harness/brain.rssrc/harness/build.rssrc/harness/mod.rssrc/harness/repo.rssrc/harness/repo/test.rssrc/policy/consequence.rssrc/runtime/cycle.rssrc/runtime/repo_manager.rssrc/runtime/repo_manager/github.rssrc/runtime/repo_manager/test.rssrc/runtime/repo_manager/types.rssrc/server/ops/repos.rs
Reconcile the write tier with two changes that landed on main: - tinyhumansai#758 (tinyhumansai#734) merged, so the diff shrinks to just the tinyhumansai#735 changes. - tinyhumansai#752 added a fourth wiring gate: repo tools are refused on a plaintext secret backend, where the credential is readable by the agent shell. Both features added a "fourth gate" test helper; unified them into one built_tool_names_with_repos_full(grants, bindings, push_capable, storage_kind) with the two convenience wrappers on top. Also extend the tinyhumansai#752 gate to the write tier: repo_publish uses the credential host-side to push, so it is refused on a plaintext backend exactly like the read tools — added the gate and a test. Submodule pointers taken from origin/main (this branch touches no vendored code). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
coderabbitai
Bot
removed
cluster:approvals
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/runtime/cycle.rs (1)
1251-1277: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd an effect-execution test.
src/runtime/repo_manager/test.rstests repository staging and pushing, but it does not executeperform_effect. Add a focused test that approvesrepo.publish, verifies the exactrepo,branch, andheadreach the repository manager, and verifies that an absent manager returnsUnimplemented.As per coding guidelines, “
**/*.rs: Add focused tests with every behavior change.”🤖 Prompt for 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. In `@src/runtime/cycle.rs` around lines 1251 - 1277, Add a focused test covering the REPO_PUBLISH_EFFECT branch in perform_effect: approve a repo.publish effect, assert the repository manager receives the exact repo, branch, and head values, and assert execution without a configured manager returns OpenCompanyError::Unimplemented. Place it with the existing repository manager tests and reuse their test helpers or mock manager setup.Source: Coding guidelines
src/runtime/repo_manager.rs (1)
893-925: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAuthorization Bypass (CWE-367): Time-of-check Time-of-use (TOCTOU) Race Condition
Reachability: Internal
Serialize
stage_publishper publish branch.
stage_publishupdates the mirror ref and reads it in separate awaits. Concurrent calls for the same key and task can bind the first approval to the second call’s commit. Protectfetchandrev-parsewith a task-scoped lock and add a concurrent regression test.🤖 Prompt for 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. In `@src/runtime/repo_manager.rs` around lines 893 - 925, Serialize the fetch and subsequent rev-parse in stage_publish for each task and publish branch using a task-scoped lock, keeping the lock held across both awaits so the staged ref cannot be replaced between them. Reuse the existing task/branch lock infrastructure if available, and add a concurrent regression test proving each approval remains bound to its own staged commit.
🤖 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.
Outside diff comments:
In `@src/runtime/cycle.rs`:
- Around line 1251-1277: Add a focused test covering the REPO_PUBLISH_EFFECT
branch in perform_effect: approve a repo.publish effect, assert the repository
manager receives the exact repo, branch, and head values, and assert execution
without a configured manager returns OpenCompanyError::Unimplemented. Place it
with the existing repository manager tests and reuse their test helpers or mock
manager setup.
In `@src/runtime/repo_manager.rs`:
- Around line 893-925: Serialize the fetch and subsequent rev-parse in
stage_publish for each task and publish branch using a task-scoped lock, keeping
the lock held across both awaits so the staged ref cannot be replaced between
them. Reuse the existing task/branch lock infrastructure if available, and add a
concurrent regression test proving each approval remains bound to its own staged
commit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 43330070-f123-4438-bc2a-cd7662899230
📒 Files selected for processing (7)
src/harness/build.rssrc/harness/repo.rssrc/harness/repo/test.rssrc/policy/consequence.rssrc/runtime/cycle.rssrc/runtime/repo_manager.rssrc/runtime/repo_manager/test.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- src/policy/consequence.rs
- src/harness/repo/test.rs
- src/harness/repo.rs
coderabbitai
Bot
added
cluster:approvals
tinyhumansai#784 merged, which carried this branch's own lineage into main along with a resolution of the same conflict — so the two sides now hold two names for one helper: `built_tool_names_with_repos_full` here and `built_tool_names_with_repos_on_cap` there, identical in signature and body. Converged on main's name rather than this branch's, because main is the side every future rebase merges against and a second name would collide again on each one. Kept this branch's doc wording, which says the thing more plainly: three wrappers, each defaulting the axis it does not vary.
2 participants
Summary
Sub-issue B of the #247 write tier. Adds
repo_publish: an agent publishes the branch it committed in a checkout, host-side, gated by operator approval — without ever holding a writable credential.Design (verified against the runtime, not just the issue text):
repo_publishis a harness tool, but it deliberately does not rely on the#243approve→re-dispatch path that #247's own scoping comment flagged as broken for a publish (the task-scoped checkout — carrying the agent's commits — is purged at turn end, before the re-dispatch fires). Instead,execute()stages the agent's committedHEADinto the mirror immediately (durable), then records anagent: Nonerepo.publisheffect that the runtime performs on approval — the same mechanismemail.senduses. So the commits are never lost, there is no re-dispatch and no commit regeneration, and a denied or expired approval leaves the remote untouched.RepoManager::stage_publish/push_published— the crate's two git write directions.stage_publishis a fetch of the checkout'sHEADinto the mirror on a host-ownedoc/<company>/<task>ref, so it never invokes the mirror'sreceive-pack; feat(repo): bind real repositories into agent workspaces with native PR/diff fetch #245'spre-receivepush-refusal hook and its no-push contract test are untouched.push_publishedpushes that ref to the real remote, credentialed, host-side — never a force push, never a+refspec.RepoManager: the branch is host-generated (oc/<company>/<task>, company = the manager's own id, task validated to a single safe segment) and re-validated before the push, so a prompt-injected--force origin mainhas no code path to reach one.repo_publishtool stages, then queues the native approval;perform_effectperforms the push once the operator approves. Wired behind four fail-closed gates: an explicitrepo.writegrant, a wired manager, a binding, and a push-capable credential.git loganswers "which agent wrote this".Acceptance criteria (#735)
repo.writeand a push-capable credential;repo/*never confer it; a read-only credential is fail-closed.oc/<company>/<task>, host-generated; an agent-supplied branch, a force flag, the default branch, or any ref outsideoc/is rejected inRepoManager, with a test per refusal.git log.Notes for reviewers
repo.writegrant that barerepodoes not confer, and credential push-capability detection #734). Until feat(repo): repo.write grant + credential push-capability detection #758 merges, this PR's diff includes feat(repo): arepo.writegrant that barerepodoes not confer, and credential push-capability detection #734's grant + push-capability work. The three#735commits are5aafc7af(primitive),f14ed979(tool + effect + wiring),925cf4cf(tests); the feat(repo): arepo.writegrant that barerepodoes not confer, and credential push-capability detection #734 portion disappears from the diff once feat(repo): repo.write grant + credential push-capability detection #758 lands.Reach::Nothingfor the tool is deliberate, and documented at length inconsequence.rs: the call only stages to a host-side cache and queues an approval — it reaches nothing external and is reversible — while the irreversible push is the separately-gatedrepo.publisheffect.Nothingis what letsexecute()run so it can stage before the checkout is cleaned up; no agent-driven change reaches a remote without the operator approving the push, in any mode.perform_effectintegration test — it is covered by composition (push_publishedis fully tested, the effect shape is tested, and the arm is a three-line extract-and-call); a real one needs aCompanyRuntimeapproval harness. (2) Attribution depends on the vendoredgit_operationshonouring the checkout's repo config; it is best-effort with a fallback.Test plan
cargo fmt --all -- --checkcargo clippy --locked --no-deps --features openhuman,tinycortex --all-targets -- -D warningscargo test --locked --features openhuman,tinycortex --lib— all pass, 0 failbrb-review-changes— 7/7 acceptance criteriaCloses #735
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes