Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 33 additions & 19 deletions docs/spec/runtime/repos.md
Original file line number Diff line number Diff line change
Expand Up @@ -452,22 +452,36 @@ missing:
Both are otherwise silent: the tools simply are not wired, and the only symptom
is an agent that says it cannot see the code.

## Not in this tier

**No push path exists anywhere.** The write tier — PR creation, agent-attributed
commits, namespaced branches, operator approval — is a separate follow-up, and
the confinement above is arranged so that adding it is an explicit new capability
rather than a hole that was already open.

Its groundwork (issue #734) has landed without opening that path: a `repo.write`
grant now exists — distinct from and tighter than `repo`, so a bare `repo` (the
read tier every company sets) never confers it and the catch-all `*` never
confers it — and each binding records whether its bound credential can push, read
from the forge's `permissions.push` at bind time and healed on the next fetch
when it is still unknown. Granting `repo.write` over a read-only credential is
fail-closed: it warns and wires nothing. No tool consumes either yet; they are
what the push follow-up will gate on.

Also absent: a distinct uid or read-only bind mount for the agent shell (the real
filesystem boundary, named under ["The honest limit"](#the-honest-limit)), and
forges other than GitHub.
## Beyond this tier — the write tier

The read tier above ships no push path of its own. The **write tier** is a
separate, explicitly-added capability layered on top, arranged so that adding it
is a new opt-in rather than a hole that was already open here. It is built in
three parts:

- **A `repo.write` grant + credential push-capability** (issue #734). The grant
is distinct from and tighter than `repo`: a bare `repo` (the read tier every
company sets) never confers it, and the catch-all `*` never confers it. Each
binding also records whether its bound credential can push, read from the
forge's `permissions.push` at bind time and healed on the next fetch while it
is still unknown. Granting `repo.write` over a read-only credential is
fail-closed — it warns and wires nothing.
- **`repo_publish`** (issue #735). The agent commits locally in its checkout,
then publishes host-side. The host *fetches* the checkout's committed HEAD into
the mirror on a host-owned `oc/<company>/<task>` branch — a fetch never invokes
`receive-pack`, so the read tier's `pre-receive` refusal and its no-push
contract test stay untouched — and, only after the operator approves, pushes
that exact commit to the remote. The agent still holds no credentialed remote
and never pushes; every structural refusal (host-generated namespaced branch,
never a force push, never the default branch, never a ref outside `oc/`) lives
in `RepoManager` where no prompt can reach it.
- **Pull-request creation** (issue #736). After the push, the host opens a pull
request into the repository's default branch, best-effort: a PR that fails to
open leaves the branch on the remote and reports that honestly on the task,
rather than failing the publish.

Still absent: **signed** commits carrying a per-agent identity key (issue #738,
deferred — plain author/committer attribution ships with the write tier, but
signing waits for a consumer that verifies a signature); a distinct uid or
read-only bind mount for the agent shell (the real filesystem boundary, named
under ["The honest limit"](#the-honest-limit)); and forges other than GitHub.
11 changes: 11 additions & 0 deletions src/harness/brain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,9 @@ impl HarnessBrain {
// over an empty ledger is a no-op, which is what every turn that touches
// no repository does.
let _checkout_janitor = CheckoutJanitor::claim(&self.deps.checkouts);
// Issue #735: a re-dispatched grant is not a task card, so clear any task
// a prior turn stamped — `repo_publish` requires a task and refuses here.
self.deps.checkouts.set_task(None);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
// Un-streamed, like a dispatched card: this turn is answered by the
// bubble returned below, and its transient frames would otherwise
// misattribute onto whichever chat thread the console is watching.
Expand Down Expand Up @@ -633,6 +636,10 @@ impl HarnessBrain {
// guard's `Drop` is what deletes the tree on every exit — success,
// error, cancel, redirect exhaustion and panic-unwind alike.
let _checkout_janitor = CheckoutJanitor::claim(&self.deps.checkouts);
// Issue #735: this is a dispatched card, so `repo_publish` names its
// branch `oc/<company>/<card>`. Stamped on the same per-turn cell the
// janitor above claims.
self.deps.checkouts.set_task(Some(card.id.clone()));
// Issue #339, same argument for staged workflow references: an operator
// chat turn earlier in this cycle may have run a workflow through the
// orchestrator's tool, and that run belongs to the conversation, not to
Expand Down Expand Up @@ -2456,6 +2463,10 @@ impl HarnessBrain {
// so it can clone a repository, and the guard's `Drop`
// removes it when this turn ends.
let _checkout_janitor = CheckoutJanitor::claim(&self.deps.checkouts);
// Issue #735: a conversation is not a task card, so clear any
// task a prior turn stamped — `repo_publish` requires a task
// and refuses on a chat turn (task turns only, this tier).
self.deps.checkouts.set_task(None);
// Drive the brain-agnostic delegation seam (issue #176): the
// orchestrator turn, its queued delegations, and the CEO-relay
// hand-back all run behind the `RunTurn` impl. `HarnessDeps` is
Expand Down
131 changes: 112 additions & 19 deletions src/harness/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,8 @@ pub fn build_agent(
bindings: deps.repo_bindings.clone().into(),
workspace: workspace.clone(),
ledger: deps.checkouts.clone(),
agent: manifest_agent.id.clone(),
approvals: deps.approval_requests.clone(),
},
));
}
Expand All @@ -526,33 +528,58 @@ pub fn build_agent(
}
}

// Repository WRITE tier (issue #734). A distinct, tighter grant than the read
// `repo` above: `grants_repo_write_explicit` matches ONLY the exact
// Repository WRITE tier (issues #734, #735). A distinct, tighter grant than
// the read `repo` above: `grants_repo_write_explicit` matches ONLY the exact
// `repo.write`, so a bare `repo` (which every read-tier company writes) and
// the catch-all `*` confer nothing here — a company that asked for agents
// reading code does not silently get agents pushing it.
//
// #734 only *learns and records* whether a bound credential can push (probed
// at bind, re-probed on fetch, stored on `RepoBinding::can_push`); the
// `repo_publish` tool that consumes it lands in #735, so this wires NO tool
// yet. What it does now is the fail-closed half of #247's requirement: when
// `repo.write` is granted but no bound repository has a push-capable
// credential, say so and wire nothing, rather than leaving the operator to
// discover at publish time that the credential they bound was read-only.
// `None` (unknown — unprobed or pre-field) reads as cannot-push here, so only
// a proven `Some(true)` clears the warning.
// FOUR gates, all fail-closed, and the fourth is the one #734 added: an
// explicit `repo.write` grant, a wired manager, at least one binding, AND a
// bound credential that can actually push (`can_push == Some(true)`; `None` —
// unprobed or pre-field — reads as cannot-push). Missing any one wires
// `repo_publish` NOT AT ALL and says which, rather than offering a publish
// that would fail at push time on a read-only credential.
//
// Like the read tier, NOT feature-gated: the mirror and git runner are always
// compiled, and `repo_publish`'s push waits on an operator approval the
// runtime performs, so there is no forge client to gate the tool behind.
if crate::company::grants_repo_write_explicit(grants) {
let has_push_capable = deps
let push_capable = deps
.repo_bindings
.iter()
.any(|binding| binding.can_push == Some(true));
if !has_push_capable {
tracing::warn!(
match (&deps.repos, deps.repo_bindings.is_empty(), push_capable) {
(Some(repos), false, true) => {
tools.push(crate::harness::repo::repo_publish_tool(
crate::harness::repo::RepoToolContext {
repos: repos.clone(),
bindings: deps.repo_bindings.clone().into(),
workspace: workspace.clone(),
ledger: deps.checkouts.clone(),
agent: manifest_agent.id.clone(),
approvals: deps.approval_requests.clone(),
},
));
}
(None, _, _) => tracing::warn!(
company = %company,
agent = %manifest_agent.id,
"[build] agent explicitly grants `repo.write` but no repository cache is configured \
on this host; repo_publish NOT wired (fail-closed)"
),
(Some(_), true, _) => tracing::warn!(
company = %company,
agent = %manifest_agent.id,
"[build] agent explicitly grants `repo.write` but this company has bound no \
repositories; repo_publish NOT wired (fail-closed)"
),
(Some(_), false, false) => tracing::warn!(
company = %company,
agent = %manifest_agent.id,
"[build] agent explicitly grants `repo.write` but no bound repository has a \
push-capable credential; write tools NOT wired (fail-closed)"
);
push-capable credential; repo_publish NOT wired (fail-closed)"
),
}
}

Expand Down Expand Up @@ -1643,7 +1670,23 @@ mod tests {
/// `deps.repo_bindings` — so the difference between the two is exactly
/// "the operator bound something", which is two of the four gate states.
fn built_tool_names_with_repos(grants: &[&str], bindings: usize) -> Vec<String> {
built_tool_names_with_repos_on(grants, bindings, crate::store::StorageKind::Mongodb)
built_tool_names_with_repos_cap(grants, bindings, false)
}

/// [`built_tool_names_with_repos`], with control over whether the bound
/// credentials read as push-capable (issue #735) — the fourth gate the write
/// tier adds. `false` matches every read-tier caller (`can_push: None`).
fn built_tool_names_with_repos_cap(
grants: &[&str],
bindings: usize,
push_capable: bool,
) -> Vec<String> {
built_tool_names_with_repos_on_cap(
grants,
bindings,
push_capable,
crate::store::StorageKind::Mongodb,
)
}

/// [`built_tool_names_with_repos`], with the secret backend spelled out —
Expand All @@ -1655,6 +1698,20 @@ mod tests {
grants: &[&str],
bindings: usize,
storage_kind: crate::store::StorageKind,
) -> Vec<String> {
built_tool_names_with_repos_on_cap(grants, bindings, false, storage_kind)
}

/// Both gates at once. #735 added the push-capability of the bound
/// credential and #752 added the secret backend holding it, independently
/// and to the same helper; a caller that fixes one still has to be able to
/// vary the other, so the two thin wrappers above each pin their own
/// default and this carries the full shape.
fn built_tool_names_with_repos_on_cap(
grants: &[&str],
bindings: usize,
push_capable: bool,
storage_kind: crate::store::StorageKind,
) -> Vec<String> {
use crate::runtime::repo_manager::types::RepoBinding;
let dir = tempfile::tempdir().expect("tempdir");
Expand All @@ -1678,7 +1735,7 @@ mod tests {
last_fetched_millis: None,
size_bytes: 0,
bound_at_millis: 1,
can_push: None,
can_push: if push_capable { Some(true) } else { None },
})
.collect();
let manifest_agent = ManifestAgent {
Expand Down Expand Up @@ -1833,7 +1890,43 @@ mod tests {
let write_granted = built_tool_names_with_repos(&["repo.write"], 1);
assert_eq!(
write_granted, baseline,
"repo.write must wire only the read pair — no write tool exists until #735"
"repo.write with a non-push-capable credential wires only the read pair"
);
}

/// The write tier's fourth gate (issue #735): `repo_publish` is wired only
/// with `repo.write` **and** a push-capable credential, and never by the read
/// `repo` grant. The non-push-capable half is
/// `repo_write_grant_wires_no_tool_beyond_the_read_pair` above.
#[test]
fn repo_write_with_a_push_capable_credential_wires_repo_publish() {
let publish = "repo_publish".to_string();

// repo.write + a push-capable credential → repo_publish joins the belt,
// and the read pair is still there (write implies read).
let pushable = built_tool_names_with_repos_cap(&["repo.write"], 1, true);
assert!(
pushable.contains(&publish),
"a push-capable `repo.write` must wire repo_publish: {pushable:?}"
);
assert!(
pushable.contains(&"repo_checkout".to_string())
&& pushable.contains(&"repo_pr".to_string()),
"the read pair must still be wired: {pushable:?}"
);

// repo.write but a read-only credential → fail-closed, no publish.
let read_only = built_tool_names_with_repos_cap(&["repo.write"], 1, false);
assert!(
!read_only.contains(&publish),
"a read-only credential must not wire repo_publish: {read_only:?}"
);

// A bare `repo` never confers it, push-capable credential or not.
let bare = built_tool_names_with_repos_cap(&["repo"], 1, true);
assert!(
!bare.contains(&publish),
"bare `repo` (the read tier) must never wire repo_publish: {bare:?}"
);
}

Expand Down
Loading
Loading