Skip to content
This repository was archived by the owner on Oct 19, 2026. It is now read-only.

Commit 5da8a68

Browse files
authored
Fix/improve buck2 build (#2046)
* fix(ceres): refresh cl to_hash and resolve push build root Signed-off-by: jackieismpc <jackieismpc@gmail.com> * fix(build): harden hybrid change-path normalization and target discovery Signed-off-by: jackieismpc <jackieismpc@gmail.com> * fix(orion): guard fuse panics and add runner preflight checks Signed-off-by: jackieismpc <jackieismpc@gmail.com> * chore(ceres): apply nightly fmt and remove dead-code warning Signed-off-by: jackieismpc <jackieismpc@gmail.com> * chore(orion): remove unused import in changes mapping Signed-off-by: jackieismpc <jackieismpc@gmail.com> * fix(orion): resolve clippy errors in changes mapping Signed-off-by: jackieismpc <jackieismpc@gmail.com> --------- Signed-off-by: jackieismpc <jackieismpc@gmail.com>
1 parent d0af03b commit 5da8a68

17 files changed

Lines changed: 697 additions & 97 deletions

File tree

‎.github/actions/prepare-orion-artifacts/action.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ runs:
2525
mv "${ARTIFACT_DIR}"/orion/runner-config/.env.prod "${ARTIFACT_DIR}"/.env
2626
mv "${ARTIFACT_DIR}"/orion/runner-config/scorpio.toml "${ARTIFACT_DIR}"/
2727
mv "${ARTIFACT_DIR}"/orion/runner-config/run.sh "${ARTIFACT_DIR}"/
28+
mv "${ARTIFACT_DIR}"/orion/runner-config/preflight.sh "${ARTIFACT_DIR}"/
2829
mv "${ARTIFACT_DIR}"/orion/runner-config/cleanup.sh "${ARTIFACT_DIR}"/
2930
3031
# Move systemd service file
@@ -40,6 +41,7 @@ runs:
4041
# Set executable permissions
4142
chmod +x "${ARTIFACT_DIR}"/orion
4243
chmod +x "${ARTIFACT_DIR}"/run.sh
44+
chmod +x "${ARTIFACT_DIR}"/preflight.sh
4345
chmod +x "${ARTIFACT_DIR}"/cleanup.sh
4446
4547
echo "==> Final artifact structure:"

‎.github/workflows/orion-client-deploy.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ jobs:
7979
orion/runner-config/.env.prod
8080
orion/runner-config/scorpio.toml
8181
orion/runner-config/run.sh
82+
orion/runner-config/preflight.sh
8283
orion/runner-config/cleanup.sh
8384
orion/systemd/orion-runner.service
8485
retention-days: 7

‎api-model/src/buck2/api.rs‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,10 @@ pub struct TaskBuildRequest {
2222
pub cl_link: String,
2323
//TODO: for old database only, delete after updated
2424
pub cl_id: i64,
25-
/// The list of changed files, expressed relative to the monorepo root.
26-
///
27-
/// Example values:
28-
/// - `jupiter/callisto/src/access_token.rs`
29-
/// - `common/lib.rs`
25+
/// The list of changed files in the hybrid path contract:
26+
/// - files inside `repo` are repo-relative (for example `src/main.rs`)
27+
/// - shared files outside `repo` stay monorepo-relative
28+
/// (for example `common/lib.rs`)
3029
pub changes: Vec<Status<ProjectRelativePath>>,
3130
/// Buck2 target path (e.g. //app:server). Optional for backward compatibility.
3231
#[serde(default, alias = "targets_path")]
@@ -45,7 +44,7 @@ pub struct RetryBuildRequest {
4544
pub build_id: String,
4645
pub cl_link: String,
4746
pub cl_id: i64,
48-
/// The list of changed files, expressed relative to the monorepo root.
47+
/// The list of changed files in the hybrid path contract used by Orion.
4948
pub changes: Vec<Status<ProjectRelativePath>>,
5049
pub targets: Option<Vec<String>>,
5150
}

‎api-model/src/buck2/ws.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ pub enum WSMessage {
1717
/// The Buck2 project path within the monorepo (for example `/jupiter/callisto`).
1818
repo: String,
1919
cl_link: String,
20-
/// The list of changed files, expressed relative to the monorepo root.
20+
/// The list of changed files in the hybrid path contract:
21+
/// repo-local files are repo-relative, shared files remain
22+
/// monorepo-relative.
2123
changes: Vec<Status<ProjectRelativePath>>,
2224
},
2325

‎ceres/src/build_trigger/changes_calculator.rs‎

Lines changed: 128 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use std::{
2-
path::{Path, PathBuf},
2+
path::{Component, Path, PathBuf},
33
sync::Arc,
44
};
55

@@ -14,51 +14,117 @@ use crate::{
1414
model::change_list::ClDiffFile,
1515
};
1616

17-
fn normalize_change_path_for_repo(repo_path: &str, path: &Path) -> ProjectRelativePath {
17+
fn is_safe_normalized_path(path: &str) -> bool {
18+
path.is_empty()
19+
|| (!path.contains("//")
20+
&& Path::new(path)
21+
.components()
22+
.all(|component| matches!(component, Component::Normal(_))))
23+
}
24+
25+
fn normalize_change_path_for_repo_with_prefix(
26+
repo_prefix: &str,
27+
repo_prefix_with_slash: Option<&str>,
28+
path: &Path,
29+
) -> Option<ProjectRelativePath> {
1830
let raw = path
1931
.to_string_lossy()
2032
.replace('\\', "/")
2133
.trim_start_matches('/')
2234
.to_string();
23-
let repo_prefix = repo_path.trim_matches('/');
2435

2536
let normalized = if repo_prefix.is_empty() {
2637
raw
2738
} else if raw == repo_prefix {
2839
String::new()
29-
} else if let Some(stripped) = raw.strip_prefix(&format!("{repo_prefix}/")) {
30-
stripped.to_string()
40+
} else if let Some(prefix) = repo_prefix_with_slash {
41+
if let Some(stripped) = raw.strip_prefix(prefix) {
42+
stripped.to_string()
43+
} else {
44+
raw
45+
}
3146
} else {
3247
raw
3348
};
3449

35-
ProjectRelativePath::new(&normalized)
50+
if !is_safe_normalized_path(&normalized) {
51+
tracing::warn!(
52+
path = %normalized,
53+
"Dropping unsafe build change path after normalization."
54+
);
55+
return None;
56+
}
57+
58+
Some(ProjectRelativePath::new(&normalized))
59+
}
60+
61+
#[cfg(test)]
62+
fn normalize_change_path_for_repo(repo_path: &str, path: &Path) -> Option<ProjectRelativePath> {
63+
let repo_prefix = repo_path.trim_matches('/');
64+
let repo_prefix_with_slash = (!repo_prefix.is_empty()).then(|| format!("{repo_prefix}/"));
65+
normalize_change_path_for_repo_with_prefix(repo_prefix, repo_prefix_with_slash.as_deref(), path)
66+
}
67+
68+
fn push_change_if_valid(
69+
changes: &mut Vec<Status<ProjectRelativePath>>,
70+
status_builder: impl FnOnce(ProjectRelativePath) -> Status<ProjectRelativePath>,
71+
normalized: Option<ProjectRelativePath>,
72+
) {
73+
if let Some(path) = normalized {
74+
changes.push(status_builder(path));
75+
}
3676
}
3777

3878
fn build_changes_for_repo(
3979
repo_path: &str,
4080
cl_diff_files: Vec<ClDiffFile>,
4181
) -> Result<Vec<Status<ProjectRelativePath>>, MegaError> {
42-
let to_project_relative = |path: &Path| -> Result<ProjectRelativePath, MegaError> {
43-
Ok(normalize_change_path_for_repo(repo_path, path))
82+
let repo_prefix = repo_path.trim_matches('/');
83+
let repo_prefix_with_slash = (!repo_prefix.is_empty()).then(|| format!("{repo_prefix}/"));
84+
let to_project_relative = |path: &Path| {
85+
normalize_change_path_for_repo_with_prefix(
86+
repo_prefix,
87+
repo_prefix_with_slash.as_deref(),
88+
path,
89+
)
4490
};
4591

4692
let mut counter_changes = Vec::new();
4793
for change in cl_diff_files {
4894
match change {
4995
ClDiffFile::New(path, _) => {
50-
counter_changes.push(Status::Added(to_project_relative(&path)?));
96+
push_change_if_valid(
97+
&mut counter_changes,
98+
Status::Added,
99+
to_project_relative(&path),
100+
);
51101
}
52102
ClDiffFile::Deleted(path, _) => {
53-
counter_changes.push(Status::Removed(to_project_relative(&path)?));
103+
push_change_if_valid(
104+
&mut counter_changes,
105+
Status::Removed,
106+
to_project_relative(&path),
107+
);
54108
}
55109
ClDiffFile::Modified(path, _, _) => {
56-
counter_changes.push(Status::Modified(to_project_relative(&path)?));
110+
push_change_if_valid(
111+
&mut counter_changes,
112+
Status::Modified,
113+
to_project_relative(&path),
114+
);
57115
}
58116
ClDiffFile::Renamed(old_path, new_path, _, _, _)
59117
| ClDiffFile::Moved(old_path, new_path, _, _, _) => {
60-
counter_changes.push(Status::Removed(to_project_relative(&old_path)?));
61-
counter_changes.push(Status::Added(to_project_relative(&new_path)?));
118+
push_change_if_valid(
119+
&mut counter_changes,
120+
Status::Removed,
121+
to_project_relative(&old_path),
122+
);
123+
push_change_if_valid(
124+
&mut counter_changes,
125+
Status::Added,
126+
to_project_relative(&new_path),
127+
);
62128
}
63129
}
64130
}
@@ -128,22 +194,44 @@ mod tests {
128194
fn test_normalize_change_path_for_repo_strips_repo_prefix_for_local_files() {
129195
assert_eq!(
130196
normalize_change_path_for_repo("/project/buck2_test", &PathBuf::from("src/main.rs")),
131-
ProjectRelativePath::new("src/main.rs")
197+
Some(ProjectRelativePath::new("src/main.rs"))
132198
);
133199
assert_eq!(
134200
normalize_change_path_for_repo(
135201
"/project/buck2_test",
136202
&PathBuf::from("project/buck2_test/src/generated.rs")
137203
),
138-
ProjectRelativePath::new("src/generated.rs")
204+
Some(ProjectRelativePath::new("src/generated.rs"))
139205
);
140206
}
141207

142208
#[test]
143209
fn test_normalize_change_path_for_repo_keeps_external_shared_paths() {
144210
assert_eq!(
145211
normalize_change_path_for_repo("/project/buck2_test", &PathBuf::from("common/lib.rs")),
146-
ProjectRelativePath::new("common/lib.rs")
212+
Some(ProjectRelativePath::new("common/lib.rs"))
213+
);
214+
}
215+
216+
#[test]
217+
fn test_normalize_change_path_for_repo_rejects_unsafe_paths() {
218+
assert_eq!(
219+
normalize_change_path_for_repo("/project/buck2_test", &PathBuf::from("../secret.rs")),
220+
None
221+
);
222+
assert_eq!(
223+
normalize_change_path_for_repo(
224+
"/project/buck2_test",
225+
&PathBuf::from("project/buck2_test/../../secret.rs")
226+
),
227+
None
228+
);
229+
assert_eq!(
230+
normalize_change_path_for_repo(
231+
"/project/buck2_test",
232+
&PathBuf::from("project//buck2_test/src/main.rs")
233+
),
234+
None
147235
);
148236
}
149237

@@ -184,4 +272,28 @@ mod tests {
184272
]
185273
);
186274
}
275+
276+
#[test]
277+
fn test_build_changes_filters_unsafe_paths() {
278+
let changes = build_changes_for_repo(
279+
"/project/buck2_test",
280+
vec![
281+
ClDiffFile::Modified(
282+
PathBuf::from("src/main.rs"),
283+
ObjectHash::from_str("1111111111111111111111111111111111111111").unwrap(),
284+
ObjectHash::from_str("2222222222222222222222222222222222222222").unwrap(),
285+
),
286+
ClDiffFile::New(
287+
PathBuf::from("../outside.rs"),
288+
ObjectHash::from_str("3333333333333333333333333333333333333333").unwrap(),
289+
),
290+
],
291+
)
292+
.unwrap();
293+
294+
assert_eq!(
295+
changes,
296+
vec![Status::Modified(ProjectRelativePath::new("src/main.rs"))]
297+
);
298+
}
187299
}

‎ceres/src/code_edit/model.rs‎

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,22 @@ pub(crate) trait Director<T: ApiHandler + Clone> {
136136
}
137137
}
138138

139+
fn cl_with_latest_to_hash(mut cl: mega_cl::Model, to_hash: &str) -> mega_cl::Model {
140+
cl.to_hash = to_hash.to_string();
141+
cl
142+
}
143+
144+
fn fresh_or_fallback_cl(
145+
original: mega_cl::Model,
146+
fresh: Option<mega_cl::Model>,
147+
to_hash: &str,
148+
) -> mega_cl::Model {
149+
match fresh {
150+
Some(cl) => cl,
151+
None => cl_with_latest_to_hash(original, to_hash),
152+
}
153+
}
154+
139155
pub(crate) struct CodeEditService<FMT, VT, AC, TCB, CK, HD, DR>
140156
where
141157
FMT: ConversationMessageFormater,
@@ -329,7 +345,14 @@ impl<
329345
Some(cl) => {
330346
self.update_existing_cl(cl.clone(), storage, &cl.from_hash, to_hash, username)
331347
.await?;
332-
Ok(cl)
348+
let fresh = storage.cl_storage().get_cl(&cl.link).await?;
349+
if fresh.is_none() {
350+
tracing::warn!(
351+
cl_link = %cl.link,
352+
"CL was updated but fresh model lookup returned None; fallback to in-memory to_hash update."
353+
);
354+
}
355+
Ok(fresh_or_fallback_cl(cl, fresh, to_hash))
333356
}
334357
None => Ok(self
335358
.create_new_cl(storage, path_str, from_hash, to_hash, username)
@@ -381,3 +404,57 @@ impl<
381404
Ok(())
382405
}
383406
}
407+
408+
#[cfg(test)]
409+
mod tests {
410+
use callisto::sea_orm_active_enums::MergeStatusEnum;
411+
412+
use super::{cl_with_latest_to_hash, fresh_or_fallback_cl};
413+
414+
fn sample_cl(to_hash: &str) -> callisto::mega_cl::Model {
415+
callisto::mega_cl::Model {
416+
id: 1,
417+
link: "C1234567".to_string(),
418+
title: "test".to_string(),
419+
merge_date: None,
420+
status: MergeStatusEnum::Open,
421+
path: "/project/buck2_test".to_string(),
422+
from_hash: "1111111111111111111111111111111111111111".to_string(),
423+
to_hash: to_hash.to_string(),
424+
created_at: chrono::Utc::now().naive_utc(),
425+
updated_at: chrono::Utc::now().naive_utc(),
426+
username: "tester".to_string(),
427+
base_branch: "main".to_string(),
428+
}
429+
}
430+
431+
#[test]
432+
fn test_fresh_or_fallback_cl_prefers_fresh_model() {
433+
let original = sample_cl("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
434+
let fresh = sample_cl("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
435+
436+
let selected = fresh_or_fallback_cl(original, Some(fresh.clone()), "cccc");
437+
438+
assert_eq!(selected.to_hash, fresh.to_hash);
439+
}
440+
441+
#[test]
442+
fn test_fresh_or_fallback_cl_updates_to_hash_when_fresh_missing() {
443+
let original = sample_cl("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
444+
let selected =
445+
fresh_or_fallback_cl(original, None, "cccccccccccccccccccccccccccccccccccccccc");
446+
447+
assert_eq!(selected.to_hash, "cccccccccccccccccccccccccccccccccccccccc");
448+
}
449+
450+
#[test]
451+
fn test_cl_with_latest_to_hash_updates_only_to_hash_field() {
452+
let original = sample_cl("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
453+
let updated =
454+
cl_with_latest_to_hash(original.clone(), "dddddddddddddddddddddddddddddddddddddddd");
455+
456+
assert_eq!(updated.to_hash, "dddddddddddddddddddddddddddddddddddddddd");
457+
assert_eq!(updated.link, original.link);
458+
assert_eq!(updated.path, original.path);
459+
}
460+
}

0 commit comments

Comments
 (0)