Skip to content

Commit d3cdb7f

Browse files
committed
Merge remote-tracking branch 'origin/main' into fix/sprig-rolling-release-bootstrap
Signed-off-by: Brian Charbonneau <github@briancharbonneau.com>
2 parents 59ab051 + 4ff4064 commit d3cdb7f

3 files changed

Lines changed: 107 additions & 43 deletions

File tree

desktop/src-tauri/src/managed_agents/backend.rs

Lines changed: 50 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -430,25 +430,19 @@ pub(crate) fn redact_env_values_in(
430430
redact_secrets_with(s, &values)
431431
}
432432

433-
/// Copy a resolved provider into a private staging directory while hashing
434-
/// exactly the bytes copied. The staged file becomes non-writable before either
435-
/// invocation, closing the path replacement and in-place rewrite races.
436-
fn stage_provider(
433+
/// Copy a resolved provider into an unpublished candidate while hashing exactly
434+
/// the bytes copied. The caller owns the writable handle until publication.
435+
fn copy_provider_to_candidate(
437436
binary: &Path,
438-
) -> Result<(tempfile::TempDir, PathBuf, String, std::fs::File), String> {
439-
let directory = tempfile::Builder::new()
440-
.prefix("buzz-provider-")
441-
.tempdir()
442-
.map_err(|error| format!("failed to create provider staging directory: {error}"))?;
443-
let suffix = if cfg!(windows) { ".exe" } else { "" };
444-
let staged_path = directory.path().join(format!("provider{suffix}"));
437+
candidate_path: &Path,
438+
) -> Result<(std::fs::File, String), String> {
445439
let mut source = std::fs::File::open(binary)
446440
.map_err(|error| format!("failed to open provider for staging: {error}"))?;
447-
let mut staged = std::fs::OpenOptions::new()
441+
let mut candidate = std::fs::OpenOptions::new()
448442
.write(true)
449443
.create_new(true)
450-
.open(&staged_path)
451-
.map_err(|error| format!("failed to create staged provider: {error}"))?;
444+
.open(candidate_path)
445+
.map_err(|error| format!("failed to create staged provider candidate: {error}"))?;
452446
let mut hasher = Sha256::new();
453447
let mut buffer = [0_u8; 64 * 1024];
454448
loop {
@@ -458,18 +452,30 @@ fn stage_provider(
458452
if count == 0 {
459453
break;
460454
}
461-
staged
455+
candidate
462456
.write_all(&buffer[..count])
463-
.map_err(|error| format!("failed to write staged provider: {error}"))?;
457+
.map_err(|error| format!("failed to write staged provider candidate: {error}"))?;
464458
hasher.update(&buffer[..count]);
465459
}
466-
staged
460+
Ok((candidate, hex::encode(hasher.finalize())))
461+
}
462+
463+
/// Seal and close a provider candidate before atomically publishing the final
464+
/// executable pathname. Linux can reject execution with `ETXTBSY` when that
465+
/// pathname has an outstanding writer, even across a very short close/exec
466+
/// boundary, so the writable pathname is never also the executable pathname.
467+
fn publish_provider_candidate(
468+
candidate_path: &Path,
469+
staged_path: &Path,
470+
candidate: std::fs::File,
471+
) -> Result<(), String> {
472+
candidate
467473
.sync_all()
468-
.map_err(|error| format!("failed to sync staged provider: {error}"))?;
474+
.map_err(|error| format!("failed to sync staged provider candidate: {error}"))?;
469475

470-
let mut permissions = staged
476+
let mut permissions = candidate
471477
.metadata()
472-
.map_err(|error| format!("failed to inspect staged provider: {error}"))?
478+
.map_err(|error| format!("failed to inspect staged provider candidate: {error}"))?
473479
.permissions();
474480
#[cfg(unix)]
475481
{
@@ -478,9 +484,29 @@ fn stage_provider(
478484
}
479485
#[cfg(not(unix))]
480486
permissions.set_readonly(true);
481-
std::fs::set_permissions(&staged_path, permissions)
482-
.map_err(|error| format!("failed to protect staged provider: {error}"))?;
483-
drop(staged);
487+
std::fs::set_permissions(candidate_path, permissions)
488+
.map_err(|error| format!("failed to protect staged provider candidate: {error}"))?;
489+
drop(candidate);
490+
std::fs::rename(candidate_path, staged_path)
491+
.map_err(|error| format!("failed to publish staged provider atomically: {error}"))?;
492+
Ok(())
493+
}
494+
495+
/// Stage one immutable provider executable in a private directory. The final
496+
/// pathname is published only after the candidate is synced, protected, and
497+
/// closed; a read-only guard then keeps that inode alive across both calls.
498+
fn stage_provider(
499+
binary: &Path,
500+
) -> Result<(tempfile::TempDir, PathBuf, String, std::fs::File), String> {
501+
let directory = tempfile::Builder::new()
502+
.prefix("buzz-provider-")
503+
.tempdir()
504+
.map_err(|error| format!("failed to create provider staging directory: {error}"))?;
505+
let suffix = if cfg!(windows) { ".exe" } else { "" };
506+
let staged_path = directory.path().join(format!("provider{suffix}"));
507+
let candidate_path = directory.path().join(format!(".provider{suffix}.partial"));
508+
let (candidate, digest) = copy_provider_to_candidate(binary, &candidate_path)?;
509+
publish_provider_candidate(&candidate_path, &staged_path, candidate)?;
484510

485511
#[cfg(windows)]
486512
let execution_guard = {
@@ -496,12 +522,7 @@ fn stage_provider(
496522
let execution_guard = std::fs::File::open(&staged_path);
497523
let execution_guard = execution_guard
498524
.map_err(|error| format!("failed to lock staged provider for execution: {error}"))?;
499-
Ok((
500-
directory,
501-
staged_path,
502-
hex::encode(hasher.finalize()),
503-
execution_guard,
504-
))
525+
Ok((directory, staged_path, digest, execution_guard))
505526
}
506527

507528
/// Deploy through one immutable staged copy: negotiate protocol v1 before the

desktop/src-tauri/src/managed_agents/backend_tests.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,33 @@ fn env_secrets_from_request_includes_resolved_launch_maps() {
134134
}
135135
}
136136

137+
#[test]
138+
fn provider_candidate_is_unpublished_until_its_writer_is_closed() {
139+
let directory = tempfile::tempdir().unwrap();
140+
let source = directory.path().join("source-provider");
141+
let candidate = directory.path().join(".provider.partial");
142+
let executable = directory.path().join("provider");
143+
let bytes = b"provider-bytes";
144+
std::fs::write(&source, bytes).unwrap();
145+
146+
let (writer, digest) = copy_provider_to_candidate(&source, &candidate).unwrap();
147+
148+
assert!(candidate.exists(), "candidate must exist while writable");
149+
assert!(
150+
!executable.exists(),
151+
"the executable pathname must remain unpublished while the writer is open"
152+
);
153+
assert_eq!(digest, hex::encode(Sha256::digest(bytes)));
154+
155+
publish_provider_candidate(&candidate, &executable, writer).unwrap();
156+
157+
assert!(
158+
!candidate.exists(),
159+
"atomic publication must consume the candidate"
160+
);
161+
assert_eq!(std::fs::read(&executable).unwrap(), bytes);
162+
}
163+
137164
#[cfg(unix)]
138165
fn write_test_provider(path: &Path, body: &str) {
139166
use std::os::unix::fs::PermissionsExt;

desktop/tests/e2e/thread-focus-mode.spec.ts

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,37 @@ async function seedLongThread(page: import("@playwright/test").Page) {
3030
});
3131
}
3232

33-
async function topVisibleMessageId(
33+
async function scrollToMiddleVisibleMessage(
3434
body: import("@playwright/test").Locator,
35+
threadRootId: string,
3536
): Promise<string> {
36-
return body.evaluate((element) => {
37-
const top = element.getBoundingClientRect().top;
38-
const row = Array.from(
39-
element.querySelectorAll<HTMLElement>("[data-message-id]"),
40-
).find((candidate) => candidate.getBoundingClientRect().bottom > top);
41-
if (!row?.dataset.messageId) throw new Error("No visible thread anchor");
42-
return row.dataset.messageId;
43-
});
37+
let anchorId: string | null = null;
38+
await expect
39+
.poll(async () => {
40+
anchorId = await body.evaluate((element) => {
41+
const maxScrollTop = element.scrollHeight - element.clientHeight;
42+
if (maxScrollTop <= 0) return null;
43+
44+
const targetScrollTop = Math.floor(maxScrollTop * 0.4);
45+
element.scrollTop = targetScrollTop;
46+
element.dispatchEvent(new Event("scroll", { bubbles: true }));
47+
48+
if (Math.abs(element.scrollTop - targetScrollTop) > 1) return null;
49+
const bounds = element.getBoundingClientRect();
50+
const row = Array.from(
51+
element.querySelectorAll<HTMLElement>("[data-message-id]"),
52+
).find((candidate) => {
53+
const rect = candidate.getBoundingClientRect();
54+
return rect.bottom > bounds.top && rect.top < bounds.bottom;
55+
});
56+
return row?.dataset.messageId ?? null;
57+
});
58+
return anchorId !== null && anchorId !== threadRootId;
59+
})
60+
.toBe(true);
61+
62+
if (!anchorId) throw new Error("No visible middle-thread anchor");
63+
return anchorId;
4464
}
4565

4666
/**
@@ -181,11 +201,7 @@ test("focus and split preserve reading context and interaction ownership", async
181201
.toBe(true);
182202
await expect(channel).toHaveAttribute("inert", "");
183203

184-
await body.evaluate((element) => {
185-
element.scrollTop = element.scrollHeight * 0.4;
186-
element.dispatchEvent(new Event("scroll", { bubbles: true }));
187-
});
188-
const anchorId = await topVisibleMessageId(body);
204+
const anchorId = await scrollToMiddleVisibleMessage(body, rootId);
189205

190206
const focusModeToggle = page.getByRole("button", {
191207
name: "Show thread beside channel",

0 commit comments

Comments
 (0)