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
7 changes: 5 additions & 2 deletions src/apps/desktop/src/api/review_platform_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::api::app_state::AppState;
use log::error;
use openbitfun_core::service::review_platform::{
untrusted_repository_error_message, ReviewPlatformCiLog, ReviewPlatformDetailSection,
ReviewPlatformError, ReviewPlatformIssueEvidence, ReviewPlatformKind,
ReviewPlatformError, ReviewPlatformIssueEvidence, ReviewPlatformKind, ReviewPlatformListState,
ReviewPlatformPullRequestDetail, ReviewPlatformPullRequestDetailPage,
ReviewPlatformPullRequestReviewTarget, ReviewPlatformService, ReviewPlatformWorkspaceSnapshot,
};
Expand All @@ -18,6 +18,8 @@ pub struct ReviewPlatformWorkspaceSnapshotRequest {
pub remote_id: Option<String>,
pub page: Option<u32>,
pub per_page: Option<u32>,
#[serde(default)]
pub state: ReviewPlatformListState,
}

#[derive(Debug, Deserialize)]
Expand Down Expand Up @@ -76,11 +78,12 @@ pub async fn review_platform_get_workspace_snapshot(
_state: State<'_, AppState>,
request: ReviewPlatformWorkspaceSnapshotRequest,
) -> Result<ReviewPlatformWorkspaceSnapshot, String> {
ReviewPlatformService::workspace_snapshot(
ReviewPlatformService::workspace_snapshot_with_state(
&request.repository_path,
request.remote_id.as_deref(),
request.page,
request.per_page,
request.state,
)
.await
.map_err(|error| {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ impl GetFileDiffTool {
let platform = match pull_request.platform() {
"github" => Some(ReviewPlatformKind::Github),
"gitlab" => Some(ReviewPlatformKind::Gitlab),
"gitee" => Some(ReviewPlatformKind::Gitee),
"gitcode" => None,
value => {
return Err(OpenBitFunError::tool(format!(
Expand Down Expand Up @@ -2034,60 +2035,65 @@ mod tests {

#[test]
fn pull_request_diff_route_uses_prepared_provider_identity_not_remote_id() {
let mut context = prepared_context();
context.custom_data.insert(
"deep_review_run_manifest".to_string(),
json!({
"evidencePack": {
"reviewTarget": {
"version": 1,
"source": "pull_request",
"fingerprint": "provider-route-fingerprint",
"baseRevision": "1111111111111111111111111111111111111111",
"headRevision": "2222222222222222222222222222222222222222",
"completeness": "complete",
"workspaceBinding": "unavailable",
"pullRequest": {
"remoteId": "fabricated-remote-that-must-not-route",
"platform": "github",
"host": "github.com",
"projectPath": "exact/project",
"pullRequestId": "42",
"number": 42,
"webUrl": "https://github.com/exact/project/pull/42"
},
"files": [{
"path": "src/lib.rs",
"status": "modified",
"completeness": "complete"
}],
"limitations": []
for (platform_name, platform, host) in [
("github", ReviewPlatformKind::Github, "github.com"),
("gitee", ReviewPlatformKind::Gitee, "gitee.com"),
] {
let mut context = prepared_context();
context.custom_data.insert(
"deep_review_run_manifest".to_string(),
json!({
"evidencePack": {
"reviewTarget": {
"version": 1,
"source": "pull_request",
"fingerprint": "provider-route-fingerprint",
"baseRevision": "1111111111111111111111111111111111111111",
"headRevision": "2222222222222222222222222222222222222222",
"completeness": "complete",
"workspaceBinding": "unavailable",
"pullRequest": {
"remoteId": "fabricated-remote-that-must-not-route",
"platform": platform_name,
"host": host,
"projectPath": "exact/project",
"pullRequestId": "42",
"number": 42,
"webUrl": format!("https://{host}/exact/project/pulls/42")
},
"files": [{
"path": "src/lib.rs",
"status": "modified",
"completeness": "complete"
}],
"limitations": []
}
}
}),
);
let evidence = GetFileDiffTool::target_evidence(&context)
.expect("evidence should parse")
.expect("evidence should exist");

let route =
GetFileDiffTool::pull_request_file_diff_route(&context, &evidence, "src/lib.rs")
.expect("prepared provider route should be exact");

assert_eq!(
route,
ProviderFileDiffRoute::Identity {
platform,
host: host.to_string(),
project_path: "exact/project".to_string(),
pull_request_id: "42".to_string(),
base_revision: "1111111111111111111111111111111111111111".to_string(),
head_revision: "2222222222222222222222222222222222222222".to_string(),
file_path: "src/lib.rs".to_string(),
file_page_hint: Some(1),
repository_path: None,
}
}),
);
let evidence = GetFileDiffTool::target_evidence(&context)
.expect("evidence should parse")
.expect("evidence should exist");

let route =
GetFileDiffTool::pull_request_file_diff_route(&context, &evidence, "src/lib.rs")
.expect("prepared provider route should be exact");

assert_eq!(
route,
ProviderFileDiffRoute::Identity {
platform: ReviewPlatformKind::Github,
host: "github.com".to_string(),
project_path: "exact/project".to_string(),
pull_request_id: "42".to_string(),
base_revision: "1111111111111111111111111111111111111111".to_string(),
head_revision: "2222222222222222222222222222222222222222".to_string(),
file_path: "src/lib.rs".to_string(),
file_page_hint: Some(1),
repository_path: None,
}
);
);
}
}

#[tokio::test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use crate::agentic::tools::framework::{
};
use crate::service::review_platform::{
ReviewPlatformApprovalRequest, ReviewPlatformCreatePullRequestRequest,
ReviewPlatformDetailSection, ReviewPlatformError, ReviewPlatformKind, ReviewPlatformRemote,
ReviewPlatformReplyToThreadRequest, ReviewPlatformRequestChangesRequest,
ReviewPlatformDetailSection, ReviewPlatformError, ReviewPlatformKind, ReviewPlatformListState,
ReviewPlatformRemote, ReviewPlatformReplyToThreadRequest, ReviewPlatformRequestChangesRequest,
ReviewPlatformResolveThreadRequest, ReviewPlatformService, ReviewPlatformSubmitReviewRequest,
ReviewSubmitEvent,
};
Expand Down Expand Up @@ -130,6 +130,7 @@ impl ReviewPlatformTool {
"github" => Ok(ReviewPlatformKind::Github),
"gitlab" => Ok(ReviewPlatformKind::Gitlab),
"gitcode" => Ok(ReviewPlatformKind::Gitcode),
"gitee" => Ok(ReviewPlatformKind::Gitee),
"unknown" => Ok(ReviewPlatformKind::Unknown),
other => Err(OpenBitFunError::tool(format!(
"Unsupported review platform kind: {}",
Expand All @@ -138,6 +139,15 @@ impl ReviewPlatformTool {
}
}

fn list_state(input: &Value) -> OpenBitFunResult<ReviewPlatformListState> {
match input.get("state") {
None | Some(Value::Null) => Ok(ReviewPlatformListState::All),
Some(state) => serde_json::from_value(state.clone()).map_err(|error| {
OpenBitFunError::tool(format!("Invalid pull request state filter: {error}"))
}),
}
}

async fn resolve_remote_id(
action: &str,
repository_path: &str,
Expand Down Expand Up @@ -242,7 +252,7 @@ impl Tool for ReviewPlatformTool {

Use this for remote review-platform operations such as discovering remotes, loading the workspace PR snapshot, counting pull requests, listing pull requests, opening full or paginated pull request detail, loading CI logs, creating a pull request, replying to review threads, submitting a comment review, approving, revoking approval, requesting changes, or resolving a review thread. Use ExecCommand for local repository state and branch/commit/push operations.

GitHub authentication is owned by the local `gh` CLI and must never use token actions. Authentication-token actions are only for GitLab and GitCode when the user explicitly provides a token or asks to clear a stored token. Never guess or expose token values.
GitHub authentication is owned by the local `gh` CLI and must never use token actions. Authentication-token actions are only for GitLab, GitCode, and Gitee when the user explicitly provides a token or asks to clear a stored token. Never guess or expose token values.

When returning pull request results to the user, include the provider web URL so the chat UI can open the pull request detail panel naturally."#.to_string())
}
Expand Down Expand Up @@ -301,6 +311,11 @@ When returning pull request results to the user, include the provider web URL so
"type": "integer",
"description": "Page size for list_pull_requests, get_workspace_snapshot, or get_pull_request_detail_page."
},
"state": {
"type": "string",
"enum": ["all", "open", "draft", "merged", "closed"],
"description": "Repository-wide PR state filter for list_pull_requests, count_pull_requests, or get_workspace_snapshot; defaults to all. Only use filters advertised in capabilities.supportedPullRequestStates."
},
"section": {
"type": "string",
"enum": ["overview", "ci", "files", "commits", "reviews"],
Expand All @@ -316,16 +331,16 @@ When returning pull request results to the user, include the provider web URL so
},
"platform": {
"type": "string",
"enum": ["github", "gitlab", "gitcode", "unknown"],
"description": "GitLab or GitCode platform kind for update_auth_token or clear_auth_token. GitHub uses local gh authentication."
"enum": ["github", "gitlab", "gitcode", "gitee", "unknown"],
"description": "GitLab, GitCode, or Gitee platform kind for update_auth_token or clear_auth_token. GitHub uses local gh authentication."
},
"host": {
"type": "string",
"description": "Review platform host for update_auth_token or clear_auth_token."
},
"token": {
"type": "string",
"description": "GitLab or GitCode personal access token for update_auth_token. Only provide this when the user explicitly asks to store that token. Never provide a GitHub token."
"description": "GitLab, GitCode, or Gitee personal access token for update_auth_token. Only provide this when the user explicitly asks to store that token. Never provide a GitHub token."
},
"title": {
"type": "string",
Expand Down Expand Up @@ -782,11 +797,12 @@ When returning pull request results to the user, include the provider web URL so
.and_then(Value::as_u64)
.map(|value| value as u32);
let remote_id = Self::optional_string_field(input, "remote_id");
let snapshot = ReviewPlatformService::workspace_snapshot(
let snapshot = ReviewPlatformService::workspace_snapshot_with_state(
&repository_path,
remote_id.as_deref(),
page,
per_page,
Self::list_state(input)?,
)
.await
.map_err(|error| OpenBitFunError::tool(error.to_string()))?;
Expand Down Expand Up @@ -820,11 +836,12 @@ When returning pull request results to the user, include the provider web URL so
let remote_id = resolved_remote_id
.clone()
.expect("remote-bound action should resolve a remote");
let snapshot = ReviewPlatformService::workspace_snapshot(
let snapshot = ReviewPlatformService::workspace_snapshot_with_state(
&repository_path,
Some(remote_id.as_str()),
Some(1),
Some(1),
Self::list_state(input)?,
)
.await
.map_err(|error| OpenBitFunError::tool(error.to_string()))?;
Expand Down Expand Up @@ -864,11 +881,12 @@ When returning pull request results to the user, include the provider web URL so
let remote_id = resolved_remote_id
.clone()
.expect("remote-bound action should resolve a remote");
let snapshot = ReviewPlatformService::workspace_snapshot(
let snapshot = ReviewPlatformService::workspace_snapshot_with_state(
&repository_path,
Some(remote_id.as_str()),
page,
per_page,
Self::list_state(input)?,
)
.await
.map_err(|error| OpenBitFunError::tool(error.to_string()))?;
Expand Down Expand Up @@ -1221,7 +1239,8 @@ fn canonical_supported_remotes(remotes: &[ReviewPlatformRemote]) -> Vec<&ReviewP
ReviewPlatformKind::Github => 0,
ReviewPlatformKind::Gitlab => 1,
ReviewPlatformKind::Gitcode => 2,
ReviewPlatformKind::Unknown => 3,
ReviewPlatformKind::Gitee => 3,
ReviewPlatformKind::Unknown => 4,
};
let normalized_host = remote.host.trim().to_ascii_lowercase();
let normalized_project = remote.project_path.trim_matches('/').to_ascii_lowercase();
Expand Down Expand Up @@ -1271,6 +1290,26 @@ fn remote_selection_result(
mod tests {
use super::*;

#[test]
fn gitee_uses_the_existing_write_permission_and_concurrency_boundary() {
let tool = ReviewPlatformTool::new();
assert_eq!(
ReviewPlatformTool::platform_kind(&json!({"platform":"gitee"})).unwrap(),
ReviewPlatformKind::Gitee
);
assert!(!tool.is_readonly());
for action in [
ACTION_CREATE,
ACTION_SUBMIT_REVIEW,
ACTION_APPROVE,
ACTION_REVOKE_APPROVAL,
ACTION_UPDATE_AUTH_TOKEN,
] {
assert!(!tool.is_concurrency_safe(Some(&json!({"action":action,"platform":"gitee"}))));
}
assert!(tool.is_concurrency_safe(Some(&json!({"action":ACTION_LIST,"platform":"gitee"}))));
}

fn github_remote(id: &str, name: &str, project_path: &str) -> ReviewPlatformRemote {
serde_json::from_value(json!({
"id": id,
Expand Down
18 changes: 15 additions & 3 deletions src/crates/assembly/core/src/service/review_platform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ pub use openbitfun_services_integrations::review_platform::{
ReviewPlatformCapabilities, ReviewPlatformCiItem, ReviewPlatformCiLog, ReviewPlatformCommit,
ReviewPlatformCreatePullRequestRequest, ReviewPlatformDetailSection, ReviewPlatformError,
ReviewPlatformFile, ReviewPlatformIssueComment, ReviewPlatformIssueEvidence,
ReviewPlatformKind, ReviewPlatformPullRequest, ReviewPlatformPullRequestDetail,
ReviewPlatformPullRequestDetailPage, ReviewPlatformPullRequestFileDiff,
ReviewPlatformPullRequestReviewTarget, ReviewPlatformRemote,
ReviewPlatformKind, ReviewPlatformListState, ReviewPlatformPullRequest,
ReviewPlatformPullRequestDetail, ReviewPlatformPullRequestDetailPage,
ReviewPlatformPullRequestFileDiff, ReviewPlatformPullRequestReviewTarget, ReviewPlatformRemote,
ReviewPlatformReplyToThreadRequest, ReviewPlatformRepositoryRef,
ReviewPlatformRequestChangesRequest, ReviewPlatformResolveThreadRequest,
ReviewPlatformSubmitReviewRequest, ReviewPlatformThread, ReviewPlatformThreadKind,
Expand Down Expand Up @@ -159,6 +159,18 @@ impl ReviewPlatformService {
.await
}

pub async fn workspace_snapshot_with_state(
repository_path: &str,
remote_id: Option<&str>,
page: Option<u32>,
per_page: Option<u32>,
state: ReviewPlatformListState,
) -> Result<ReviewPlatformWorkspaceSnapshot, ReviewPlatformError> {
owner_service()?
.workspace_snapshot_with_state(repository_path, remote_id, page, per_page, state)
.await
}

pub async fn pull_request_detail(
repository_path: &str,
remote_id: &str,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ fn parse_pull_request_identity(
));
}
let platform = required_string(value, &["platform"], "reviewTarget.pullRequest.platform")?;
if !matches!(platform.as_str(), "github" | "gitlab" | "gitcode") {
if !matches!(platform.as_str(), "github" | "gitlab" | "gitcode" | "gitee") {
return Err(ReviewTargetEvidenceValidationError::invalid(
"reviewTarget.pullRequest.platform",
"unknown provider",
Expand Down Expand Up @@ -1046,6 +1046,32 @@ mod tests {
assert_eq!(evidence.diff_revisions_for_path("src/lib.rs"), None);
}

#[test]
fn gitee_targets_keep_provider_identity_and_disallow_live_repository_fallback() {
let mut value = manifest();
let target = &mut value["evidencePack"]["reviewTarget"];
target["source"] = json!("pull_request");
target["workspaceBinding"] = json!("unavailable");
target["pullRequest"] = json!({
"remoteId": "origin:gitee:example__repo", "platform": "gitee", "host": "gitee.com",
"projectPath": "example/repo", "pullRequestId": "69", "number": 69,
"webUrl": "https://gitee.com/example/repo/pulls/69"
});
let evidence = ReviewTargetEvidence::from_manifest(&value)
.unwrap()
.unwrap();
assert_eq!(evidence.pull_request().unwrap().platform(), "gitee");
assert_eq!(evidence.pull_request().unwrap().pull_request_id(), "69");
assert!(!evidence.allows_live_repository_context());
assert_eq!(evidence.diff_revisions_for_path("src/lib.rs"), None);
value["evidencePack"]["reviewTarget"]["pullRequest"]["platform"] = json!("future-provider");
assert!(ReviewTargetEvidence::from_manifest(&value).is_err());
assert_eq!(
value["evidencePack"]["reviewTarget"]["pullRequest"]["platform"],
"future-provider"
);
}

#[test]
fn validates_evidence_and_packet_paths_against_the_target() {
let value = scoped_manifest();
Expand Down
Loading
Loading