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
2 changes: 2 additions & 0 deletions .github/actions/prepare-orion-artifacts/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ runs:
mv "${ARTIFACT_DIR}"/orion/runner-config/.env.prod "${ARTIFACT_DIR}"/.env
mv "${ARTIFACT_DIR}"/orion/runner-config/scorpio.toml "${ARTIFACT_DIR}"/
mv "${ARTIFACT_DIR}"/orion/runner-config/run.sh "${ARTIFACT_DIR}"/
mv "${ARTIFACT_DIR}"/orion/runner-config/preflight.sh "${ARTIFACT_DIR}"/
mv "${ARTIFACT_DIR}"/orion/runner-config/cleanup.sh "${ARTIFACT_DIR}"/

# Move systemd service file
Expand All @@ -40,6 +41,7 @@ runs:
# Set executable permissions
chmod +x "${ARTIFACT_DIR}"/orion
chmod +x "${ARTIFACT_DIR}"/run.sh
chmod +x "${ARTIFACT_DIR}"/preflight.sh
chmod +x "${ARTIFACT_DIR}"/cleanup.sh

echo "==> Final artifact structure:"
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/orion-client-deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ jobs:
orion/runner-config/.env.prod
orion/runner-config/scorpio.toml
orion/runner-config/run.sh
orion/runner-config/preflight.sh
orion/runner-config/cleanup.sh
orion/systemd/orion-runner.service
retention-days: 7
Expand Down
11 changes: 5 additions & 6 deletions api-model/src/buck2/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,10 @@ pub struct TaskBuildRequest {
pub cl_link: String,
//TODO: for old database only, delete after updated
pub cl_id: i64,
/// The list of changed files, expressed relative to the monorepo root.
///
/// Example values:
/// - `jupiter/callisto/src/access_token.rs`
/// - `common/lib.rs`
/// The list of changed files in the hybrid path contract:
/// - files inside `repo` are repo-relative (for example `src/main.rs`)
/// - shared files outside `repo` stay monorepo-relative
/// (for example `common/lib.rs`)
pub changes: Vec<Status<ProjectRelativePath>>,
/// Buck2 target path (e.g. //app:server). Optional for backward compatibility.
#[serde(default, alias = "targets_path")]
Expand All @@ -45,7 +44,7 @@ pub struct RetryBuildRequest {
pub build_id: String,
pub cl_link: String,
pub cl_id: i64,
/// The list of changed files, expressed relative to the monorepo root.
/// The list of changed files in the hybrid path contract used by Orion.
pub changes: Vec<Status<ProjectRelativePath>>,
pub targets: Option<Vec<String>>,
}
Expand Down
4 changes: 3 additions & 1 deletion api-model/src/buck2/ws.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ pub enum WSMessage {
/// The Buck2 project path within the monorepo (for example `/jupiter/callisto`).
repo: String,
cl_link: String,
/// The list of changed files, expressed relative to the monorepo root.
/// The list of changed files in the hybrid path contract:
/// repo-local files are repo-relative, shared files remain
/// monorepo-relative.
changes: Vec<Status<ProjectRelativePath>>,
},

Expand Down
144 changes: 128 additions & 16 deletions ceres/src/build_trigger/changes_calculator.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::{
path::{Path, PathBuf},
path::{Component, Path, PathBuf},
sync::Arc,
};

Expand All @@ -14,51 +14,117 @@ use crate::{
model::change_list::ClDiffFile,
};

fn normalize_change_path_for_repo(repo_path: &str, path: &Path) -> ProjectRelativePath {
fn is_safe_normalized_path(path: &str) -> bool {
path.is_empty()
|| (!path.contains("//")
&& Path::new(path)
.components()
.all(|component| matches!(component, Component::Normal(_))))
}

fn normalize_change_path_for_repo_with_prefix(
repo_prefix: &str,
repo_prefix_with_slash: Option<&str>,
path: &Path,
) -> Option<ProjectRelativePath> {
let raw = path
.to_string_lossy()
.replace('\\', "/")
.trim_start_matches('/')
.to_string();
let repo_prefix = repo_path.trim_matches('/');

let normalized = if repo_prefix.is_empty() {
raw
} else if raw == repo_prefix {
String::new()
} else if let Some(stripped) = raw.strip_prefix(&format!("{repo_prefix}/")) {
stripped.to_string()
} else if let Some(prefix) = repo_prefix_with_slash {
if let Some(stripped) = raw.strip_prefix(prefix) {
stripped.to_string()
} else {
raw
}
} else {
raw
};

ProjectRelativePath::new(&normalized)
if !is_safe_normalized_path(&normalized) {
tracing::warn!(
path = %normalized,
"Dropping unsafe build change path after normalization."
);
return None;
}

Some(ProjectRelativePath::new(&normalized))
}

#[cfg(test)]
fn normalize_change_path_for_repo(repo_path: &str, path: &Path) -> Option<ProjectRelativePath> {
let repo_prefix = repo_path.trim_matches('/');
let repo_prefix_with_slash = (!repo_prefix.is_empty()).then(|| format!("{repo_prefix}/"));
normalize_change_path_for_repo_with_prefix(repo_prefix, repo_prefix_with_slash.as_deref(), path)
}

fn push_change_if_valid(
changes: &mut Vec<Status<ProjectRelativePath>>,
status_builder: impl FnOnce(ProjectRelativePath) -> Status<ProjectRelativePath>,
normalized: Option<ProjectRelativePath>,
) {
if let Some(path) = normalized {
changes.push(status_builder(path));
}
}

fn build_changes_for_repo(
repo_path: &str,
cl_diff_files: Vec<ClDiffFile>,
) -> Result<Vec<Status<ProjectRelativePath>>, MegaError> {
let to_project_relative = |path: &Path| -> Result<ProjectRelativePath, MegaError> {
Ok(normalize_change_path_for_repo(repo_path, path))
let repo_prefix = repo_path.trim_matches('/');
let repo_prefix_with_slash = (!repo_prefix.is_empty()).then(|| format!("{repo_prefix}/"));
let to_project_relative = |path: &Path| {
normalize_change_path_for_repo_with_prefix(
repo_prefix,
repo_prefix_with_slash.as_deref(),
path,
)
};

let mut counter_changes = Vec::new();
for change in cl_diff_files {
match change {
ClDiffFile::New(path, _) => {
counter_changes.push(Status::Added(to_project_relative(&path)?));
push_change_if_valid(
&mut counter_changes,
Status::Added,
to_project_relative(&path),
);
}
ClDiffFile::Deleted(path, _) => {
counter_changes.push(Status::Removed(to_project_relative(&path)?));
push_change_if_valid(
&mut counter_changes,
Status::Removed,
to_project_relative(&path),
);
}
ClDiffFile::Modified(path, _, _) => {
counter_changes.push(Status::Modified(to_project_relative(&path)?));
push_change_if_valid(
&mut counter_changes,
Status::Modified,
to_project_relative(&path),
);
}
ClDiffFile::Renamed(old_path, new_path, _, _, _)
| ClDiffFile::Moved(old_path, new_path, _, _, _) => {
counter_changes.push(Status::Removed(to_project_relative(&old_path)?));
counter_changes.push(Status::Added(to_project_relative(&new_path)?));
push_change_if_valid(
&mut counter_changes,
Status::Removed,
to_project_relative(&old_path),
);
push_change_if_valid(
&mut counter_changes,
Status::Added,
to_project_relative(&new_path),
);
}
}
}
Expand Down Expand Up @@ -128,22 +194,44 @@ mod tests {
fn test_normalize_change_path_for_repo_strips_repo_prefix_for_local_files() {
assert_eq!(
normalize_change_path_for_repo("/project/buck2_test", &PathBuf::from("src/main.rs")),
ProjectRelativePath::new("src/main.rs")
Some(ProjectRelativePath::new("src/main.rs"))
);
assert_eq!(
normalize_change_path_for_repo(
"/project/buck2_test",
&PathBuf::from("project/buck2_test/src/generated.rs")
),
ProjectRelativePath::new("src/generated.rs")
Some(ProjectRelativePath::new("src/generated.rs"))
);
}

#[test]
fn test_normalize_change_path_for_repo_keeps_external_shared_paths() {
assert_eq!(
normalize_change_path_for_repo("/project/buck2_test", &PathBuf::from("common/lib.rs")),
ProjectRelativePath::new("common/lib.rs")
Some(ProjectRelativePath::new("common/lib.rs"))
);
}

#[test]
fn test_normalize_change_path_for_repo_rejects_unsafe_paths() {
assert_eq!(
normalize_change_path_for_repo("/project/buck2_test", &PathBuf::from("../secret.rs")),
None
);
assert_eq!(
normalize_change_path_for_repo(
"/project/buck2_test",
&PathBuf::from("project/buck2_test/../../secret.rs")
),
None
);
assert_eq!(
normalize_change_path_for_repo(
"/project/buck2_test",
&PathBuf::from("project//buck2_test/src/main.rs")
),
None
);
}

Expand Down Expand Up @@ -184,4 +272,28 @@ mod tests {
]
);
}

#[test]
fn test_build_changes_filters_unsafe_paths() {
let changes = build_changes_for_repo(
"/project/buck2_test",
vec![
ClDiffFile::Modified(
PathBuf::from("src/main.rs"),
ObjectHash::from_str("1111111111111111111111111111111111111111").unwrap(),
ObjectHash::from_str("2222222222222222222222222222222222222222").unwrap(),
),
ClDiffFile::New(
PathBuf::from("../outside.rs"),
ObjectHash::from_str("3333333333333333333333333333333333333333").unwrap(),
),
],
)
.unwrap();

assert_eq!(
changes,
vec![Status::Modified(ProjectRelativePath::new("src/main.rs"))]
);
}
}
79 changes: 78 additions & 1 deletion ceres/src/code_edit/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,22 @@ pub(crate) trait Director<T: ApiHandler + Clone> {
}
}

fn cl_with_latest_to_hash(mut cl: mega_cl::Model, to_hash: &str) -> mega_cl::Model {
cl.to_hash = to_hash.to_string();
cl
}

fn fresh_or_fallback_cl(
original: mega_cl::Model,
fresh: Option<mega_cl::Model>,
to_hash: &str,
) -> mega_cl::Model {
match fresh {
Some(cl) => cl,
None => cl_with_latest_to_hash(original, to_hash),
}
}

pub(crate) struct CodeEditService<FMT, VT, AC, TCB, CK, HD, DR>
where
FMT: ConversationMessageFormater,
Expand Down Expand Up @@ -329,7 +345,14 @@ impl<
Some(cl) => {
self.update_existing_cl(cl.clone(), storage, &cl.from_hash, to_hash, username)
.await?;
Ok(cl)
let fresh = storage.cl_storage().get_cl(&cl.link).await?;
if fresh.is_none() {
tracing::warn!(
cl_link = %cl.link,
"CL was updated but fresh model lookup returned None; fallback to in-memory to_hash update."
);
}
Ok(fresh_or_fallback_cl(cl, fresh, to_hash))
}
None => Ok(self
.create_new_cl(storage, path_str, from_hash, to_hash, username)
Expand Down Expand Up @@ -381,3 +404,57 @@ impl<
Ok(())
}
}

#[cfg(test)]
mod tests {
use callisto::sea_orm_active_enums::MergeStatusEnum;

use super::{cl_with_latest_to_hash, fresh_or_fallback_cl};

fn sample_cl(to_hash: &str) -> callisto::mega_cl::Model {
callisto::mega_cl::Model {
id: 1,
link: "C1234567".to_string(),
title: "test".to_string(),
merge_date: None,
status: MergeStatusEnum::Open,
path: "/project/buck2_test".to_string(),
from_hash: "1111111111111111111111111111111111111111".to_string(),
to_hash: to_hash.to_string(),
created_at: chrono::Utc::now().naive_utc(),
updated_at: chrono::Utc::now().naive_utc(),
username: "tester".to_string(),
base_branch: "main".to_string(),
}
}

#[test]
fn test_fresh_or_fallback_cl_prefers_fresh_model() {
let original = sample_cl("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
let fresh = sample_cl("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");

let selected = fresh_or_fallback_cl(original, Some(fresh.clone()), "cccc");

assert_eq!(selected.to_hash, fresh.to_hash);
}

#[test]
fn test_fresh_or_fallback_cl_updates_to_hash_when_fresh_missing() {
let original = sample_cl("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
let selected =
fresh_or_fallback_cl(original, None, "cccccccccccccccccccccccccccccccccccccccc");

assert_eq!(selected.to_hash, "cccccccccccccccccccccccccccccccccccccccc");
}

#[test]
fn test_cl_with_latest_to_hash_updates_only_to_hash_field() {
let original = sample_cl("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
let updated =
cl_with_latest_to_hash(original.clone(), "dddddddddddddddddddddddddddddddddddddddd");

assert_eq!(updated.to_hash, "dddddddddddddddddddddddddddddddddddddddd");
assert_eq!(updated.link, original.link);
assert_eq!(updated.path, original.path);
}
}
Loading
Loading