feat(repo): open a pull request host-side via the GitHub REST API (#736) - #784
Conversation
…inyhumansai#735) The write tier's one git write direction, host-side. Split so the approval gate can sit between the two halves: - stage_publish: fetches the agent's committed HEAD out of its task-scoped checkout and into the mirror on a host-owned `oc/<company>/<task>` ref. Being a fetch, it never invokes the mirror's receive-pack, so tinyhumansai#245's pre-receive push-refusal hook and its no-push contract test are untouched — the agent still holds no credentialed remote and never pushes. - push_published: pushes an already-staged `oc/<company>/<task>` branch from the mirror to the real remote, credentialed, host-side. Never a force push and never a `+` refspec, so a non-fast-forward is refused by the remote rather than clobbering it. Every structural refusal lives in RepoManager, not a tool description: the branch is host-generated (`oc/<company>/<task>`, company is the manager's own id, task validated to one safe segment) and re-validated before the push, so no prompt-injected argument can reach `--force origin main`. Tests: end-to-end stage+push to the namespaced branch; task-id refusals; push refused for the default branch / a foreign namespace / the bare prefix / a traversal; and a non-fast-forward refused rather than forced. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ansai#735) Wires the write tier's tool on top of the RepoManager primitive. `repo_publish` is a harness tool, but its irreversible half is a native effect the runtime performs, so it survives the approval gap without preserving a checkout or re-doing the agent's work — the same shape `email.send` uses. - repo_publish tool (harness/repo.rs): execute() resolves the repo, requires a task context (this tier is task turns only; a chat turn refuses), stages the agent's committed HEAD into the mirror via RepoManager::stage_publish while the checkout still exists, then records an `agent: None` `repo.publish` Effect on the shared approval queue and returns "staged, pending approval". The push itself is not done here. - perform_effect (runtime/cycle.rs): a `repo.publish` arm performs the staged push (RepoManager::push_published) only once the operator approves. `agent: None` is what routes the approval to the runtime rather than re-dispatching the agent; a denied or expired approval never reaches it, so the remote is untouched. - consequence.rs: `repo_publish` is `Reach::Nothing` / `EffectGroup::Publish`. Classified by what the CALL does — a reversible host-side stage plus a queued approval, reaching no counterparty — not by what the approval settles; the push is the separately-gated effect. `Nothing` lets execute() run in every mode so it can stage before the checkout is cleaned up, and no agent-driven change reaches a remote without the operator approving the push. - build_agent: wires repo_publish behind four fail-closed gates — the explicit `repo.write` grant, a wired manager, a binding, and a push-capable credential (can_push == Some(true)); any missing wires nothing and says which. - Per-turn task id rides on CheckoutLedger (stamped from the card id at the turn entry points), so the long-lived tool names `oc/<company>/<task>` per turn. - Checkout commits are attributed to the agent seat (git identity set at materialize), so a published branch's git log answers "which agent wrote this". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ueueing (tinyhumansai#735) - build: repo_publish is wired only with `repo.write` AND a push-capable credential, and never by a bare `repo` grant (`built_tool_names_with_repos_cap` gains push-capability control; the read-only half stays fail-closed). - harness/repo: publishing outside a task refuses and queues nothing; on a task turn it stages the agent's commit onto `oc/<company>/<task>` in the mirror and queues exactly one native (`agent: None`) `repo.publish` approval carrying the host-generated branch — proving the push is deferred to the operator, not done in the tool. - Re-flow a long signature and call the formatter left from the primitive slice. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # src/company/mod.rs
…arness (tinyhumansai#735) The perform_effect arm lives in the always-compiled runtime, but the effect-kind constant lived in `harness::repo`, which is behind the `openhuman` feature. The default build compiles `runtime::cycle` without `harness`, so the reference failed to compile there (CI clippy: `cannot find harness in crate`). Move the constant beside EMAIL_SEND_KIND in `runtime::cycle` and have the tool reference it through `crate::runtime::cycle::REPO_PUBLISH_EFFECT`, the same shape `workflows::delivery` uses. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ad-only bindings (tinyhumansai#735) Two review findings on the approval boundary: - Bind the approval to the exact commit. stage_publish now returns the staged HEAD, the tool records it in the effect payload, and push_published sends that SHA (`<sha>:refs/heads/<branch>`) rather than whatever the mirror's branch ref points at when the approval settles. A second repo_publish on the same task force-updates the branch ref, but it can no longer ride in on an earlier approval — the approved commit is what reaches the remote. The head is validated as a bare object id before it shapes the refspec. - Reject a read-only binding up front. The tool is wired when ANY bound credential can push, but the agent may name a repository whose own credential is read-only; refuse it before staging and before an operator is asked to approve, rather than letting it fail only at push time. Tests: an approval pushes its approved commit after a same-task re-stage; a read-only binding stages and queues nothing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nt (tinyhumansai#736) The GitHub client was GET-only, so there was no path to POST /pulls. Add a `post` verb beside `get` — sharing its auth headers and credential-error mapping, and surfacing GitHub's own `message` on a create failure — and a `create_pull_request` seam on `RepoHost` returning the new PR's number and browser URL. The write verb is the only one on this seam; every other reads. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ai#736) `RepoManager::open_pull_request` opens a PR from the published `oc/<company>/<task>` branch into the repository's default branch (fetched fresh so it always targets the current default), with task and agent linkage in the body. It degrades honestly — Unimplemented with no forge client — and refuses a read-only binding, the same push-capable credential the publish rode. `perform_effect`'s repo.publish arm now opens the PR after the push, best-effort: the push is irreversible and has landed, so a PR failure does NOT fail the effect. It is reported instead — a warning, and a note on the task telling the operator the branch is on the remote but the PR was not opened. The agent makes no API call; the client stays host-side. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 6 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 (8)
📝 WalkthroughWalkthroughThis change adds an explicit ChangesRepository publishing
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant TaskTurn
participant RepoPublishTool
participant ApprovalQueue
participant RuntimeCycle
participant RepoManager
participant GitHub
TaskTurn->>RepoPublishTool: submit publish message
RepoPublishTool->>RepoManager: stage committed checkout
RepoManager-->>RepoPublishTool: branch and exact commit
RepoPublishTool->>ApprovalQueue: queue repo.publish approval
ApprovalQueue->>RuntimeCycle: approve effect
RuntimeCycle->>RepoManager: push exact commit
RepoManager->>GitHub: push branch
RuntimeCycle->>RepoManager: create pull request
RepoManager->>GitHub: POST pull request
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (9)
src/harness/mod.rs (1)
5247-5248: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a positive
repo.writefixture.
can_push: Nonekeeps the write-capable binding out of the belt. The grant matrix near Line 5334 also testsrepo, but it does not grantrepo.write. The declaration test can therefore pass whilerepo_publishis missing or misclassified. Add a separatecan_push: Some(true)case withrepo.write, and assert thatrepo_publishstays absent forNone,Some(false),repo, and*.As per coding guidelines,
src/**/*.rsmust 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/harness/mod.rs` around lines 5247 - 5248, Add a focused positive fixture in the declaration test around the existing can_push cases, using can_push: Some(true) with repo.write, and assert that repo_publish is present only for that binding. Also assert repo_publish remains absent for can_push: None, can_push: Some(false), repo, and *; keep the existing grant-matrix coverage unchanged.Source: Coding guidelines
src/runtime/cycle.rs (1)
1341-1373: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for
repo_publish_pr_titleandrepo_publish_pr_body.Both functions are pure and branch on empty input. Neither has visible coverage in this change. The cases worth pinning are:
repo_publish_pr_title: an empty or whitespace-only message returns the fallback title; a multi-line message returns the first line only; a long line truncates to 72 characters.repo_publish_pr_body: an empty message omits the leading block; an emptyagentortaskomits that clause.As per coding guidelines for
**/*.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 1341 - 1373, Add focused Rust tests for repo_publish_pr_title covering empty/whitespace input, first-line selection from multiline messages, and truncation to 72 characters. Add tests for repo_publish_pr_body verifying empty messages omit the leading block and empty agent or task values omit their respective clauses, using the existing test conventions.Source: Coding guidelines
src/harness/repo/test.rs (1)
962-1075: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the empty-message refusal and for the task, agent, and message payload fields.
The three tests cover the task gate, the happy path, and the read-only gate. Two behaviors introduced by this change stay unpinned:
executerefuses an empty or whitespace-onlymessage. No test asserts that refusal or that nothing is queued.- The effect payload now carries
task,agent, andmessage.src/runtime/cycle.rsbuilds the pull-request title and body frommessage,agent, andtask, so these three fields are a cross-file contract. The current assertions check onlybranchandhead.💚 Suggested additions
/// An empty message refuses before staging: the operator's card would say nothing. #[tokio::test] async fn repo_publish_requires_a_message() { let scratch = Scratch::new("publish-no-message"); let (manager, mut binding) = bound(&scratch, &["main"]).await; binding.can_push = Some(true); let ctx = context(&scratch, manager, vec![binding.clone()]); ctx.ledger.set_task(Some("card-1".to_string())); let tool = repo_publish_tool(ctx.clone()); let result = tool .execute(json!({ "repo": binding.key, "message": " " })) .await .unwrap(); assert!(result.is_error, "{result:?}"); assert_eq!(ctx.approvals.queued(), 0, "nothing may be queued when refused"); }Extend
repo_publish_stages_and_queues_a_native_approvalwith the linkage fields the runtime reads:assert_eq!( req.effect.payload.get("task").and_then(|v| v.as_str()), Some("card-1") ); assert_eq!( req.effect.payload.get("agent").and_then(|v| v.as_str()), Some("desk") ); assert_eq!( req.effect.payload.get("message").and_then(|v| v.as_str()), Some("the fix") );As per coding guidelines for
**/*.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/harness/repo/test.rs` around lines 962 - 1075, Add a focused async test near repo_publish_without_a_task_refuses that submits a whitespace-only message to repo_publish_tool and verifies execution errors without staging or queuing approval. Extend repo_publish_stages_and_queues_a_native_approval to assert the approval effect payload includes task "card-1", agent "desk", and message "the fix", matching the fields consumed by the runtime.Source: Coding guidelines
src/runtime/repo_manager.rs (2)
971-1002: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRe-validate
branchwithassert_publish_branchhere too.
assert_publish_branchexists because "the push is a separate call that could be reached with a hand-built argument, so it re-checks rather than trusts" (line 782-785).open_pull_requestis also a separatepubmethod, and it forwardsbranchto the forge asheadwith no shape check. The only caller in the provided context passes the host-generated staged branch, so nothing is exploitable today. The asymmetry is the concern: the same argument is re-checked on one public entry point and trusted on the other.One line restores the invariant and makes a hand-built branch a clean
InvalidRequestinstead of an opaque forge 422.🛡️ Proposed defense-in-depth fix
) -> Result<PullRequestRef> { let binding = self.get(key).await?; + // The same re-check the push performs: this is a separate public entry + // point, so it validates the branch rather than trusting its caller. + self.assert_publish_branch(branch)?; let Some(host) = self.host.as_ref() else {Note that
open_pull_request_targets_the_default_branch_with_the_given_bodyand the other three tests already passoc/acme/card-1, so they stay green.🤖 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 971 - 1002, Update the public open_pull_request flow to call assert_publish_branch on branch before forwarding it to host.create_pull_request. Preserve the existing validation and default-branch checks, ensuring malformed or hand-built branch names return InvalidRequest consistently with the push path.
632-645: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the probe failure instead of discarding it.
.ok()discards therepo_metaerror. If the probe fails for every fetch — an expired credential, a revoked scope, a forge outage —can_pushstaysNoneforever.repo_publishis then never wired, andsrc/harness/build.rslogs only "no bound repository has a push-capable credential". The operator has no evidence of the real cause.Add one trace line so the failing probe is visible. The behavior stays best-effort.
🔭 Proposed observability fix
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::warn!( + company = %self.company, + key, + error = %err, + "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 632 - 645, Update the repo metadata probe in the reprobed_push logic to log a trace message when host.repo_meta(&coords, tok) fails, while preserving the existing best-effort behavior and None result. Replace the silent .ok() conversion with error-aware handling and include sufficient probe context and the error in the trace log.src/runtime/repo_manager/test.rs (1)
1301-1379: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the
branch == baserefusal.
open_pull_requestrefuses a branch equal to the fetched default branch (src/runtime/repo_manager.rslines 997-1001). The comment there explains why the guard exists: GitHub answers an opaque 422 for a PR from a branch into itself. These four tests cover default-branch targeting, an unwired forge, a read-only binding, and a forge refusal. None reaches that guard.The test is four lines with the existing
pushable_boundhelper.🧪 Proposed test
/// A PR from the default branch into itself is refused host-side, before the /// forge answers an opaque 422. #[tokio::test] async fn open_pull_request_refuses_a_branch_equal_to_the_base() { let scratch = Scratch::new("open-pr-self"); let host = Arc::new(FakeHost::new(1).pushable()); let mgr = pushable_bound(&scratch, host.clone()).await; let err = mgr .open_pull_request("fixture", "main", "t", "b") .await .unwrap_err(); assert!( matches!(err, OpenCompanyError::InvalidRequest(_)), "a PR into itself must be refused: {err:?}" ); assert!( host.created_prs.lock().unwrap().is_empty(), "nothing may reach the forge" ); }If you apply the
assert_publish_branchsuggestion onsrc/runtime/repo_manager.rslines 971-1002, this same test then pins the earlier namespace refusal instead, which is the stronger guarantee.As per coding guidelines for
**/*.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/repo_manager/test.rs` around lines 1301 - 1379, Add a focused async test beside the existing open_pull_request tests named open_pull_request_refuses_a_branch_equal_to_the_base, using pushable_bound with a FakeHost and passing "main" as the branch. Assert that OpenCompanyError::InvalidRequest is returned and host.created_prs remains empty, confirming self-targeted PRs are rejected before reaching the forge.Source: Coding guidelines
src/runtime/repo_manager/github.rs (2)
135-139: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the doc comment with
github_message, and add unit tests for the new helpers.Two small points on the same helper pair.
The doc comment states the detail is "trimmed to its first line so an API response never pastes a wall of JSON into ours".
github_messageextracts the JSONmessagefield whole. It does not trim to one line. A multi-linemessagereaches the error string intact. Either trim it or correct the sentence. This crate already has afirst_linehelper insrc/runtime/repo_manager.rs(lines 1207-1214).
github_messageand thepoststatus mapping have no test in this file.create_pull_requestis covered only through theRepoHostfakes insrc/runtime/repo_manager/test.rs, which never exercise this HTTP client.github_messageis a pure function, so a test costs three lines.🧪 Proposed alignment and test
fn github_message(body: &str) -> String { serde_json::from_str::<serde_json::Value>(body) .ok() .and_then(|v| { v.get("message") .and_then(|m| m.as_str()) + .and_then(|m| m.lines().map(str::trim).find(|l| !l.is_empty())) .map(str::to_string) }) .unwrap_or_default() }Add beside the module's existing tests:
#[test] fn github_message_reads_the_first_line_and_tolerates_a_non_json_body() { assert_eq!( github_message(r#"{"message":"Validation Failed\nsecond line"}"#), "Validation Failed" ); assert_eq!(github_message("<html>502</html>"), ""); assert_eq!(github_message(r#"{"documentation_url":"x"}"#), ""); }As per coding guidelines for
**/*.rs: "Add focused tests with every behavior change", and forsrc/**/*.rs: "Maintain at least 80% coverage for meaningful library behavior and document intentionally untested edge cases in the pull request."Also applies to: 183-193
🤖 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/github.rs` around lines 135 - 139, Update github_message to return only the first line of the JSON message, reusing the existing first_line helper, while preserving empty output for non-JSON bodies or responses without a message. Add focused unit tests beside the module’s existing tests covering multiline messages, non-JSON bodies, and missing message fields; also test the post status mapping behavior around the referenced status-handling code.Source: Coding guidelines
164-179: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCap both POST response bodies with
read_cappedandMAX_DIFF_BYTES.
poststill reads success and error bodies without a bound. The existing helper returnsResult<Vec<u8>>and maps stream errors toOpenCompanyError::Store, so the success path can use?.🤖 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/github.rs` around lines 164 - 179, Update the GitHub POST response handling around post to read both successful and unsuccessful response bodies through read_capped using MAX_DIFF_BYTES. Preserve github_message processing for error responses, and propagate the helper’s OpenCompanyError::Store result with ? on the success path instead of calling response.text().src/harness/build.rs (1)
1789-1797: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the stale doc comment:
repo_publishis wired in this change.The comment states that the write tier "wires NO tool of its own yet" and that "
repo_publishlands in#735". This same file now wiresrepo_publishat line 534, andrepo_write_with_a_push_capable_credential_wires_repo_publishbelow asserts it. The assertion itself stays correct, because the helper builds bindings withcan_push: None. Only the prose is wrong, and it invites a reader to conclude no publish tool exists.📝 Proposed doc correction
- /// The repository *write* tier (issue `#734`) wires NO tool of its own yet — - /// `repo_publish` lands in `#735`. Granting `repo.write` confers the read pair - /// (write implies read, since `repo.write` matches the read predicate's - /// `repo.` prefix) and nothing more, whatever the bound credential's push - /// capability. This pins "`#734` wires nothing": if a later change wires a - /// write tool without the push-capability gate, the exact-set assertion here - /// breaks rather than shipping an ungated push surface. The bindings the - /// helper builds carry `can_push: None`, so the write tier fails closed - /// (warns) — and still adds no tool. + /// The write tier's non-push-capable state: granting `repo.write` over a + /// binding whose credential is not push-capable confers the read pair (write + /// implies read, since `repo.write` matches the read predicate's `repo.` + /// prefix) and nothing more. The bindings the helper builds carry + /// `can_push: None`, so the fourth gate fails closed (warns) and + /// `repo_publish` is NOT wired. If a later change wires a write tool without + /// the push-capability gate, the exact-set assertion here breaks rather than + /// shipping an ungated push surface. The push-capable half is + /// `repo_write_with_a_push_capable_credential_wires_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/build.rs` around lines 1789 - 1797, Update the stale documentation comment above the repository write-tier assertion to state that repo_publish is now wired in this change, removing the claims that no write tool exists and that it lands in `#735`. Preserve the explanation that bindings use can_push: None, so the write tier fails closed and does not add the tool.
🤖 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 `@docs/spec/runtime/repos.md`:
- Around line 447-454: Update the repository capability description around the
repo.write grant to document the current repo_publish flow, approved pushes, and
best-effort pull-request creation; remove stale statements that no push path
exists or that no tool consumes repo.write. Ensure the section accurately
reflects the PR’s implemented behavior, or defer only if a later stack change
owns the final documentation.
In `@src/harness/brain.rs`:
- Around line 395-397: Add focused integration tests covering checkout
task-context transitions around run_task and grant redispatch: verify run_task
sets CheckoutLedger::task() to the card ID, operator messages clear inherited
context, and both single-use and standing grant redispatches run without task
context. Also assert task-scoped repo_publish stages oc/<company>/<card> and
queues the native repo.publish effect, while native approvals do not redispatch
an agent.
---
Nitpick comments:
In `@src/harness/build.rs`:
- Around line 1789-1797: Update the stale documentation comment above the
repository write-tier assertion to state that repo_publish is now wired in this
change, removing the claims that no write tool exists and that it lands in `#735`.
Preserve the explanation that bindings use can_push: None, so the write tier
fails closed and does not add the tool.
In `@src/harness/mod.rs`:
- Around line 5247-5248: Add a focused positive fixture in the declaration test
around the existing can_push cases, using can_push: Some(true) with repo.write,
and assert that repo_publish is present only for that binding. Also assert
repo_publish remains absent for can_push: None, can_push: Some(false), repo, and
*; keep the existing grant-matrix coverage unchanged.
In `@src/harness/repo/test.rs`:
- Around line 962-1075: Add a focused async test near
repo_publish_without_a_task_refuses that submits a whitespace-only message to
repo_publish_tool and verifies execution errors without staging or queuing
approval. Extend repo_publish_stages_and_queues_a_native_approval to assert the
approval effect payload includes task "card-1", agent "desk", and message "the
fix", matching the fields consumed by the runtime.
In `@src/runtime/cycle.rs`:
- Around line 1341-1373: Add focused Rust tests for repo_publish_pr_title
covering empty/whitespace input, first-line selection from multiline messages,
and truncation to 72 characters. Add tests for repo_publish_pr_body verifying
empty messages omit the leading block and empty agent or task values omit their
respective clauses, using the existing test conventions.
In `@src/runtime/repo_manager.rs`:
- Around line 971-1002: Update the public open_pull_request flow to call
assert_publish_branch on branch before forwarding it to
host.create_pull_request. Preserve the existing validation and default-branch
checks, ensuring malformed or hand-built branch names return InvalidRequest
consistently with the push path.
- Around line 632-645: Update the repo metadata probe in the reprobed_push logic
to log a trace message when host.repo_meta(&coords, tok) fails, while preserving
the existing best-effort behavior and None result. Replace the silent .ok()
conversion with error-aware handling and include sufficient probe context and
the error in the trace log.
In `@src/runtime/repo_manager/github.rs`:
- Around line 135-139: Update github_message to return only the first line of
the JSON message, reusing the existing first_line helper, while preserving empty
output for non-JSON bodies or responses without a message. Add focused unit
tests beside the module’s existing tests covering multiline messages, non-JSON
bodies, and missing message fields; also test the post status mapping behavior
around the referenced status-handling code.
- Around line 164-179: Update the GitHub POST response handling around post to
read both successful and unsuccessful response bodies through read_capped using
MAX_DIFF_BYTES. Preserve github_message processing for error responses, and
propagate the helper’s OpenCompanyError::Store result with ? on the success path
instead of calling response.text().
In `@src/runtime/repo_manager/test.rs`:
- Around line 1301-1379: Add a focused async test beside the existing
open_pull_request tests named
open_pull_request_refuses_a_branch_equal_to_the_base, using pushable_bound with
a FakeHost and passing "main" as the branch. Assert that
OpenCompanyError::InvalidRequest is returned and host.created_prs remains empty,
confirming self-targeted PRs are rejected before reaching the forge.
🪄 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: da58cf50-a7aa-4053-b489-03540ee8a4d5
📒 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
…mansai#736) The "Not in this tier" section still said no push path exists and no tool consumes repo.write — stale once tinyhumansai#735 and tinyhumansai#736 land in the stack. Rewrite it as "Beyond this tier — the write tier": the repo.write grant + push-capability (tinyhumansai#734), repo_publish's fetch-into-mirror + approved host-side push (tinyhumansai#735), and best-effort PR creation (tinyhumansai#736), and what remains absent (signed commits tinyhumansai#738, a uid/bind-mount shell boundary, non-GitHub forges). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
One conflict, in `harness::build`'s test helper, where two PRs added a fourth gate to the same function from opposite sides: this branch's tinyhumansai#735 lineage added `push_capable` (does the bound credential read as push-capable), and main's tinyhumansai#752 added `storage_kind` (can this host hold a repository credential safely). Kept both rather than choosing. `built_tool_names_with_repos_on_cap` carries the full shape and the two existing wrappers each pin their own default — callers that vary push capability (`…_cap`, four sites) and callers that vary the secret backend (`…_on`, one site) both keep compiling and testing exactly what they were written to test. Submodule pins re-synced after the merge; main had moved `vendor/openhuman/vendor/tinycortex`, whose own nested `api` crate is a cargo dependency, so the tree does not build until that resolves.
|
Merged current One conflict, and it is the interesting kind: two PRs added a fourth gate to the same test helper in Kept both rather than picking. A note for anyone else rebasing onto this main: the merge also moves Verified on the merge commit: CI is running; I will report if anything comes back red. |
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.
Summary
Sub-issue C of the #247 write tier. After an approved
repo_publishpushes itsoc/<company>/<task>branch (#735), open a pull request for it — host-side, targeting the default branch.postbesideget, sharing its auth headers and credential-error mapping (and surfacing GitHub's ownmessageon a create failure), plus acreate_pull_requestseam onRepoHostreturning the new PR's number and URL.RepoManager::open_pull_request. Opens the PR from the published branch into the repository's default branch — fetched fresh viarepo_metaso it always targets the current default — with task and agent linkage in the body. Degrades honestly (Unimplementedwith no forge client) and refuses a read-only binding, the same push-capable credential the publish rode.perform_effect. Therepo.publisharm opens the PR after the push, best-effort: the push is irreversible and has landed, so a PR failure does not fail the effect — it is reported instead (a warning, and a note on the task telling the operator the branch is up but the PR was not opened).Acceptance criteria
repo.writegrant and push-capable credential as the publish (feat(repo): arepo.writegrant that barerepodoes not confer, and credential push-capability detection #734/feat(repo): host-siderepo_publish— namespaced branches, structural push refusals, approval-gated #735); never reachable on a read-only binding.Notes for reviewers
repo_publish— namespaced branches, structural push refusals, approval-gated #735) → feat(repo): repo.write grant + credential push-capability detection #758 (feat(repo): arepo.writegrant that barerepodoes not confer, and credential push-capability detection #734). Until those merge, this diff carries their commits. The feat(repo): open a pull request host-side via the GitHub REST API #736 commits are4b572a42(POST verb + seam) ande4693a2b(open-PR wiring).TaskDiscussionPostednote on the card (reusing an existing event), plus a grep-friendly warn log — no new event variant.create_pull_requestresponse → number/url) lives in thegithub-gated client; the trait method andopen_pull_requestare always compiled and unit-tested via a fake host.Test plan
cargo fmt --all -- --checkcargo clippy --locked --all-targets -- -D warnings(default features)cargo clippy --features openhuman,tinycortex,github --all-targets -- -D warningscargo test --features openhuman,tinycortex --lib— 3409 pass, 0 failCloses #736
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes