Skip to content
Closed
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
3 changes: 2 additions & 1 deletion .github/workflows/native.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ jobs:
run: rustup toolchain install 1.95.0 --profile minimal --component rustfmt --component clippy
- run: cargo +1.95.0 fmt --all --check
- run: cargo +1.95.0 clippy --workspace --all-targets --locked -- -D warnings
- run: cargo +1.95.0 test --workspace --locked
# qualification_real spawns target/debug/shadowcode; cargo test does not build it.
- run: cargo +1.95.0 build -p shadowcode-desktop --locked
- run: cargo +1.95.0 test --workspace --locked
- name: Exercise the native CLI without a display
run: node scripts/test-native-cli.mjs
- name: Stress repeated native tasks, cancellation, output and cleanup
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ jobs:
run: |
cargo +1.95.0 fmt --all --check
cargo +1.95.0 clippy --workspace --all-targets --locked -- -D warnings
cargo +1.95.0 test --workspace --locked
# qualification_real spawns target/debug/shadowcode; cargo test does not build it.
cargo +1.95.0 build -p shadowcode-desktop --locked
cargo +1.95.0 test --workspace --locked
- name: Exercise native application behavior
run: |
node scripts/test-native-cli.mjs
Expand Down
48 changes: 46 additions & 2 deletions native/core/src/control/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ pub struct ViewClient {
reader: Mutex<Option<AbortOnDropHandle<Result<()>>>>,
events: broadcast::Sender<Value>,
closed: Arc<AtomicBool>,
closing: Arc<AtomicBool>,
attach: Mutex<()>,
}
impl Client {
Expand Down Expand Up @@ -126,6 +127,7 @@ impl Client {
reader: Mutex::new(Some(AbortOnDropHandle::new(task))),
events,
closed,
closing: Arc::new(AtomicBool::new(false)),
attach: Mutex::new(()),
})
}
Expand Down Expand Up @@ -158,14 +160,28 @@ impl ViewClient {
/// tools, or emit durable events. Existing subscribers keep this
/// broadcast and receive `view.reattached`.
pub async fn reattach(&self) -> Result<Value> {
let _gate = self.attach.lock().await;
ensure!(
!self.closing.load(Ordering::Acquire),
"Attached view is closed"
);
ensure!(
self.closed.load(Ordering::Acquire),
"Attached view is still connected; detach before reattaching"
);
let mut base = self.client()?;
base.view = None;
base.wait_available(Duration::from_secs(20)).await?;
// Do not hold `attach` while waiting: close() must stay able to
// detach after the owner exits instead of blocking for 20s.
self.wait_for_owner(&base, Duration::from_secs(20)).await?;
let _gate = self.attach.lock().await;
ensure!(
!self.closing.load(Ordering::Acquire),
"Attached view is closed"
);
ensure!(
self.closed.load(Ordering::Acquire),
"Attached view is still connected; detach before reattaching"
);
let _ = self.reader.lock().await.take();
let _ = self.writer.lock().await.take();
let fresh = base.open_view().await?;
Expand Down Expand Up @@ -195,7 +211,34 @@ impl ViewClient {
"tools_replayed": 0,
}))
}
async fn wait_for_owner(&self, client: &Client, timeout: Duration) -> Result<()> {
let deadline = tokio::time::Instant::now() + timeout;
loop {
ensure!(
!self.closing.load(Ordering::Acquire),
"Attached view is closed"
);
match client.available().await {
Ok(true) => return Ok(()),
Ok(false) if tokio::time::Instant::now() < deadline => {
tokio::time::sleep(Duration::from_millis(100)).await;
}
Ok(false) => bail!("No running engine is available to attach"),
Err(error)
if error.to_string().contains("different version")
|| error.to_string().contains("protocol/profile mismatch") =>
{
return Err(error);
}
Err(_error) if tokio::time::Instant::now() < deadline => {
tokio::time::sleep(Duration::from_millis(100)).await;
}
Err(error) => return Err(error),
}
}
}
pub async fn close(&self) -> Result<()> {
self.closing.store(true, Ordering::Release);
let _gate = self.attach.lock().await;
self.closed.store(true, Ordering::Release);
let Some(mut task) = self.reader.lock().await.take() else {
Expand Down Expand Up @@ -317,6 +360,7 @@ mod tests {
reader: Mutex::new(Some(AbortOnDropHandle::new(task))),
events,
closed: Arc::new(AtomicBool::new(false)),
closing: Arc::new(AtomicBool::new(false)),
attach: Mutex::new(()),
});
let closing_view = view.clone();
Expand Down
60 changes: 31 additions & 29 deletions native/core/tests/control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,37 @@ async fn attached_views_receive_bounded_completion_notifications_and_detect_owne
service.engine.shutdown().await.unwrap();
}

#[tokio::test]
async fn attached_view_close_does_not_wait_for_missing_owner() {
let (_root, service) = setup();
let server = Server::start_with_mode(service.clone(), "server").unwrap();
let client = server.endpoint().client(service.workspace().unwrap(), None);
let view = std::sync::Arc::new(client.open_view().await.unwrap());
server.close();
server.wait_closed().await;
let reattach = {
let view = view.clone();
tokio::spawn(async move { view.reattach().await })
};
tokio::time::sleep(Duration::from_millis(150)).await;
let started = std::time::Instant::now();
let _ = view.close().await;
assert!(
started.elapsed() < Duration::from_secs(2),
"close waited for owner reconnect: {:?}",
started.elapsed()
);
let reattach = tokio::time::timeout(Duration::from_secs(2), reattach)
.await
.expect("reattach should stop when the view closes")
.unwrap();
assert!(
reattach.is_err(),
"reattach must not succeed after close: {reattach:?}"
);
service.engine.shutdown().await.unwrap();
}

#[tokio::test]
async fn attached_view_reattaches_after_owner_restart_without_replay_or_duplicates() {
let (_root, service) = setup();
Expand Down Expand Up @@ -598,32 +629,3 @@ async fn attached_view_reattaches_after_owner_restart_without_replay_or_duplicat
assert_eq!(running, 0, "reattach must not start or resume jobs");
stop(server, service).await;
}

fn descriptor_count() -> usize {
std::fs::read_dir("/proc/self/fd")
.map(|entries| entries.count())
.unwrap_or(0)
}

#[tokio::test]
async fn attach_close_loop_does_not_grow_descriptors() {
let (_root, service) = setup();
let server = Server::start_with_mode(service.clone(), "server").unwrap();
let client = server.endpoint().client(service.workspace().unwrap(), None);
for _ in 0..2 {
let view = client.open_view().await.unwrap();
view.close().await.unwrap();
}
let baseline = descriptor_count();
for _ in 0..20 {
let view = client.open_view().await.unwrap();
view.close().await.unwrap();
}
let after = descriptor_count();
eprintln!("attach_close_loop descriptors baseline={baseline} after={after}");
assert!(
after <= baseline + 24,
"descriptor leak: baseline={baseline} after={after}"
);
stop(server, service).await;
}
49 changes: 49 additions & 0 deletions native/core/tests/control_descriptors.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
//! Process-wide `/proc/self/fd` counts. This file is a separate integration
//! binary so sibling `control` tests cannot inflate the measurement.
use serde_json::json;
use shadowcode_core::{config::Config, control::Server, paths::AppPaths, service::Service};
use std::fs;

fn setup() -> (tempfile::TempDir, Service) {
let root = tempfile::tempdir().unwrap();
let workspace = root.path().join("project");
fs::create_dir(&workspace).unwrap();
let paths = AppPaths::isolated(&root.path().join("profile")).unwrap();
Config::patch(&paths,json!({"trusted_workspaces":[workspace],"model":{"provider":"local","name":"fixture","endpoint":"http://127.0.0.1:9/v1"},"permissions":{"approve_shell":false}})).unwrap();
(root, Service::open(paths, Some(workspace)).unwrap())
}

async fn stop(server: Server, service: Service) {
server.close();
server.wait_closed().await;
service.engine.shutdown().await.unwrap();
}

fn descriptor_count() -> usize {
std::fs::read_dir("/proc/self/fd")
.map(|entries| entries.count())
.unwrap_or(0)
}

#[tokio::test]
async fn attach_close_loop_does_not_grow_descriptors() {
let (_root, service) = setup();
let server = Server::start_with_mode(service.clone(), "server").unwrap();
let client = server.endpoint().client(service.workspace().unwrap(), None);
for _ in 0..2 {
let view = client.open_view().await.unwrap();
view.close().await.unwrap();
}
let baseline = descriptor_count();
for _ in 0..20 {
let view = client.open_view().await.unwrap();
view.close().await.unwrap();
}
let after = descriptor_count();
eprintln!("attach_close_loop descriptors baseline={baseline} after={after}");
assert!(
after <= baseline + 24,
"descriptor leak: baseline={baseline} after={after}"
);
stop(server, service).await;
}
34 changes: 34 additions & 0 deletions scripts/native-packaging-env.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,45 @@ export function isUnsafePackagingDir(dir) {
);
}

function dirContains(dir, name) {
try {
statSync(path.join(dir, name));
return true;
} catch {
return false;
}
}

// rustc/cargo live on the caller PATH (rustup ~/.cargo/bin, CARGO_HOME).
// linuxdeploy still must not see /usr/local/bin or Hermes; keep only the
// toolchain directories themselves when they pass the same safety checks.
function rustToolchainDirs() {
const candidates = [];
if (process.env.CARGO_HOME) {
candidates.push(path.join(process.env.CARGO_HOME, "bin"));
}
if (process.env.HOME) {
candidates.push(path.join(process.env.HOME, ".cargo", "bin"));
}
for (const dir of (process.env.PATH || "").split(path.delimiter)) {
if (!dir) continue;
const resolved = resolveExistingDir(dir);
if (
resolved &&
(dirContains(resolved, "rustc") || dirContains(resolved, "cargo"))
) {
candidates.push(resolved);
}
}
return candidates;
}

export function packagingDirs(root, options = {}) {
const execDir = options.execDir ?? path.dirname(process.execPath);
const extras = [
path.join(root, "tools/rust-dev/extracted/usr/bin"),
path.join(root, "..", "tools/rust-dev/extracted/usr/bin"),
...rustToolchainDirs(),
path.join(root, "target/release"),
path.join(root, "target/debug"),
path.join(root, "target/.tauri"),
Expand Down
10 changes: 8 additions & 2 deletions scripts/test-native-cli.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ import { fileURLToPath } from "node:url";
import { DatabaseSync } from "node:sqlite";

const root = fileURLToPath(new URL("../", import.meta.url));
const version = JSON.parse(
await readFile(path.join(root, "src-tauri/tauri.conf.json"), "utf8"),
).version;
const versionLine = new RegExp(
`^ShadowCode ${String(version).replaceAll(".", "\\.")}\\s*$`,
);
const binary = process.env.SHADOW_DESKTOP_BINARY || path.join(root, "target/debug/shadowcode");
const binaryArgs = JSON.parse(process.env.SHADOW_CLI_ARGS || "[]");
assert.ok(Array.isArray(binaryArgs) && binaryArgs.every(arg => typeof arg === "string"));
Expand Down Expand Up @@ -124,8 +130,8 @@ await writeFile(path.join(profile, "config/config.yaml"), JSON.stringify({
}));
let server;
try {
assert.match((await finish(launch(["--version"]))).stdout, /^ShadowCode 0\.20\.0\s*$/);
assert.match((await finish(launch(["ui", "--version"]))).stdout, /^ShadowCode 0\.20\.0\s*$/);
assert.match((await finish(launch(["--version"]))).stdout, versionLine);
assert.match((await finish(launch(["ui", "--version"]))).stdout, versionLine);
assert.match((await finish(launch(["--help"]))).stdout, /serve/);
await finish(launch(["--unknown-option"]), 2);
assert.equal((await cli(["health"])).runtime, "rust");
Expand Down
10 changes: 10 additions & 0 deletions scripts/test-native-packaging-env.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ test("packaging PATH is constructed, not inherited", () => {
assert.doesNotMatch(joined, /\.hermes/);
const rustDev = path.resolve(root, "../tools/rust-dev/extracted/usr/bin");
if (existsSync(rustDev)) assert.ok(dirs.includes(rustDev));
const rustcDir = dirs.find((dir) => existsSync(path.join(dir, "rustc")));
assert.ok(
rustcDir,
"sanitized PATH must still find rustc for dependency notices",
);
assert.notEqual(path.resolve(rustcDir), "/usr/local/bin");
const tauriTools = path.join(root, "target/.tauri");
if (existsSync(tauriTools)) assert.ok(dirs.includes(tauriTools));
});
Expand All @@ -64,6 +70,10 @@ test("applyPackagingPath overwrites a dirty process PATH", () => {
assert.doesNotMatch(next, /(^|:)\/usr\/local\/bin(:|$)/);
assert.match(next, /(^|:)\/usr\/bin(:|$)/);
assert.notEqual(next, dirtyPath);
assert.ok(
next.split(path.delimiter).some((dir) => existsSync(path.join(dir, "rustc"))),
"dirty caller PATH must not drop rustc",
);
} finally {
process.env.PATH = previous;
}
Expand Down
10 changes: 8 additions & 2 deletions scripts/test-native-runtime.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ import { tmpdir } from "node:os";
import path from "node:path";
import { fileURLToPath } from "node:url";
const root = fileURLToPath(new URL("../", import.meta.url));
const version = JSON.parse(
await readFile(path.join(root, "src-tauri/tauri.conf.json"), "utf8"),
).version;
const versionLine = new RegExp(
`^ShadowCode ${String(version).replaceAll(".", "\\.")}\\s*$`,
);
const binary = path.resolve(
process.argv[2] ||
path.join(
Expand Down Expand Up @@ -224,10 +230,10 @@ try {
Array.from({ length: 8 }, () => finish(launch(["--version"]))),
);
for (const reply of replies)
assert.match(reply.stdout, /^ShadowCode 0\.20\.0\s*$/);
assert.match(reply.stdout, versionLine);
assert.match(
(await finish(launch(["ui", "--version"], {}, true))).stdout,
/^ShadowCode 0\.20\.0\s*$/,
versionLine,
"Environment-based extraction preserves arguments",
);
await intact(first);
Expand Down
Loading