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
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,10 @@ Postil reviews only the new commits, marks earlier findings whose code was chang
resolved, and carries still-open findings forward so the gate cannot be cleared by
pushing an unrelated commit.

Bitbucket incremental reviews are disabled unless
`POSTIL_ENABLE_BITBUCKET_INCREMENTAL=1` is set. Set it only after validating the
`/diff/{head}..{since}` compare path against the target Bitbucket deployment.

## The envelope

`--output-json` prints a stable versioned envelope (`summary`, `silent`, `findings`,
Expand Down
22 changes: 17 additions & 5 deletions src/forge/bitbucket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@
//!
//! Checks map to commit build statuses. Bitbucket has no `neutral`, so an
//! operational error marks the gate `FAILED` — fail closed, never grey.
//!
//! Incremental diffs are disabled unless `POSTIL_ENABLE_BITBUCKET_INCREMENTAL=1`
//! is present. Bitbucket's compare endpoint uses the opposite two-dot order from
//! `git diff`; keep the path available for verified deployments without making
//! unverified hosted runs trust it by default.

use anyhow::{Context, Result, anyhow};
use serde::Deserialize;
Expand All @@ -15,6 +20,8 @@ use serde_json::json;
use super::{CheckState, Forge, PrMeta, ThreadKind, check_summary, check_title};
use crate::envelope::{Envelope, Finding};

const ENABLE_INCREMENTAL_ENV: &str = "POSTIL_ENABLE_BITBUCKET_INCREMENTAL";

pub struct Bitbucket {
http: reqwest::Client,
api_base: String,
Expand Down Expand Up @@ -202,11 +209,16 @@ impl Forge for Bitbucket {
}

async fn fetch_diff_since(&self, since_sha: &str, head_sha: &str) -> Result<String> {
// Bitbucket's `diff/{spec}` two-dot form is `{to}..{from}`: it renders the
// changes that take the repo from `from` to `to`. We want everything new
// between `since` and `head`, so `to` = head and `from` = since. Atlassian's
// docs are ambiguous on the order and we have no live instance in CI, so
// this ordering is best-evidence, not verified end to end — see ROADMAP.
if std::env::var(ENABLE_INCREMENTAL_ENV).as_deref() != Ok("1") {
return Err(anyhow!(
"Bitbucket incremental review is disabled because the compare-diff path has \
not been verified for this deployment; set {ENABLE_INCREMENTAL_ENV}=1 only \
after validating /diff/{{head}}..{{since}} against the target Bitbucket API"
));
}
// Bitbucket's `diff/{spec}` two-dot form is `{to}..{from}`: it renders
// the changes that take the repo from `from` to `to`. We want everything
// new between `since` and `head`, so `to` = head and `from` = since.
let resp = self
.request(
reqwest::Method::GET,
Expand Down
133 changes: 133 additions & 0 deletions tests/e2e.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ fn postil() -> Command {
.env_remove("POSTIL_API_KEY")
.env_remove("POSTIL_API_BASE")
.env_remove("POSTIL_DETAILS_URL")
.env_remove("POSTIL_ENABLE_BITBUCKET_INCREMENTAL")
.env("MODEL_API_KEY", "test-key");
cmd
}
Expand Down Expand Up @@ -1710,6 +1711,138 @@ async fn bitbucket_flow_posts_comment_and_sets_statuses() {
assert_eq!(body["inline"]["to"], 41);
}

#[tokio::test]
async fn bitbucket_incremental_is_disabled_without_verification_gate() {
let server = MockServer::start().await;
Mock::given(method("POST"))
.and(path("/chat/completions"))
.respond_with(ResponseTemplate::new(200).set_body_json(llm_content(json!([]))))
.expect(0)
.mount(&server)
.await;
Mock::given(method("GET"))
.and(path("/repositories/acme/api/pullrequests/7"))
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
"title": "Add login",
"summary": {"raw": "PR body"},
"source": {"commit": {"hash": "headsha111"}},
"destination": {"commit": {"hash": "basesha222"}}
})))
.mount(&server)
.await;

let dir = tempfile::tempdir().unwrap();
let out = postil()
.current_dir(dir.path())
.env("POSTIL_API_BASE", server.uri())
.env("BITBUCKET_API_URL", server.uri())
.env("BITBUCKET_TOKEN", "bb-test-token")
.env_remove("BITBUCKET_USER")
.args([
"review",
"--forge",
"bitbucket",
"--repo",
"acme/api",
"--pr",
"7",
"--sha",
"headsha111",
"--since-sha",
"oldsha000",
"--no-post",
"--output-json",
])
.assert()
.code(1);
let env: Value =
serde_json::from_str(&String::from_utf8(out.get_output().stdout.clone()).unwrap()).unwrap();
assert_eq!(env["findings"][0]["path"], ".postil/provider");
assert!(
env["findings"][0]["body"]
.as_str()
.unwrap()
.contains("POSTIL_ENABLE_BITBUCKET_INCREMENTAL=1")
);

let reqs = server.received_requests().await.unwrap();
assert!(
!reqs
.iter()
.any(|request| request.url.path().contains("/diff/"))
);
}

#[tokio::test]
async fn bitbucket_incremental_fetches_documented_compare_when_enabled() {
let server = MockServer::start().await;
Mock::given(method("POST"))
.and(path("/chat/completions"))
.respond_with(ResponseTemplate::new(200).set_body_json(llm_content(json!([]))))
.mount(&server)
.await;
Mock::given(method("GET"))
.and(path("/repositories/acme/api/pullrequests/7"))
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
"title": "Add login",
"summary": {"raw": "PR body"},
"source": {"commit": {"hash": "headsha111"}},
"destination": {"commit": {"hash": "basesha222"}}
})))
.mount(&server)
.await;
Mock::given(method("GET"))
.and(path("/repositories/acme/api/diff/headsha111..oldsha000"))
.respond_with(ResponseTemplate::new(200).set_body_string(DIFF))
.expect(1)
.mount(&server)
.await;

let dir = tempfile::tempdir().unwrap();
let out = postil()
.current_dir(dir.path())
.env("POSTIL_API_BASE", server.uri())
.env("BITBUCKET_API_URL", server.uri())
.env("BITBUCKET_TOKEN", "bb-test-token")
.env("POSTIL_ENABLE_BITBUCKET_INCREMENTAL", "1")
.env("REVIEW_MODEL", "test-model")
.env_remove("BITBUCKET_USER")
.args([
"review",
"--forge",
"bitbucket",
"--repo",
"acme/api",
"--pr",
"7",
"--sha",
"headsha111",
"--since-sha",
"oldsha000",
"--no-post",
"--output-json",
])
.assert()
.code(0);
let env: Value =
serde_json::from_str(&String::from_utf8(out.get_output().stdout.clone()).unwrap()).unwrap();
assert_eq!(env["sinceSha"], "oldsha000");
assert_eq!(env["modelUsed"], "test-model");

let reqs = server.received_requests().await.unwrap();
let llm_request = reqs
.iter()
.find(|request| request.url.path() == "/chat/completions")
.unwrap();
let body: Value = llm_request.body_json().unwrap();
assert!(
body["messages"][1]["content"]
.as_str()
.unwrap()
.contains("This is an INCREMENTAL review")
);
}

#[tokio::test]
async fn azure_flow_reconstructs_diff_and_posts_thread() {
let server = MockServer::start().await;
Expand Down
Loading