Skip to content
Draft
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
6 changes: 6 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,12 @@ jobs:
if ($LASTEXITCODE -ne 0) { throw "Computer helper ownership tests failed" }
cargo test --locked -p webcodex-environment --lib windows_helper_task_scheduler_roundtrip_is_owned -- --ignored
if ($LASTEXITCODE -ne 0) { throw "Computer helper Task Scheduler roundtrip failed" }
- name: Test Windows Tunnel heartbeat contention and recovery
run: |
cargo test --locked -p webcodex-environment --lib tunnel::health_tests
if ($LASTEXITCODE -ne 0) { throw "Windows Tunnel health file tests failed" }
cargo test --locked -p webcodex --lib server_tunnels
if ($LASTEXITCODE -ne 0) { throw "Windows Tunnel heartbeat supervision tests failed" }
- name: Test Windows unified installer routing and Desktop build freshness
run: python -m unittest scripts.tests.test_prepare_windows_unified_installer scripts.tests.test_desktop_runtime_manifest scripts.tests.test_desktop_frontend_build
- name: Test optional Codex observation adapter on Windows
Expand Down
2 changes: 1 addition & 1 deletion crates/webcodex-environment/src/tunnel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1893,7 +1893,7 @@ pub(crate) fn read_tunnel_health(
})
}

#[cfg(all(test, unix))]
#[cfg(test)]
mod health_tests;

#[cfg(test)]
Expand Down
43 changes: 43 additions & 0 deletions crates/webcodex-environment/src/tunnel/health_tests.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::*;

#[test]
#[cfg(unix)]
fn health_locks_release_without_waiting_for_inherited_handles() {
for exclusive in [false, true] {
for fail_after_lock in [false, true] {
Expand Down Expand Up @@ -53,3 +54,45 @@ fn health_locks_release_without_waiting_for_inherited_handles() {
}
}
}

#[test]
fn missed_heartbeats_expire_during_contention_and_recover_after_unlock() {
let temp = crate::test_tempdir().unwrap();
let store = EnvironmentStore::open(temp.path().join("environment")).unwrap();
let path = store.root().join("readiness.json");
let stale = serde_json::json!({
"schema_version": 1,
"observed_at_ms": 0,
"service_pid": std::process::id(),
"profile_revision": 7,
"state": "running",
"tunnel_ready": true,
"local_mcp_ready": true,
});
atomic_private_write(&path, stale.to_string().as_bytes()).unwrap();
let file = std::fs::File::open(&path).unwrap();
fs2::FileExt::try_lock_shared(&file).unwrap();
let reader = TunnelHealthFileLock { file };
for _ in 0..4 {
assert!(write_embedded_tunnel_observation(
&path,
7,
TunnelState::Running,
true,
true,
None,
)
.is_err());
let health = read_tunnel_health(&path).unwrap();
assert!(!health.fresh);
assert!(!health.tunnel_ready && !health.local_mcp_ready);
assert_eq!(health.state, Some(TunnelState::Running));
assert_eq!(health.profile_revision, Some(7));
}
drop(reader);
write_embedded_tunnel_observation(&path, 7, TunnelState::Running, true, true, None).unwrap();
let health = read_tunnel_health(&path).unwrap();
assert!(health.fresh && health.tunnel_ready && health.local_mcp_ready);
assert_eq!(health.service_pid, Some(std::process::id()));
assert_eq!(health.profile_revision, Some(7));
}
10 changes: 10 additions & 0 deletions docs/architecture/server-owned-tunnels.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,16 @@ existing per-profile service for standalone. Readiness files keep the existing
freshness contract. Transport readiness is not evidence of a successful client
operation, and process liveness is not either readiness bit.

The initial status write must succeed before a profile launches. Once its Tunnel
task is owned, a heartbeat publication error (including a concurrent reader's
shared file lock) does not end or restart that task. Runtime status keeps the live
readiness bits and reports `tunnel_health_unavailable` until publication succeeds
at a later normal two-second health tick. Only failure/recovery transitions are
logged. A missed write does not refresh the saved observation: readers still
reject it after seven seconds. The writer retains the precreated file, its private
ACL and its nonblocking lock; it never replaces the file or bypasses validation.
Task failure, startup deadlines, shutdown and restart-fence semantics are unchanged.

Server termination runs these phases in order:

1. Stop new Tunnel polling/admission.
Expand Down
83 changes: 83 additions & 0 deletions docs/reviews/2026-11-13-tunnel-heartbeat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# Tunnel heartbeat recovery backport — 2026-11-13

## Provenance and scope

Backport upstream [#1411](https://github.com/yyjeqhc/webcodex/pull/1411),
commit `6d50934b451c9a524cf9077dbc67656c2a8e5129`, unchanged onto maintained
`integrate/reviewed-upstream-20261020` at `8340dd0fc9192c2a0cc594e0d754ead83578a607`.
The reconstructed baseline tree is exactly `cbeeb57cdcc04aaab19830378d241e6b34825d09`.
The upstream six-file patch applies without adaptation. Existing fork diagnostics,
process regressions, full-history recovery and Windows path fixes are retained.

Heartbeat persistence failure no longer terminates the owned Tunnel task. It records
the diagnostic and retries at the normal cadence. Initial publication remains a
launch prerequisite. File validation, private ACL, nonblocking locking, seven-second
saved-observation expiry, shutdown and task-failure semantics remain unchanged.

## Actual validation

- `cargo fmt --all -- --check`: passed.
- `python3 scripts/check_markdown_links.py`: 254 files, 1081 local links, zero missing
before adding this report; final report is checked again before publication.
- `python3 -m unittest scripts.tests.test_ci_contract_ownership scripts.tests.test_ci_path_risk`:
43 tests passed; independent reviewer reran these successfully.
- `git diff --check`: passed.
- Independent static review found no blocking concern.
- `cargo test --locked --offline -p webcodex-environment --lib tunnel::health_tests`:
exit 101, required `atomic-waker v1.1.2` was not cached.
- Online retry with the same locked dependencies: dependency download rejected with
HTTP 403 by the cloud network policy. No Rust test execution is claimed.

Exact-head fork CI is required before merge. This draft preserves the candidate and
lets normal CI validate it; no skipped check is treated as native acceptance.
No real Host, Windows/macOS, deployed Server/Runner or restart acceptance is claimed.
No deployment, release, credentials, permissions, proxy settings or service restart.

### First exact-head CI

[Run 43354889474](https://github.com/franksong2702/webcodex/actions/runs/43354889474)
on `9bc6de5119d01a4153050638bd4ad2702c80742f` ran the Linux Server, Runner and
workspace-crate suites successfully. Windows core executed the new contention /
expiry test (1 passed) and Server heartbeat tests (3 passed). This is CI evidence,
not deployed Host acceptance or a successful overall run.

Tooling and contract lanes failed in the unchanged build-identity fixture:
`test_packed_branch_without_reflog_refreshes_and_then_stays_cached` and
`test_tracked_dirty_state_refreshes_at_same_head` each failed for normal and linked
worktrees because a hard-coded Cargo output glob returned zero files. CI installed
Rust 1.100.0, updated November 12. The old local 1.99.1 fixture passes; the original
four failures remain recorded and are not waived.

The follow-up fixture uses Cargo JSON `build-script-executed.out_dir` and the
matching executable artifact instead of guessing package-directory spelling.
It still requires exactly one build script and executable, the same package ID,
one stable output path, unchanged cached-output timestamps, and all original
identity/dirty-state assertions. No production build behavior or gate is removed.
Independent review of the delta and all six build-identity tests passed locally
with Rust/Cargo 1.99.1; Rust 1.100.0 validation remains the new exact-head CI gate.

The second run, [43357001385](https://github.com/franksong2702/webcodex/actions/runs/43357001385),
exposed an incomplete first adaptation: Cargo 1.100 also no longer provides the
internal sibling `output` file. Its Linux Rust, Docker, Windows and macOS execution
lanes passed, but the tooling/contract gates still failed; aggregate failures and
MCP-conformance skip are not counted as success.

The corrected fixture now wraps the unchanged production collector only inside
its disposable test project and writes a private execution marker under documented
`OUT_DIR`. The marker observes actual build-script execution, without relying on
Cargo's private directory or output layout. Production source is unchanged.

## Other upstream changes inspected

Upstream main is `3c1419678e9c5f513e1f0fcc24d48ca1cd897f93`; latest stable is
[v0.5.0](https://github.com/yyjeqhc/webcodex/releases/tag/v0.5.0).
Git ancestry comparison reports upstream ahead 158 / behind 8 against maintenance;
this is not a count of missing behaviors because some patches were backported.
Fork main is not evidence of the installed version.

[#1418](https://github.com/yyjeqhc/webcodex/pull/1418) introduces a default-off
loopback proxy path with a DNS-pinning trust tradeoff. It is not enabled or included.
[#1415](https://github.com/yyjeqhc/webcodex/pull/1415) improves project catalog latency
but spans inventory generations, cancellation and eight files; deferred from this
small recovery fix. Existing fork [PR #2](https://github.com/franksong2702/webcodex/pull/2)
remains separate; no duplicate work or close/reopen attempt was made.
111 changes: 94 additions & 17 deletions scripts/tests/test_build_identity.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,69 @@


class BuildIdentityTests(unittest.TestCase):
def install_observed_build_script(self, package):
# Instrument only the disposable fixture. Run the shipped collector
# unchanged, then record execution in the documented OUT_DIR workspace.
source = BUILD_SCRIPT.read_text(encoding="utf-8")
self.assertEqual(source.count("fn main() {"), 1)
source = source.replace("fn main() {", "fn collect_build_identity() {", 1)
source += r'''
fn main() {
collect_build_identity();
let marker = std::path::PathBuf::from(std::env::var_os("OUT_DIR").unwrap())
.join("fixture-build-executions");
let previous = std::fs::read_to_string(&marker)
.map(|value| value.parse::<u64>().unwrap())
.unwrap_or(0);
std::fs::write(marker, (previous + 1).to_string()).unwrap();
}
'''
(package / "build.rs").write_text(source, encoding="utf-8")

@staticmethod
def marker_state(output):
return int(output.read_text(encoding="utf-8")), output.stat().st_mtime_ns

def build_fixture(self, run, package):
# Cargo's build-directory package spelling is not a stable interface.
# Resolve the one dependency-free fixture from its structured messages.
messages = run(package, "cargo", "build", "--offline", "--quiet", "--message-format=json")
events = [json.loads(line) for line in messages.splitlines()]
scripts = [event for event in events if event.get("reason") == "build-script-executed"]
binaries = [event for event in events if event.get("reason") == "compiler-artifact"
and event.get("executable")]
self.assertEqual(len(scripts), 1, scripts)
self.assertEqual(len(binaries), 1, binaries)
self.assertEqual(scripts[0]["package_id"], binaries[0]["package_id"])
output = Path(scripts[0]["out_dir"]) / "fixture-build-executions"
self.assertTrue(output.is_file(), output)
return run(package, binaries[0]["executable"]), output

def test_fixture_output_path_follows_cargo_messages(self):
for directory in ("identity-fixture-hash", "identity_fixture-hash", "identity-fixture/hash"):
with self.subTest(directory=directory), tempfile.TemporaryDirectory() as tmp:
package = Path(tmp) / "package"
out_dir = Path(tmp) / "custom-build-dir" / directory / "out"
out_dir.mkdir(parents=True)
output = out_dir / "fixture-build-executions"
output.write_text("1", encoding="utf-8")
self.assertFalse((out_dir.parent / "output").exists())
executable = str(Path(tmp) / "fixture-bin")
events = [
{"reason": "build-script-executed", "package_id": "fixture", "out_dir": str(out_dir)},
{"reason": "compiler-artifact", "package_id": "fixture", "executable": executable},
]

def run(cwd, *args):
self.assertEqual(cwd, package)
if args[0] == "cargo":
self.assertEqual(args, ("cargo", "build", "--offline", "--quiet", "--message-format=json"))
return "\n".join(json.dumps(event) for event in events)
self.assertEqual(args, (executable,))
return "fixture identity"

self.assertEqual(self.build_fixture(run, package), ("fixture identity", output))

def test_packed_branch_without_reflog_refreshes_and_then_stays_cached(self):
for linked_worktree in (False, True):
with self.subTest(linked_worktree=linked_worktree), tempfile.TemporaryDirectory() as tmp:
Expand Down Expand Up @@ -43,7 +106,7 @@ def run(cwd, *args):
'[package]\nname = "identity-fixture"\nversion = "0.0.0"\n'
'edition = "2021"\n', encoding="utf-8",
)
shutil.copyfile(BUILD_SCRIPT, package / "build.rs")
self.install_observed_build_script(package)
(package / "src/main.rs").write_text(
'fn main() { println!("{}", env!("WEBCODEX_BUILD_GIT_COMMIT")); }\n',
encoding="utf-8",
Expand All @@ -59,22 +122,29 @@ def run(cwd, *args):
head_log = Path(run(repo, "git", "rev-parse", "--git-path", "logs/HEAD"))
self.assertFalse((repo / head_log).exists())

outputs = []

def build():
return run(package, "cargo", "run", "--offline", "--quiet")
value, output = self.build_fixture(run, package)
if outputs:
self.assertEqual(outputs, [output])
else:
outputs.append(output)
return value

before = build()
outputs = list((root / "target/debug/build").glob("identity-fixture-*/output"))
self.assertEqual(len(outputs), 1)
stamp = outputs[0].stat().st_mtime_ns
stamp = self.marker_state(outputs[0])
self.assertEqual(build(), before)
self.assertEqual(outputs[0].stat().st_mtime_ns, stamp)
self.assertEqual(self.marker_state(outputs[0]), stamp)
run(repo, "git", "commit", "--allow-empty", "-m", "new identity")
expected = run(repo, "git", "rev-parse", "--short=12", "HEAD")
self.assertNotEqual(before, expected)
self.assertEqual(build(), expected)
stamp = outputs[0].stat().st_mtime_ns
self.assertGreater(self.marker_state(outputs[0])[0], stamp[0])
stamp = self.marker_state(outputs[0])
self.assertEqual(build(), expected)
self.assertEqual(outputs[0].stat().st_mtime_ns, stamp)
self.assertEqual(self.marker_state(outputs[0]), stamp)

def test_tracked_dirty_state_refreshes_at_same_head(self):
for linked_worktree in (False, True):
Expand Down Expand Up @@ -107,7 +177,7 @@ def run(cwd, *args):
'[package]\nname = "identity-fixture"\nversion = "0.0.0"\n'
'edition = "2021"\n', encoding="utf-8",
)
shutil.copyfile(BUILD_SCRIPT, package / "build.rs")
self.install_observed_build_script(package)
(package / "src/main.rs").write_text(
'fn main() { println!("{}", env!("WEBCODEX_BUILD_GIT_DIRTY")); }\n',
encoding="utf-8",
Expand All @@ -120,20 +190,28 @@ def run(cwd, *args):
repo = checkout
package = repo / "crates/fixture"

outputs = []

def build():
return run(package, "cargo", "run", "--offline", "--quiet")
value, output = self.build_fixture(run, package)
if outputs:
self.assertEqual(outputs, [output])
else:
outputs.append(output)
return value

head = run(repo, "git", "rev-parse", "HEAD")
self.assertEqual(build(), "false")
clean_count = self.marker_state(outputs[0])[0]
(repo / "tracked.txt").write_text("dirty\n", encoding="utf-8")
self.assertEqual(run(repo, "git", "rev-parse", "HEAD"), head)
self.assertEqual(build(), "true")
outputs = list((root / "target/debug/build").glob("identity-fixture-*/output"))
self.assertGreater(self.marker_state(outputs[0])[0], clean_count)
self.assertEqual(len(outputs), 1)
dirty_stamp = outputs[0].stat().st_mtime_ns
dirty_stamp = self.marker_state(outputs[0])
(repo / "other.txt").write_text("also dirty\n", encoding="utf-8")
self.assertEqual(build(), "true")
self.assertEqual(outputs[0].stat().st_mtime_ns, dirty_stamp)
self.assertEqual(self.marker_state(outputs[0]), dirty_stamp)

run(repo, "git", "restore", "tracked.txt")
self.assertEqual(run(repo, "git", "rev-parse", "HEAD"), head)
Expand All @@ -144,20 +222,19 @@ def build():

(repo / "tracked.txt").unlink()
self.assertEqual(build(), "true")
deleted_stamp = outputs[0].stat().st_mtime_ns
deleted_stamp = self.marker_state(outputs[0])
self.assertEqual(build(), "true")
self.assertEqual(outputs[0].stat().st_mtime_ns, deleted_stamp)
self.assertEqual(self.marker_state(outputs[0]), deleted_stamp)
run(repo, "git", "restore", "tracked.txt")
self.assertEqual(build(), "false")

env["WEBCODEX_GIT_DIRTY"] = "false"
self.assertEqual(build(), "false")
outputs = list((root / "target/debug/build").glob("identity-fixture-*/output"))
self.assertEqual(len(outputs), 1)
stamp = outputs[0].stat().st_mtime_ns
stamp = self.marker_state(outputs[0])
(repo / "tracked.txt").write_text("dirty with pinned identity\n", encoding="utf-8")
self.assertEqual(build(), "false")
self.assertEqual(outputs[0].stat().st_mtime_ns, stamp)
self.assertEqual(self.marker_state(outputs[0]), stamp)


def test_stat_only_change_is_clean_without_refreshing_index(self):
Expand Down
Loading
Loading