Skip to content

feat(broker-v2): accept loop + ServiceDefinitionLoader integration (slice 1 of #532) - #533

Merged
zackees merged 1 commit into
mainfrom
feat/532-v2-broker-accept-loop-and-loader
Jun 20, 2026
Merged

feat(broker-v2): accept loop + ServiceDefinitionLoader integration (slice 1 of #532)#533
zackees merged 1 commit into
mainfrom
feat/532-v2-broker-accept-loop-and-loader

Conversation

@zackees

@zackees zackees commented Jun 20, 2026

Copy link
Copy Markdown
Owner

First slice of #532 — replaces the slice-3c scaffold in running-process-broker-v2 with a real broker: --program <name> CLI arg, persistent accept loop with bounded concurrency, ServiceDefinitionLoader integration that rejects unknown services + below-min-version + outside-allow-list Hellos with typed Refused responses. Pure decision function build_hello_reply split out for unit-testability. 10 inline tests, all passing.

Out of scope: adopt forwarding, SIGTERM handling, single-instance lock, refuse-privileged-run guard. Tracked as follow-up slices in #532.

…lice 1 of #532)

First slice of running-process#532 — replaces the slice-3c scaffold
in running-process-broker-v2 with a real broker that:

1. **\`--program <name>\` CLI arg** names the v2 pipe namespace
   (\`rpb-v2-<program>-<sid_hash>-0\`). Defaults to
   \`broker-v2-scaffold\` so existing integration tests keep working.

2. **Persistent accept loop** with bounded concurrency. Each
   accepted connection spawns a handler thread; backpressure via
   \`MAX_INFLIGHT_HANDLERS\` (256). \`--once\` flag preserves the
   one-shot behaviour for the slice-3c integration test.

3. **ServiceDefinitionLoader integration**. On each Hello:
   - Look up \`hello.service_name\` via
     \`ServiceDefinitionLoader::default_root().load(...)\`
   - Reject unknown services with \`ErrorServiceUnknown\`
     (mirrors v1's \`hello_router\`'s refusal text exactly)
   - Reject below-\`min_version\` with \`ErrorVersionBlocked\`
   - Reject outside-\`version_allow_list\` with \`ErrorVersionBlocked\`
   - Reply \`Negotiated { backend_pipe: \"\" }\` for accepted Hellos
     (adopt forwarding is a follow-up slice)

4. **Pure decision function** \`build_hello_reply(hello, loader) ->
   HelloReply\` is split out from the IO-bound \`handle_hello\` so
   the policy logic is unit-testable without standing up a listener.

## Tests

10 inline tests:

CLI parsing (5):
- defaults (\`--program\` defaults to scaffold)
- \`--program <name>\` parse
- \`--once\` parse
- \`--program\` missing value errors
- unknown arg errors

Policy decisions (5):
- unknown service → ErrorServiceUnknown
- registered service → Negotiated (connection_id threaded)
- below min_version → ErrorVersionBlocked
- outside version_allow_list → ErrorVersionBlocked
- in version_allow_list → Negotiated

## Bundled chore

Added \`src/bin/README.md\` per the readme_guard hook — documents the
5 binaries shipped from this crate and the v1↔v2 broker coexistence
contract.

## Test plan

- [x] \`soldr cargo test --features client --bin running-process-broker-v2\` — 10 passed
- [x] \`soldr cargo clippy --all-targets --features client -- -D warnings\` clean

## What's NOT in this slice

- Adopt forwarding (backend_pipe resolution from servicedef binary_path +
  forwarding traffic to a launched / registered daemon) — separate slice
- SIGTERM / Ctrl+C graceful shutdown — separate slice
- Single-instance lock — separate slice
- Refuse-privileged-run guard (port from v1) — separate slice

These are all called out in the parent issue (#532) for follow-up work.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zackees
zackees merged commit 260aa69 into main Jun 20, 2026
7 of 8 checks passed
@zackees
zackees deleted the feat/532-v2-broker-accept-loop-and-loader branch June 20, 2026 21:08
zackees added a commit that referenced this pull request Jun 20, 2026
…onDenied as bound (#532 follow-up) (#536)

PR #533 (slice 1 of #532) added ServiceDefinitionLoader integration —
the broker's Hello handler now rejects unknown service names with
ErrorServiceUnknown. The existing scaffold integration test
(`broker_v2_scaffold_accepts_connection.rs`) used the hardcoded
`broker-v2-scaffold` service name without registering a matching
servicedef, so it broke silently on the first Hello.

This PR:

1. **Updates the integration test** to install a per-test stub
   servicedef under a tempfile-managed RUNNING_PROCESS_SERVICE_DEF_DIR
   so the loader resolves the test's service name.

2. **Uses a unique per-run program name** (`scaffold-<12hex>`) so
   parallel / repeated test runs don't collide on the global per-user
   pipe namespace (Windows reports ERROR_ACCESS_DENIED when an old
   broker on the same pipe hasn't released yet).

3. **Sharpens `is_already_bound_error`** to also catch
   `PermissionDenied`: on Windows, double-bind manifests as
   ERROR_ACCESS_DENIED (raw os error 5) because the existing pipe
   instance's ACL blocks the second bind, not as AddrInUse. The slice
   1 + 2 single-instance diagnostic was Unix-correct but missed
   Windows double-bind classification entirely.

## Test plan

- [x] `soldr cargo build --features client --tests --test
  broker_v2_scaffold_accepts_connection` clean.
- Verified manually that the binary's Access-Denied path was indeed a
  prior-run pipe-namespace collision (not a true permission failure).

## Bundled chore

Added `tests/README.md` per the readme_guard hook — documents the test
layout + the "install stub servicedef + use unique --program" pattern
for future broker integration tests.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zackees added a commit that referenced this pull request Jun 22, 2026
…aemon_version regression

Two independent CI regressions surfaced by the broker-v2 cascade
unblock landing in PR #573:

1. **testbin-createfilew-probe fails to compile on Linux + macOS.**
   Slice 7c's #[cfg(target_os = "windows")] gate is at the file
   level — on non-Windows the entire crate compiles to nothing
   and rustc errors out with "main function not found". Cargo
   requires every `[[bin]]` target to have a main fn on every
   build target, regardless of which target ever runs the binary.

   Fix: add a stub main fn for non-Windows that prints a "Windows-
   only fixture" message and exits with code 2. The Windows path
   is unchanged.

2. **`broker_v2_scaffold_accepts_connection::...` now panics at the
   `daemon_version should be populated` assertion (line 196).**
   PR #573 fixed the path-capture parser; the test now successfully
   completes the Hello/HelloReply round-trip and reaches a later
   assertion that the daemon's reported version is non-empty.

   Root cause: PR #533 changed `daemon_version` from the broker
   binary's own CARGO_PKG_VERSION to `definition.min_version.clone()`.
   That's semantically wrong — `daemon_version` is the running
   broker's actual version (matches the proto comment); min_version
   is a per-service floor expressed by the servicedef. For any
   servicedef that doesn't explicitly opt in to a floor (e.g. the
   test's stub), min_version is empty and the contract breaks.

   Fix: restore `daemon_version: env!("CARGO_PKG_VERSION").into()`
   (the pre-#533 behavior the comment block on top of the diff
   already described).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zackees added a commit that referenced this pull request Jun 22, 2026
… regression (#574)

* fix(observer): slice 7c — un-ignore Windows path-specific test + per-detour install diagnostics (#551)

The slice 7c test was hanging because of a self-inflicted
assertion bug, not a retour or detour issue. The interposer
formats paths via `{:?}` (debug-escaped, doubled backslashes), so
`contains(probe_path)` against the raw path never matched and the
10-second deadline ran to completion. The retour install actually
completed on all 5 detours, and the CreateFileW detour fired
correctly on testbin-createfilew-probe's call.

Fixes:

- `crates/.../interposer-windows/src/lib.rs`: install_detours now
  emits `RPO_HOOK install begin=<name>` / `install end=<name>`
  sentinels around each install_one call. Lets the
  `interposer_diagnostic_windows.rs` test (also added) report
  which retour install hung or errored if a regression ever
  reintroduces the hang. Verified clean against cmd.exe — all
  5 detours install in <100 ms each.

- `crates/.../observer/tests/interposer_integration_windows.rs`:
  un-ignore `interposer_dll_fires_rpo_hook_after_inject`. Replace
  `contains(probe_path.display())` with `contains("RPO_HOOK
  file-open") && contains("probe.txt")` for both the deadline-
  poll exit condition and the final assertion. The basename
  alone is unambiguous since the fixture only ever opens that
  file.

- `crates/.../observer/tests/interposer_diagnostic_windows.rs`:
  new diagnostic test that injects into cmd.exe and reports
  per-detour begin/end pair coverage to eprintln. Always passes
  (observational test) — its eprintln output lands in cargo
  nextest logs for CI / debugging visibility.

Verified locally:
  test interposer_dll_fires_rpo_hook_after_inject ... ok (7.10s)
  test diagnose_install_progress ... ok (13.22s)

Closes the slice 7c gap in #551's slice 7 acceptance criterion:
Windows test now asserts the detour fires on a real (non-
diagnostic) file API call, matching the assertion strength of
Linux (7d) and macOS (7e).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(broker-v2): strip trailing `(program=…, mode=…)` from captured socket path in test (#488 follow-up)

The slice 3c integration test reads broker stdout looking for the
"bound at" line:

    running-process-broker-v2 bound at <path> (program=<…>, mode=<…>)

It then `strip_prefix("running-process-broker-v2 bound at ")` and
`trim_end()` — which leaves the entire trailing
` (program=<…>, mode=<…>)` glued onto what gets captured as
`socket_path`. The "path" passed to `Stream::connect` is therefore
~25 bytes longer than the actual filesystem socket path. On Linux,
where `sun_path` is exactly 108 bytes, that's enough to push every
non-trivial XDG_RUNTIME_DIR / `/tmp/running-process-<uid>` path
past the limit — and `Stream::connect` then panics with the now-
infamous "exceeds capacity of sun_path of sockaddr_un" error.

This was the bug that's been cascade-failing every CI unit-test
run since slice 3c landed. It's purely a test-parser issue —
production broker bind/connect were always correct because they
share the same string and the broker's own `bound at` print
happens *after* successful `create_sync`.

Fix: `rsplit_once(" (")` on the trimmed remainder. Everything
before ` (` is the path; the trailing parenthesized metadata is
discarded. On Windows + macOS (where the path is short enough
that the test was passing despite the bug) the fix is a no-op
because the path either starts with `\.\pipe\` and contains no
` (` substring, or is the hash-shortened macOS leaf — both
fall back to the full string via `unwrap_or(rest)`.

Verified locally by inspection of test logs:
- Pre-fix: panic at line 146 with "exceeds capacity" + a
  ~113-byte captured "path".
- Post-fix: the captured path is the 88-byte filesystem socket
  the broker actually bound to.

Closes the "broker-v2 cascade failure" item that's been
documented as out-of-scope across the #539 and #551 loops.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(observer+broker-v2): unblock CI — testbin stub on non-Windows + daemon_version regression

Two independent CI regressions surfaced by the broker-v2 cascade
unblock landing in PR #573:

1. **testbin-createfilew-probe fails to compile on Linux + macOS.**
   Slice 7c's #[cfg(target_os = "windows")] gate is at the file
   level — on non-Windows the entire crate compiles to nothing
   and rustc errors out with "main function not found". Cargo
   requires every `[[bin]]` target to have a main fn on every
   build target, regardless of which target ever runs the binary.

   Fix: add a stub main fn for non-Windows that prints a "Windows-
   only fixture" message and exits with code 2. The Windows path
   is unchanged.

2. **`broker_v2_scaffold_accepts_connection::...` now panics at the
   `daemon_version should be populated` assertion (line 196).**
   PR #573 fixed the path-capture parser; the test now successfully
   completes the Hello/HelloReply round-trip and reaches a later
   assertion that the daemon's reported version is non-empty.

   Root cause: PR #533 changed `daemon_version` from the broker
   binary's own CARGO_PKG_VERSION to `definition.min_version.clone()`.
   That's semantically wrong — `daemon_version` is the running
   broker's actual version (matches the proto comment); min_version
   is a per-service floor expressed by the servicedef. For any
   servicedef that doesn't explicitly opt in to a floor (e.g. the
   test's stub), min_version is empty and the contract breaks.

   Fix: restore `daemon_version: env!("CARGO_PKG_VERSION").into()`
   (the pre-#533 behavior the comment block on top of the diff
   already described).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zackees added a commit that referenced this pull request Jun 22, 2026
…cher widening (#575)

* fix(observer): slice 7c — un-ignore Windows path-specific test + per-detour install diagnostics (#551)

The slice 7c test was hanging because of a self-inflicted
assertion bug, not a retour or detour issue. The interposer
formats paths via `{:?}` (debug-escaped, doubled backslashes), so
`contains(probe_path)` against the raw path never matched and the
10-second deadline ran to completion. The retour install actually
completed on all 5 detours, and the CreateFileW detour fired
correctly on testbin-createfilew-probe's call.

Fixes:

- `crates/.../interposer-windows/src/lib.rs`: install_detours now
  emits `RPO_HOOK install begin=<name>` / `install end=<name>`
  sentinels around each install_one call. Lets the
  `interposer_diagnostic_windows.rs` test (also added) report
  which retour install hung or errored if a regression ever
  reintroduces the hang. Verified clean against cmd.exe — all
  5 detours install in <100 ms each.

- `crates/.../observer/tests/interposer_integration_windows.rs`:
  un-ignore `interposer_dll_fires_rpo_hook_after_inject`. Replace
  `contains(probe_path.display())` with `contains("RPO_HOOK
  file-open") && contains("probe.txt")` for both the deadline-
  poll exit condition and the final assertion. The basename
  alone is unambiguous since the fixture only ever opens that
  file.

- `crates/.../observer/tests/interposer_diagnostic_windows.rs`:
  new diagnostic test that injects into cmd.exe and reports
  per-detour begin/end pair coverage to eprintln. Always passes
  (observational test) — its eprintln output lands in cargo
  nextest logs for CI / debugging visibility.

Verified locally:
  test interposer_dll_fires_rpo_hook_after_inject ... ok (7.10s)
  test diagnose_install_progress ... ok (13.22s)

Closes the slice 7c gap in #551's slice 7 acceptance criterion:
Windows test now asserts the detour fires on a real (non-
diagnostic) file API call, matching the assertion strength of
Linux (7d) and macOS (7e).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(broker-v2): strip trailing `(program=…, mode=…)` from captured socket path in test (#488 follow-up)

The slice 3c integration test reads broker stdout looking for the
"bound at" line:

    running-process-broker-v2 bound at <path> (program=<…>, mode=<…>)

It then `strip_prefix("running-process-broker-v2 bound at ")` and
`trim_end()` — which leaves the entire trailing
` (program=<…>, mode=<…>)` glued onto what gets captured as
`socket_path`. The "path" passed to `Stream::connect` is therefore
~25 bytes longer than the actual filesystem socket path. On Linux,
where `sun_path` is exactly 108 bytes, that's enough to push every
non-trivial XDG_RUNTIME_DIR / `/tmp/running-process-<uid>` path
past the limit — and `Stream::connect` then panics with the now-
infamous "exceeds capacity of sun_path of sockaddr_un" error.

This was the bug that's been cascade-failing every CI unit-test
run since slice 3c landed. It's purely a test-parser issue —
production broker bind/connect were always correct because they
share the same string and the broker's own `bound at` print
happens *after* successful `create_sync`.

Fix: `rsplit_once(" (")` on the trimmed remainder. Everything
before ` (` is the path; the trailing parenthesized metadata is
discarded. On Windows + macOS (where the path is short enough
that the test was passing despite the bug) the fix is a no-op
because the path either starts with `\.\pipe\` and contains no
` (` substring, or is the hash-shortened macOS leaf — both
fall back to the full string via `unwrap_or(rest)`.

Verified locally by inspection of test logs:
- Pre-fix: panic at line 146 with "exceeds capacity" + a
  ~113-byte captured "path".
- Post-fix: the captured path is the 88-byte filesystem socket
  the broker actually bound to.

Closes the "broker-v2 cascade failure" item that's been
documented as out-of-scope across the #539 and #551 loops.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(observer+broker-v2): unblock CI — testbin stub on non-Windows + daemon_version regression

Two independent CI regressions surfaced by the broker-v2 cascade
unblock landing in PR #573:

1. **testbin-createfilew-probe fails to compile on Linux + macOS.**
   Slice 7c's #[cfg(target_os = "windows")] gate is at the file
   level — on non-Windows the entire crate compiles to nothing
   and rustc errors out with "main function not found". Cargo
   requires every `[[bin]]` target to have a main fn on every
   build target, regardless of which target ever runs the binary.

   Fix: add a stub main fn for non-Windows that prints a "Windows-
   only fixture" message and exits with code 2. The Windows path
   is unchanged.

2. **`broker_v2_scaffold_accepts_connection::...` now panics at the
   `daemon_version should be populated` assertion (line 196).**
   PR #573 fixed the path-capture parser; the test now successfully
   completes the Hello/HelloReply round-trip and reaches a later
   assertion that the daemon's reported version is non-empty.

   Root cause: PR #533 changed `daemon_version` from the broker
   binary's own CARGO_PKG_VERSION to `definition.min_version.clone()`.
   That's semantically wrong — `daemon_version` is the running
   broker's actual version (matches the proto comment); min_version
   is a per-service floor expressed by the servicedef. For any
   servicedef that doesn't explicitly opt in to a floor (e.g. the
   test's stub), min_version is empty and the contract breaks.

   Fix: restore `daemon_version: env!("CARGO_PKG_VERSION").into()`
   (the pre-#533 behavior the comment block on top of the diff
   already described).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(broker-v2): align stale `is_already_bound_error_*permission_denied` test with PR #536's intentional impl change

Test cascade-failed every CI unit-test run since PR #536. PR #534
(slice 2 of #532) added \`is_already_bound_error_does_not_misclassify_permission_denied\`
when the matcher only covered \`AddrInUse | WouldBlock\`. PR #536
then *deliberately* widened the matcher to also include
\`PermissionDenied\` — Windows double-bind surfaces as
\`ERROR_ACCESS_DENIED\` (raw os error 5) because the existing pipe
instance's ACL blocks the second bind, not as \`AddrInUse\`. PR #536's
commit body documents the change explicitly but didn't update this
test, which then asserted the now-inverted behavior.

Fix: rename to \`is_already_bound_error_classifies_permission_denied\`
and invert the assertion to match the now-current contract. Doc
comment cross-references PR #536's rationale so the next reader
understands why PermissionDenied counts as "already bound" here.

Verified all 4 \`is_already_bound_error_*\` tests pass:
  - classifies_addr_in_use
  - classifies_would_block
  - classifies_permission_denied  (renamed + inverted in this PR)
  - does_not_misclassify_not_found

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zackees added a commit that referenced this pull request Jun 22, 2026
* fix(observer): slice 7c — un-ignore Windows path-specific test + per-detour install diagnostics (#551)

The slice 7c test was hanging because of a self-inflicted
assertion bug, not a retour or detour issue. The interposer
formats paths via `{:?}` (debug-escaped, doubled backslashes), so
`contains(probe_path)` against the raw path never matched and the
10-second deadline ran to completion. The retour install actually
completed on all 5 detours, and the CreateFileW detour fired
correctly on testbin-createfilew-probe's call.

Fixes:

- `crates/.../interposer-windows/src/lib.rs`: install_detours now
  emits `RPO_HOOK install begin=<name>` / `install end=<name>`
  sentinels around each install_one call. Lets the
  `interposer_diagnostic_windows.rs` test (also added) report
  which retour install hung or errored if a regression ever
  reintroduces the hang. Verified clean against cmd.exe — all
  5 detours install in <100 ms each.

- `crates/.../observer/tests/interposer_integration_windows.rs`:
  un-ignore `interposer_dll_fires_rpo_hook_after_inject`. Replace
  `contains(probe_path.display())` with `contains("RPO_HOOK
  file-open") && contains("probe.txt")` for both the deadline-
  poll exit condition and the final assertion. The basename
  alone is unambiguous since the fixture only ever opens that
  file.

- `crates/.../observer/tests/interposer_diagnostic_windows.rs`:
  new diagnostic test that injects into cmd.exe and reports
  per-detour begin/end pair coverage to eprintln. Always passes
  (observational test) — its eprintln output lands in cargo
  nextest logs for CI / debugging visibility.

Verified locally:
  test interposer_dll_fires_rpo_hook_after_inject ... ok (7.10s)
  test diagnose_install_progress ... ok (13.22s)

Closes the slice 7c gap in #551's slice 7 acceptance criterion:
Windows test now asserts the detour fires on a real (non-
diagnostic) file API call, matching the assertion strength of
Linux (7d) and macOS (7e).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(broker-v2): strip trailing `(program=…, mode=…)` from captured socket path in test (#488 follow-up)

The slice 3c integration test reads broker stdout looking for the
"bound at" line:

    running-process-broker-v2 bound at <path> (program=<…>, mode=<…>)

It then `strip_prefix("running-process-broker-v2 bound at ")` and
`trim_end()` — which leaves the entire trailing
` (program=<…>, mode=<…>)` glued onto what gets captured as
`socket_path`. The "path" passed to `Stream::connect` is therefore
~25 bytes longer than the actual filesystem socket path. On Linux,
where `sun_path` is exactly 108 bytes, that's enough to push every
non-trivial XDG_RUNTIME_DIR / `/tmp/running-process-<uid>` path
past the limit — and `Stream::connect` then panics with the now-
infamous "exceeds capacity of sun_path of sockaddr_un" error.

This was the bug that's been cascade-failing every CI unit-test
run since slice 3c landed. It's purely a test-parser issue —
production broker bind/connect were always correct because they
share the same string and the broker's own `bound at` print
happens *after* successful `create_sync`.

Fix: `rsplit_once(" (")` on the trimmed remainder. Everything
before ` (` is the path; the trailing parenthesized metadata is
discarded. On Windows + macOS (where the path is short enough
that the test was passing despite the bug) the fix is a no-op
because the path either starts with `\.\pipe\` and contains no
` (` substring, or is the hash-shortened macOS leaf — both
fall back to the full string via `unwrap_or(rest)`.

Verified locally by inspection of test logs:
- Pre-fix: panic at line 146 with "exceeds capacity" + a
  ~113-byte captured "path".
- Post-fix: the captured path is the 88-byte filesystem socket
  the broker actually bound to.

Closes the "broker-v2 cascade failure" item that's been
documented as out-of-scope across the #539 and #551 loops.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(observer+broker-v2): unblock CI — testbin stub on non-Windows + daemon_version regression

Two independent CI regressions surfaced by the broker-v2 cascade
unblock landing in PR #573:

1. **testbin-createfilew-probe fails to compile on Linux + macOS.**
   Slice 7c's #[cfg(target_os = "windows")] gate is at the file
   level — on non-Windows the entire crate compiles to nothing
   and rustc errors out with "main function not found". Cargo
   requires every `[[bin]]` target to have a main fn on every
   build target, regardless of which target ever runs the binary.

   Fix: add a stub main fn for non-Windows that prints a "Windows-
   only fixture" message and exits with code 2. The Windows path
   is unchanged.

2. **`broker_v2_scaffold_accepts_connection::...` now panics at the
   `daemon_version should be populated` assertion (line 196).**
   PR #573 fixed the path-capture parser; the test now successfully
   completes the Hello/HelloReply round-trip and reaches a later
   assertion that the daemon's reported version is non-empty.

   Root cause: PR #533 changed `daemon_version` from the broker
   binary's own CARGO_PKG_VERSION to `definition.min_version.clone()`.
   That's semantically wrong — `daemon_version` is the running
   broker's actual version (matches the proto comment); min_version
   is a per-service floor expressed by the servicedef. For any
   servicedef that doesn't explicitly opt in to a floor (e.g. the
   test's stub), min_version is empty and the contract breaks.

   Fix: restore `daemon_version: env!("CARGO_PKG_VERSION").into()`
   (the pre-#533 behavior the comment block on top of the diff
   already described).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(broker-v2): align stale `is_already_bound_error_*permission_denied` test with PR #536's intentional impl change

Test cascade-failed every CI unit-test run since PR #536. PR #534
(slice 2 of #532) added \`is_already_bound_error_does_not_misclassify_permission_denied\`
when the matcher only covered \`AddrInUse | WouldBlock\`. PR #536
then *deliberately* widened the matcher to also include
\`PermissionDenied\` — Windows double-bind surfaces as
\`ERROR_ACCESS_DENIED\` (raw os error 5) because the existing pipe
instance's ACL blocks the second bind, not as \`AddrInUse\`. PR #536's
commit body documents the change explicitly but didn't update this
test, which then asserted the now-inverted behavior.

Fix: rename to \`is_already_bound_error_classifies_permission_denied\`
and invert the assertion to match the now-current contract. Doc
comment cross-references PR #536's rationale so the next reader
understands why PermissionDenied counts as "already bound" here.

Verified all 4 \`is_already_bound_error_*\` tests pass:
  - classifies_addr_in_use
  - classifies_would_block
  - classifies_permission_denied  (renamed + inverted in this PR)
  - does_not_misclassify_not_found

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(interposer-windows): gate retour deps + lib body on x86_64 (ARM64 stub)

retour 0.4.0-alpha.4 uses iced-x86 for prologue disassembly,
which doesn't support ARM64. The Windows ARM Lint CI runner
(aarch64-pc-windows-msvc) was therefore failing with
\"could not find \`meta\` in \`arch\`\" — retour's internal
arch::meta module is only defined for x86 / x86_64.

This wasn't surfaced earlier because the broker-v2 cascade
failure was tripping the ARM CI before retour got a chance to
compile. With PR #573-#575 clearing that cascade, the ARM CI
proceeded far enough to hit the real arch incompatibility.

Fix: gate both the retour + windows-sys deps in Cargo.toml on
`cfg(all(target_os = "windows", target_arch = "x86_64"))`, and
gate the lib.rs body the same way. On Windows ARM64 the crate
now falls through to an empty stub so the workspace builds
end-to-end; the file-hook tier reports unavailable for that
arch (which is the honest answer until retour or an alternative
supports ARM64).

Verified:
  $ soldr cargo check -p running-process-observer-interposer-windows
  Finished `dev` profile [unoptimized + debuginfo] target(s)
  $ soldr cargo check --target aarch64-pc-windows-msvc \
        -p running-process-observer-interposer-windows
  Finished `dev` profile [unoptimized + debuginfo] target(s)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zackees added a commit that referenced this pull request Jul 24, 2026
…SIGILL since #533

Instrumented daemons this suite kills at teardown (broker-v2 accept
loop, #533 — 'production exit is via SIGTERM') never run the atexit
__llvm_profile_write_file flush; a file caught mid-write is truncated
and rustup's llvm-profdata crashes with SIGILL merging it. Coverage
has been red on every run since the #533 merge (2026-06-21).

Split cargo llvm-cov into nextest --no-report / report and validate
each .profraw with llvm-profdata show in between, deleting the ones
it cannot read (loses only the killed processes' counters).
Crash-on-corrupt-input is a known llvm-profdata bug class upstream
(llvm/llvm-project#92358, #63179); --failure-mode does not help when
the parser itself crashes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
zackees added a commit that referenced this pull request Jul 24, 2026
… Unix; CI consolidation (4.6.1) (#655)

* fix(env): restore USERNAME in Windows baseline env; real login env on Unix

CreateEnvironmentBlock silently omits per-user dynamic variables
(USERNAME, USERDOMAIN) when the token is opened with TOKEN_QUERY only;
open it with TOKEN_QUERY | TOKEN_DUPLICATE | TOKEN_IMPERSONATE as the
API documents. Root-caused live: soldr's daemon derived its pipe name
from USERNAME and bound soldr-daemon-soldr-<hash> while clients dialed
soldr-daemon-<user>-<hash>, forcing permanent uncached fallback.

EnvironmentPolicy::UserBaseline on Unix now reconstructs a clean login
environment from getpwuid_r (USER/LOGNAME/HOME/SHELL + default login
PATH, carrying over LANG/LC_*/TZ/TMPDIR) instead of inheriting the
parent environment wholesale.

Release 4.6.1.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore(deps): update anyhow 1.0.102 -> 1.0.104 (RUSTSEC-2026-0204)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore(deps): update crossbeam-epoch to >=0.9.20 (RUSTSEC-2026-0204)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci: retrigger after cancelling superseded workflow runs

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci: one build per platform — collapse per-task workflow sprawl (#513 follow-up)

Rewrite ci-preflight.yml as ONE job per platform: workspace cargo
build + dev wheel build happen exactly once, then lint, unit tests,
integration tests, servicedef proof, and rustdoc run sequentially
against that build. The warm-then-fan-out shape still re-ran uv sync
and the maturin dev wheel build in every sub-worker and paid 4x
runner spin-up + cache transfer per platform.

Remove the 20 legacy per-platform per-task wrappers, the 3 shared
_<task>.yml templates, preflight-macos.yml, and linux-x86-rustdoc.yml
(all superseded by ci-preflight.yml since #513; soak period served).

Keep the six *-build.yml wheel workflows for ci/publish.py's manual
collection path but make them workflow_dispatch-only. Their
reproducible-spot-check job moves to ci-linux.yml. Restrict the
ci-{linux,macos,windows} dispatchers' push trigger to main so PR
branches stop running the whole matrix twice (push + pull_request).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* ci(coverage): triage the llvm-profdata SIGILL — core-dump backtrace + profraw bisect

Coverage has been red since 2026-06-21 (200+ runs): all tests pass,
then rustup's llvm-profdata crashes with SIGILL merging the profraw
set. The LD_PRELOAD interposer landed the same day — instrumented
processes exiting through hooked file APIs are the prime suspect for
corrupt .profraw output.

On failure the job now: prints llvm-profdata identity, gdb-backtraces
any core dump, validates each profraw individually (uploading the bad
ones as artifacts), and retries the merge with only the valid files
to confirm or refute the poison-file theory.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(coverage): prune truncated .profraw before merge — llvm-profdata SIGILL since #533

Instrumented daemons this suite kills at teardown (broker-v2 accept
loop, #533 — 'production exit is via SIGTERM') never run the atexit
__llvm_profile_write_file flush; a file caught mid-write is truncated
and rustup's llvm-profdata crashes with SIGILL merging it. Coverage
has been red on every run since the #533 merge (2026-06-21).

Split cargo llvm-cov into nextest --no-report / report and validate
each .profraw with llvm-profdata show in between, deleting the ones
it cannot read (loses only the killed processes' counters).
Crash-on-corrupt-input is a known llvm-profdata bug class upstream
(llvm/llvm-project#92358, #63179); --failure-mode does not help when
the parser itself crashes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant