From ce3d8ec40923096df1552aa5330f58f6430d9a25 Mon Sep 17 00:00:00 2001 From: Postil Maintainer Date: Sat, 11 Jul 2026 00:27:35 +0000 Subject: [PATCH] Gate Bitbucket incremental reviews --- README.md | 4 ++ src/forge/bitbucket.rs | 22 +++++-- tests/e2e.rs | 133 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 7f69471..df47346 100644 --- a/README.md +++ b/README.md @@ -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`, diff --git a/src/forge/bitbucket.rs b/src/forge/bitbucket.rs index a3a481e..f02f81e 100644 --- a/src/forge/bitbucket.rs +++ b/src/forge/bitbucket.rs @@ -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; @@ -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, @@ -202,11 +209,16 @@ impl Forge for Bitbucket { } async fn fetch_diff_since(&self, since_sha: &str, head_sha: &str) -> Result { - // 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, diff --git a/tests/e2e.rs b/tests/e2e.rs index 1f856b0..b834ad6 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -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 } @@ -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;