diff --git a/app-server-transport/src/websocket.rs b/app-server-transport/src/websocket.rs index dcde680..1abda01 100644 --- a/app-server-transport/src/websocket.rs +++ b/app-server-transport/src/websocket.rs @@ -375,25 +375,33 @@ mod tests { .expect("app server") } + async fn start_test_transport( + app: AppServer, + config: WebSocketTransportConfig, + ) -> ( + SocketAddr, + tokio::task::JoinHandle>, + ) { + let addr: SocketAddr = "127.0.0.1:0".parse().expect("ephemeral loopback address"); + let (bound_addr_tx, bound_addr_rx) = tokio::sync::oneshot::channel(); + let transport_handle = tokio::spawn(async move { + run_websocket_transport_with_bound_addr(addr, app, config, Some(bound_addr_tx)).await + }); + let bound_addr = tokio::time::timeout(Duration::from_secs(5), bound_addr_rx) + .await + .expect("timeout waiting for WebSocket transport bind") + .expect("WebSocket transport exited before reporting its bound address"); + (bound_addr, transport_handle) + } + // ------------------------------------------------------------------- // 1. Initialize over WebSocket // ------------------------------------------------------------------- #[tokio::test] async fn websocket_initialize() { let app = test_app("ws-init").await; - - // Bind to an ephemeral port. - let addr: SocketAddr = "127.0.0.1:0".parse().unwrap(); - let listener = TcpListener::bind(addr).await.expect("bind"); - let bound_addr = listener.local_addr().expect("local_addr"); - drop(listener); // Free the port for the transport to re-bind. - - let config = WebSocketTransportConfig::default(); - let transport_handle = - tokio::spawn(async move { run_websocket_transport(bound_addr, app, config).await }); - - // Wait briefly for the listener to bind. - tokio::time::sleep(Duration::from_millis(100)).await; + let (bound_addr, transport_handle) = + start_test_transport(app, WebSocketTransportConfig::default()).await; // Connect as a WebSocket client. let url = format!("ws://{bound_addr}"); @@ -442,17 +450,8 @@ mod tests { #[tokio::test] async fn websocket_disconnect() { let app = test_app("ws-disc").await; - - let addr: SocketAddr = "127.0.0.1:0".parse().unwrap(); - let listener = TcpListener::bind(addr).await.expect("bind"); - let bound_addr = listener.local_addr().expect("local_addr"); - drop(listener); - - let config = WebSocketTransportConfig::default(); - let transport_handle = - tokio::spawn(async move { run_websocket_transport(bound_addr, app, config).await }); - - tokio::time::sleep(Duration::from_millis(100)).await; + let (bound_addr, transport_handle) = + start_test_transport(app, WebSocketTransportConfig::default()).await; let url = format!("ws://{bound_addr}"); let (ws, _resp) = connect_async(&url).await.expect("WS connect"); @@ -472,21 +471,12 @@ mod tests { let app = test_app("ws-tcp-no-upgrade").await; - let addr: SocketAddr = "127.0.0.1:0".parse().unwrap(); - let listener = TcpListener::bind(addr).await.expect("bind"); - let bound_addr = listener.local_addr().expect("local_addr"); - drop(listener); - // Use a 1-second handshake timeout so the test is fast. let config = WebSocketTransportConfig { handshake_timeout: Duration::from_secs(1), ..Default::default() }; - - let transport_handle = - tokio::spawn(async move { run_websocket_transport(bound_addr, app, config).await }); - - tokio::time::sleep(Duration::from_millis(100)).await; + let (bound_addr, transport_handle) = start_test_transport(app, config).await; // Connect via raw TCP — no WS upgrade sent. let _tcp = TcpStream::connect(bound_addr).await.expect("TCP connect"); diff --git a/docs/plans/rust-maintenance-current-head-inventory.md b/docs/plans/rust-maintenance-current-head-inventory.md index 0326596..9882287 100644 --- a/docs/plans/rust-maintenance-current-head-inventory.md +++ b/docs/plans/rust-maintenance-current-head-inventory.md @@ -1,5 +1,12 @@ # Rust maintenance current-head inventory +> Historical handoff note (2026-08-26): this document remains the authoritative +> `daa1f04` inventory and deferred-family classification. Its four selected +> implementation families subsequently landed in `c100f5f`, `bfdb327`, +> `f4d0be5`, and `746aace`. Current counts, sustainable guardrails, final +> dispositions, and gate evidence are recorded in +> [`rust-maintenance-slice8-closeout.md`](rust-maintenance-slice8-closeout.md). + ## Conclusion This inventory audits `daa1f041ca5ebf2c2c6a5d7ff6dd1e27394e22a3` diff --git a/docs/plans/rust-maintenance-slice8-closeout.md b/docs/plans/rust-maintenance-slice8-closeout.md new file mode 100644 index 0000000..b5fedb7 --- /dev/null +++ b/docs/plans/rust-maintenance-slice8-closeout.md @@ -0,0 +1,220 @@ +# Rust maintenance Slice 8 closeout + +## Conclusion + +The bounded Rust idiomatic maintenance plan is closed on top of +`746aacec794470ae5325bfa71e83846fccb6027c` (`746aace`). Every family selected +by the `daa1f04` current-head inventory landed in its assigned owner boundary; +all unselected findings remain explicitly deferred rather than being reported +as fixed. Production library and binary `unwrap_used` remain at zero, standard +workspace gates remain the required bar, and `clippy::pedantic` remains an +advisory inventory rather than a deny gate. + +This closeout changes no Rust production code, public API, protocol DTO, +serialization, transcript, CLI/TUI output, fixture, snapshot, or golden. It +adds sustainable guardrails for the three deliberately narrow boundaries that +the parent plan asked to keep from spreading. + +## Audited state + +The work ran in the dedicated worktree +`/Users/iluo/github/beiwei30/orbcode-slice-8-closeout` on branch +`chore/slice-8-closeout`. Both `HEAD` and `origin/main` resolved to +`746aacec794470ae5325bfa71e83846fccb6027c`. A first clean report captured the +selected-child state before closeout changes. Verification then exposed and +fixed two test-only, load-sensitive races: WebSocket listener readiness and a +slow MCP stdio request's completion synchronization. The report was run twice +again against the final worktree so the final all-target counts include both +test changes. Production Rust remained identical to `746aace`. + +| Field | Evidence | +| --- | --- | +| rustc | `1.97.1 (8bab26f4f 2026-07-14)`, LLVM 22.1.6, host `aarch64-apple-darwin` | +| Cargo | `1.97.1 (c980f4866 2026-06-30)` | +| Clippy | `0.1.97 (8bab26f4f6 2026-07-14)` | +| Report script | version 2, Git blob `c539ad3e3f1dd1ba1bc1d003b333c15dc1a3ac67` | +| Clean selected-child report SHA-256 | `bb4a8f49c6bed5102eff6e057137b347230666173582095b8d2c636168ac275d` for both runs | +| Final closeout report SHA-256 | `da48915927bcbed49f19d122de9fb65db4f38171051abf0f80ff10d31376e3fa` for both runs | + +Reproduction: + +```sh +scripts/rust-maintenance-report.py > /tmp/orbcode-slice8-final-1.md +scripts/rust-maintenance-report.py > /tmp/orbcode-slice8-final-2.md +shasum -a 256 \ + /tmp/orbcode-slice8-final-1.md \ + /tmp/orbcode-slice8-final-2.md +diff -u \ + /tmp/orbcode-slice8-final-1.md \ + /tmp/orbcode-slice8-final-2.md +``` + +The two outputs were byte-identical. The report is not checked in as a global +warning golden: only the focused counts and dispositions below are durable. + +## Selected P3 work disposition + +| Plan | Selected owner family | Landed evidence | Closeout disposition | +| --- | --- | --- | --- | +| P3-01 | Current-head reinventory and bounded handoff | `c7e765b` | Complete; `rust-maintenance-current-head-inventory.md` remains the authoritative detailed classification. | +| P3-02 | Connection-owned app-server subscription pumps | `c100f5f` | Complete; finished handles are joined, panic/unexpected cancellation is observable, expected Drop abort remains quiet. | +| P3-03 | MCP trust-settings persistence source | `bfdb327` | Complete; `ConfigError` remains in the source chain while Display/Debug/protocol canaries remain redacted. | +| P3-04 | MCP WebSocket frame lengths | `f4d0be5` | Complete; outgoing conversion is checked, incoming over-limit input is rejected before allocation, and the 32-bit bound is proved. | +| P3-05 | Two private TUI overlay equivalent-arm matches | `746aace` | Complete; both warnings are removed and cursor/render behavior is covered across every overlay variant and frame captures. | + +No selected child absorbed an inventory family that P3-01 deferred. + +## Before and after counts + +The “before” column is the reproducible P3-01 snapshot at `daa1f04`; the +“after” column is the final closeout worktree. Counts are diagnostics +deduplicated by lint, crate, path, line, and column. They are evidence for the +selected work, not a global quality score. + +| Scope | `daa1f04` before | Final worktree | Change | +| --- | ---: | ---: | ---: | +| Classified production library, all warnings | 1,766 | 1,761 | -5 | +| Classified production library, selected warnings | 1,449 | 1,444 | -5 | +| Production library `unwrap_used` | 0 | 0 | 0 | +| Classified production other target, all warnings | 73 | 73 | 0 | +| Classified production other target, selected warnings | 46 | 46 | 0 | +| Production other target `unwrap_used` | 0 | 0 | 0 | +| Workspace all targets, all warnings | 3,786 | 3,785 | -1 | +| Workspace all targets, selected warnings | 3,166 | 3,160 | -6 | +| Workspace all targets `unwrap_used` | 1,367 | 1,364 | -3 | + +The focused production-library reductions are exact: + +| Lint family | `daa1f04` | Final worktree | Reason | +| --- | ---: | ---: | --- | +| `cast_possible_truncation` | 56 | 53 | The three selected MCP frame conversions now use proved widening or checked conversion. | +| `match_same_arms` | 35 | 33 | The two selected private TUI matches were merged. | +| Every other selected family | unchanged | unchanged | P3-02/P3-03 are behavioral risk fixes, and unselected lint families were not churned. | + +At clean `746aace`, the children had added three all-target warnings while +reducing the five selected production diagnostics above. The closeout's +test-only WebSocket readiness fix then removed three test `unwrap_used` +diagnostics. Replacing the MCP test's literal fixed wait with event +synchronization removed one additional non-selected test diagnostic, so the +final all-warning total is one below the P3-01 snapshot and all-target +`unwrap_used` is lower by three. Neither test movement is reported as +production risk reduction. + +## Sustainable guardrails + +`scripts/audit-rust-maintenance.py` is now part of `scripts/check.sh`. It masks +comments and literals, excludes dedicated and `cfg(test)` code, and checks +stable semantic anchors instead of line numbers. Its focused self-tests cover +test-only exclusion, comments/literals, brace ownership, multiple spawns in one +owner, and the permission/nested-option scans. + +The audit deliberately covers only: + +1. **String-typed permission declarations:** six reviewed occurrences. Two are + child-session persistence fields, one is the background-task compatibility + DTO, two are stream/control wire projections, and one is a same-function + parser staging variable converted to `PermissionMode` before runtime state. +2. **Raw nested options:** eleven reviewed occurrences. They are the goal PATCH + wire/parser/runtime/UI chain (nine occurrences), transcript + `PresentJsonValue` (one), and TUI `EffortOverrideSelection` (one). Each keeps + absent/null/value semantics within its named owner boundary. +3. **Production `tokio::spawn`:** 55 static call sites across 35 owning-function + anchors. The checked-in allow-list records a lifecycle disposition for each + anchor: 17 sites are complete/observed, 14 are bounded best-effort, and 24 + remain assigned to a named deferred owner family. + +A new occurrence, changed count, or move to another spawn owner fails the gate +until its lifecycle is classified. Intentional updates use: + +```sh +scripts/audit-rust-maintenance.py --list-spawns +``` + +The command prints `UNCLASSIFIED`; it does not automatically bless a new +spawn. There is no update mode for pedantic totals. + +## Deferred findings + +The following work is intentionally not part of this closed maintenance plan: + +- **Task lifecycle:** 24 static spawn sites remain in named client transport, + ACP, core turn/background/goal/workflow, and MCP stdio owner families. Their + present completion/cancellation behavior and remaining panic/`JoinError` gap + are recorded in the current-head inventory and spawn allow-list. Reopening + requires a focused owner plan and deterministic failure test. +- **Numeric diagnostics:** 171 production library/binary diagnostics remain + across external values, counters/cost, time, buffers, and TUI display + coordinates. Different domains require different checked, saturation, or + clamping policies; a global cast rewrite would be unsafe. +- **Mechanical diagnostics:** 135 plausible `match_same_arms`, + `manual_let_else`, `map_unwrap_or`, and `needless_pass_by_value` diagnostics + remain across production library/binary targets. They need private call-graph + review and are not valuable as a bulk target. +- **Error sources:** client transports, MCP TLS, core/tool adapters, and + parser/final projections remain separated by owner and redaction risk. Fixed + timeout/channel text has no underlying source to retain. None is claimed as + fixed by the trust-settings batch. +- **Broad pedantic categories:** `missing_errors_doc`, `must_use_candidate`, + `doc_markdown`, and `too_many_lines`, plus test/fixture/compatibility + findings, remain outside the maintenance objective. +- **Protocol/API and representation changes:** public `Debug`, + `non_exhaustive`, broad signature changes, transcript representation, and + runtime-model semantics require independent API/product ownership. + +These are deferred because the expected signal does not justify continued +cross-owner churn, not because their current warning count is zero. + +## Focused verification + +The closeout reran the proof commands for every selected implementation child: + +| Command | Result | +| --- | --- | +| `python3 scripts/audit-rust-maintenance.py --self-test` | Passed; 4 focused scanner tests. | +| `scripts/audit-rust-maintenance.py` | Passed; 6 permission, 11 nested-option, and 55 spawn occurrences matched their reviewed boundaries. | +| `cargo test -p orbcode-app-server message_processor` | Passed; 40 focused tests, including normal completion, panic, unexpected cancellation, Drop, and pending-state cleanup. | +| `cargo test -p orbcode-app-server processor_drop_aborts_active_subscriptions` | Passed. | +| `cargo test -p orbcode-mcp set_server_trust` | Passed; 3 source/redaction/persistence tests. | +| `cargo test -p orbcode-mcp websocket_frame` | Passed; 4 outgoing/incoming/limit/platform-boundary tests. | +| `cargo test -p orbcode-tui overlay_cursor_style` | Passed; every overlay variant is enumerated. | +| `cargo test -p orbcode-tui frame_capture` | Passed; 58 frame-capture tests. | +| `cargo test -p orbcode-app-server-transport --lib` | Passed; 24 tests after replacing fixed-sleep startup with the transport's bound-address readiness signal. | +| `cargo test -p orbcode-tools skills::loader_tests::bounded_mcp_skill_discovery_timeout_does_not_cancel_stdio_request -- --exact` | Passed in 10 consecutive runs after replacing the fixed completion wait with a fake-server event. | +| `cargo test -p orbcode-tools --lib` | Passed; 346 tests passed and 13 repository-designated tests remained ignored. | + +## Final gate + +Two pre-fix workspace runs reproduced `ConnectionRefused` in +`websocket_disconnect` and +`tcp_without_upgrade_times_out_and_server_continues` under parallel load. The +tests released a temporary port, spawned a server that still had to re-bind, +slept for a fixed 100 ms, and then raced the client against that bind. Both +tests and the full transport crate passed in isolation, confirming the +load-sensitive startup race. The test helper now starts the existing +`run_websocket_transport_with_bound_addr` path on `127.0.0.1:0` and awaits its +bounded oneshot readiness signal before connecting. No production transport +behavior changed and no failure is suppressed or retried by the gate. + +A later final workspace run exposed the same fixed-time assumption in +`bounded_mcp_skill_discovery_timeout_does_not_cancel_stdio_request`: under +load, its one-second sleep could finish before the first stdio client completed +initialization and entered the registry, so the follow-up probe could start a +second process. The fake MCP server now records receipt of `prompts/list`, and +the test waits with a bounded deadline for that event before probing connection +reuse. This keeps the cancellation/lifecycle assertion intact without assuming +a scheduler speed. The exact test passed 10 consecutive runs and the full +`orbcode-tools` library suite passed. No production MCP behavior changed. + +The complete post-both-fixes canonical run passed: + +| Command | Result | +| --- | --- | +| `scripts/check.sh` | Passed end to end: docs, fmt, Clippy, normal/no-default-feature checks, maintenance/public-surface/brand audits, and `cargo test --workspace`. Repository-designated manual, Node-dependent, release-boundary, and PTY tests remained ignored as declared by the canonical gate. | +| `cargo clippy --workspace --all-targets -- -D warnings` | Passed. | +| `cargo check --workspace` | Passed. | +| `cargo test --workspace` | Passed as the test stage of the complete post-fix `scripts/check.sh` run. | +| `scripts/check-docs.sh` | Passed. | +| `scripts/audit-rust-maintenance.py` | Passed. | +| `scripts/audit-public-surface.sh` | Passed. | +| `scripts/audit-brand.sh` | Passed. | +| `git diff --check` | Passed in both the code worktree and external plan repository. | diff --git a/scripts/audit-rust-maintenance.py b/scripts/audit-rust-maintenance.py new file mode 100755 index 0000000..c3668b7 --- /dev/null +++ b/scripts/audit-rust-maintenance.py @@ -0,0 +1,581 @@ +#!/usr/bin/env python3 +"""Guard the narrow Rust-maintenance boundaries closed by Slice 8. + +This is deliberately not a global Clippy snapshot. It checks only boundaries +whose current exceptions have been semantically reviewed: + +* string-typed ``permission_mode`` declarations; +* raw ``Option>`` spellings; and +* production ``tokio::spawn`` owner/count anchors. + +The spawn allow-list records the owning function rather than a source line, so +unrelated edits do not move the baseline. A new spawn, a removed spawn, or a +move to another owner requires an explicit disposition update. +""" + +from __future__ import annotations + +import argparse +import bisect +import collections +from dataclasses import dataclass +from pathlib import Path +import re +import sys +import unittest + + +PERMISSION_STRING_RE = re.compile( + r"\bpermission_mode\s*:\s*(?:Option\s*<\s*String\s*>|String)" +) +NESTED_OPTION_RE = re.compile(r"\bOption\s*<\s*Option\s*<") +SPAWN_RE = re.compile(r"\btokio\s*::\s*spawn\s*\(") +FUNCTION_RE = re.compile(r"\bfn\s+([A-Za-z_][A-Za-z0-9_]*)\s*(?:<[^>{};]*>)?\s*\(") +RAW_STRING_RE = re.compile(r"(?:br|rb|cr|rc|r)(?P#{0,255})\"") +CHARACTER_RE = re.compile(r"'(?:\\(?:.|u\{[0-9A-Fa-f_]+\})|[^'\\\n])'") +CFG_TEST_RE = re.compile(r"#\s*\[\s*cfg\s*\(\s*test\s*\)\s*]") +FILE_CFG_TEST_RE = re.compile(r"#\s*!\[\s*cfg\s*\(\s*test\s*\)\s*]") +TEST_ATTRIBUTE_RE = re.compile(r"#\s*\[\s*(?:(?:tokio|async_std)\s*::\s*)?test\b") + +SPAWN_ALLOW_LIST = "scripts/rust-maintenance-spawn-allow-list.txt" +SPAWN_DISPOSITION_RE = re.compile(r"(?:complete|deferred|best-effort):[a-z0-9-]+$") + + +def normalize_line(line: str) -> str: + return " ".join(line.strip().split()) + + +EXPECTED_PERMISSION_STRINGS = collections.Counter( + { + ( + "protocol/src/background_task_view.rs", + "pub permission_mode: Option,", + ): 1, + ( + "session-store/src/child_sessions.rs", + "pub permission_mode: Option,", + ): 2, + ( + "config/src/agents.rs", + "let mut permission_mode: Option = None;", + ): 1, + ("cli/src/stream_json.rs", "permission_mode: String,"): 1, + ("protocol/src/control.rs", "pub permission_mode: String,"): 1, + } +) + + +EXPECTED_NESTED_OPTIONS = collections.Counter( + { + ( + "app-server-protocol/src/contracts.rs", + "pub token_budget: Option>,", + ): 1, + ( + "app-server-protocol/src/contracts.rs", + "fn deserialize_present_option<'de, D, T>(deserializer: D) -> Result>, D::Error>", + ): 1, + ( + "core/src/session_manager/session_goal.rs", + "pub token_budget: Option>,", + ): 1, + ( + "core/src/session_manager/session_goal.rs", + "pub stop_reason: Option>,", + ): 1, + ( + "core/src/session_manager/session_goal.rs", + "fn validated_budget(value: Option>) -> Result, GoalError> {", + ): 1, + ( + "tui/src/commands/goal.rs", + "token_budget: Option>,", + ): 2, + ( + "tui/src/commands/goal.rs", + "fn budget_allows_resume(update: Option>, goal: &SessionGoal) -> bool {", + ): 1, + ( + "tui/src/commands/goal.rs", + "fn parse_objective_and_budget(args: &str) -> Result<(Option>, String)> {", + ): 1, + ( + "session-store/src/transcript_schema.rs", + "type PresentJsonValue = Option>;", + ): 1, + ( + "tui/src/overlays/mod.rs", + "pub(crate) type EffortOverrideSelection = Option>;", + ): 1, + } +) + + +@dataclass(frozen=True) +class FunctionRange: + name: str + start: int + end: int + is_test: bool + + +@dataclass(frozen=True) +class SourceAnalysis: + path: str + source: str + masked: str + functions: tuple[FunctionRange, ...] + excluded_ranges: tuple[tuple[int, int], ...] + test_only: bool + + +def mask_non_code(source: str) -> str: + """Replace comments and literals with spaces while preserving offsets.""" + + masked = list(source) + length = len(source) + index = 0 + + def blank(start: int, end: int) -> None: + for offset in range(start, end): + if masked[offset] != "\n": + masked[offset] = " " + + while index < length: + if source.startswith("//", index): + end = source.find("\n", index + 2) + end = length if end < 0 else end + blank(index, end) + index = end + continue + + if source.startswith("/*", index): + depth = 1 + end = index + 2 + while end < length and depth: + if source.startswith("/*", end): + depth += 1 + end += 2 + elif source.startswith("*/", end): + depth -= 1 + end += 2 + else: + end += 1 + blank(index, end) + index = end + continue + + raw = RAW_STRING_RE.match(source, index) + if raw is not None: + delimiter = '"' + raw.group("hashes") + content_start = raw.end() + close = source.find(delimiter, content_start) + end = length if close < 0 else close + len(delimiter) + blank(index, end) + index = end + continue + + quote_index = index + if source[index] in {"b", "c"} and index + 1 < length and source[index + 1] == '"': + quote_index += 1 + if source[quote_index] == '"': + end = quote_index + 1 + escaped = False + while end < length: + char = source[end] + end += 1 + if char == '"' and not escaped: + break + if char == "\\" and not escaped: + escaped = True + else: + escaped = False + blank(index, end) + index = end + continue + + if source[index] == "'": + character = CHARACTER_RE.match(source, index) + if character is not None: + end = character.end() + blank(index, end) + index = end + continue + + index += 1 + + return "".join(masked) + + +def matching_braces(masked: str) -> dict[int, int]: + stack: list[int] = [] + pairs: dict[int, int] = {} + for index, char in enumerate(masked): + if char == "{": + stack.append(index) + elif char == "}" and stack: + opening = stack.pop() + pairs[opening] = index + return pairs + + +def next_body_open(masked: str, start: int) -> int | None: + opening = masked.find("{", start) + terminator = masked.find(";", start) + if opening < 0 or (terminator >= 0 and terminator < opening): + return None + return opening + + +def cfg_test_ranges(masked: str, braces: dict[int, int]) -> list[tuple[int, int]]: + ranges: list[tuple[int, int]] = [] + for match in CFG_TEST_RE.finditer(masked): + opening = next_body_open(masked, match.end()) + if opening is not None and opening in braces: + ranges.append((opening, braces[opening])) + return ranges + + +def function_ranges(masked: str, braces: dict[int, int]) -> list[FunctionRange]: + test_ranges = cfg_test_ranges(masked, braces) + item_boundaries = [ + index for index, char in enumerate(masked) if char == "}" or char == ";" + ] + ranges: list[FunctionRange] = [] + for match in FUNCTION_RE.finditer(masked): + opening = next_body_open(masked, match.end()) + if opening is None or opening not in braces: + continue + end = braces[opening] + boundary_index = bisect.bisect_left(item_boundaries, match.start()) - 1 + boundary = item_boundaries[boundary_index] if boundary_index >= 0 else -1 + prefix = masked[boundary + 1 : match.start()] + is_test = bool(TEST_ATTRIBUTE_RE.search(prefix) or CFG_TEST_RE.search(prefix)) + is_test = is_test or any(start < opening < finish for start, finish in test_ranges) + ranges.append(FunctionRange(match.group(1), opening, end, is_test)) + return ranges + + +def owner_for(position: int, functions: list[FunctionRange]) -> FunctionRange | None: + owners = [function for function in functions if function.start < position < function.end] + return min(owners, key=lambda function: function.end - function.start, default=None) + + +def is_test_path(path: Path) -> bool: + parts = path.parts + return "tests" in parts or path.name.startswith("test_") or path.name.endswith("_test.rs") + + +def analyze_source(relative_path: Path, source: str) -> SourceAnalysis: + path = relative_path.as_posix() + if is_test_path(relative_path): + return SourceAnalysis(path, source, "", (), (), True) + masked = mask_non_code(source) + if FILE_CFG_TEST_RE.search(masked): + return SourceAnalysis(path, source, masked, (), (), True) + braces = matching_braces(masked) + functions = tuple(function_ranges(masked, braces)) + excluded_ranges = cfg_test_ranges(masked, braces) + excluded_ranges.extend( + (function.start, function.end) for function in functions if function.is_test + ) + return SourceAnalysis( + path, + source, + masked, + functions, + tuple(excluded_ranges), + False, + ) + + +def production_spawns_in(analysis: SourceAnalysis) -> collections.Counter[tuple[str, str]]: + owners: collections.Counter[tuple[str, str]] = collections.Counter() + if analysis.test_only: + return owners + for match in SPAWN_RE.finditer(analysis.masked): + owner = owner_for(match.start(), list(analysis.functions)) + if owner is not None and owner.is_test: + continue + owners[(analysis.path, owner.name if owner is not None else "")] += 1 + return owners + + +def production_spawn_owners(relative_path: Path, source: str) -> collections.Counter[tuple[str, str]]: + return production_spawns_in(analyze_source(relative_path, source)) + + +def rust_sources(root: Path) -> list[Path]: + return sorted(path for path in root.glob("*/src/**/*.rs") if path.is_file()) + + +def workspace_analyses(root: Path) -> list[SourceAnalysis]: + return [ + analyze_source(path.relative_to(root), path.read_text(encoding="utf-8")) + for path in rust_sources(root) + ] + + +def line_matches_in( + analyses: list[SourceAnalysis], pattern: re.Pattern[str] +) -> collections.Counter[tuple[str, str]]: + matches: collections.Counter[tuple[str, str]] = collections.Counter() + for analysis in analyses: + if analysis.test_only: + continue + for match in pattern.finditer(analysis.masked): + if any( + start < match.start() < end for start, end in analysis.excluded_ranges + ): + continue + line_start = analysis.source.rfind("\n", 0, match.start()) + 1 + line_end = analysis.source.find("\n", match.end()) + line_end = len(analysis.source) if line_end < 0 else line_end + matches[ + (analysis.path, normalize_line(analysis.source[line_start:line_end])) + ] += 1 + return matches + + +def line_matches(root: Path, pattern: re.Pattern[str]) -> collections.Counter[tuple[str, str]]: + return line_matches_in(workspace_analyses(root), pattern) + + +def production_spawns_in_workspace( + analyses: list[SourceAnalysis], +) -> collections.Counter[tuple[str, str]]: + owners: collections.Counter[tuple[str, str]] = collections.Counter() + for analysis in analyses: + owners.update(production_spawns_in(analysis)) + return owners + + +def all_production_spawns(root: Path) -> collections.Counter[tuple[str, str]]: + return production_spawns_in_workspace(workspace_analyses(root)) + + +def parse_spawn_allow_list(path: Path) -> dict[tuple[str, str], tuple[int, str]]: + expected: dict[tuple[str, str], tuple[int, str]] = {} + for line_number, raw_line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1): + if not raw_line or raw_line.startswith("#"): + continue + fields = raw_line.split("\t") + if len(fields) != 4: + raise ValueError(f"{path}:{line_number}: expected four tab-separated fields") + file_name, owner, raw_count, disposition = fields + try: + count = int(raw_count) + except ValueError as error: + raise ValueError(f"{path}:{line_number}: invalid count {raw_count!r}") from error + if count < 1 or SPAWN_DISPOSITION_RE.fullmatch(disposition) is None: + raise ValueError( + f"{path}:{line_number}: count must be positive and disposition must use " + "complete:, deferred:, or best-effort:" + ) + key = (file_name, owner) + if key in expected: + raise ValueError(f"{path}:{line_number}: duplicate owner {file_name}:{owner}") + expected[key] = (count, disposition) + return expected + + +def report_counter_difference( + label: str, + expected: collections.Counter[tuple[str, str]], + actual: collections.Counter[tuple[str, str]], +) -> bool: + if actual == expected: + print(f"✓ {label}: {sum(actual.values())} reviewed occurrence(s)") + return True + print(f"error: {label} changed; classify the boundary before updating the audit:", file=sys.stderr) + for key in sorted(set(expected) | set(actual)): + if expected[key] != actual[key]: + print( + f" {key[0]}: {key[1]} (expected {expected[key]}, actual {actual[key]})", + file=sys.stderr, + ) + return False + + +def audit_spawns( + root: Path, actual: collections.Counter[tuple[str, str]] | None = None +) -> bool: + allow_path = root / SPAWN_ALLOW_LIST + if not allow_path.is_file(): + print(f"error: spawn allow-list not found: {allow_path}", file=sys.stderr) + return False + try: + expected = parse_spawn_allow_list(allow_path) + except ValueError as error: + print(f"error: {error}", file=sys.stderr) + return False + actual = actual if actual is not None else all_production_spawns(root) + expected_counts = collections.Counter({key: value[0] for key, value in expected.items()}) + if actual != expected_counts: + print( + "error: production tokio::spawn owners changed; add a lifecycle disposition " + "before updating the allow-list:", + file=sys.stderr, + ) + for key in sorted(set(expected_counts) | set(actual)): + if expected_counts[key] != actual[key]: + print( + f" {key[0]}::{key[1]} (expected {expected_counts[key]}, actual {actual[key]})", + file=sys.stderr, + ) + return False + print( + f"✓ production tokio::spawn owners: {sum(actual.values())} spawn(s) " + f"across {len(actual)} reviewed owner anchor(s)" + ) + return True + + +class ScannerTests(unittest.TestCase): + def test_boundary_scanner_excludes_test_only_declarations(self) -> None: + source = """ +struct Production { + permission_mode: Option, + update: Option>, +} + +#[cfg(test)] +mod tests { + struct Fixture { + permission_mode: Option, + update: Option>, + } +} +""" + analysis = analyze_source(Path("crate/src/lib.rs"), source) + self.assertEqual( + line_matches_in([analysis], PERMISSION_STRING_RE), + collections.Counter( + {("crate/src/lib.rs", "permission_mode: Option,"): 1} + ), + ) + self.assertEqual( + line_matches_in([analysis], NESTED_OPTION_RE), + collections.Counter( + {("crate/src/lib.rs", "update: Option>,"): 1} + ), + ) + + def test_spawn_scanner_ignores_tests_comments_and_literals(self) -> None: + source = r''' +async fn production() { + tokio::spawn(async move {}); + let _literal = r#"tokio::spawn(async move { fake() })"#; + // tokio::spawn(async move {}); + /* tokio::spawn(async move {}); */ +} + +#[cfg(test)] +mod tests { + async fn helper() { + tokio::spawn(async move {}); + } +} + +#[tokio::test] +async fn standalone_test() { + tokio::spawn(async move {}); +} + +#[cfg(not(test))] +mod production_build { + async fn production_when_not_testing() { + tokio::spawn(async move {}); + } +} + +fn two_spawns() { + tokio::spawn(async move {}); + tokio::spawn(async move {}); +} +''' + self.assertEqual( + production_spawn_owners(Path("crate/src/lib.rs"), source), + collections.Counter( + { + ("crate/src/lib.rs", "production"): 1, + ("crate/src/lib.rs", "production_when_not_testing"): 1, + ("crate/src/lib.rs", "two_spawns"): 2, + } + ), + ) + + def test_dedicated_test_paths_are_excluded(self) -> None: + source = "fn helper() { tokio::spawn(async move {}); }" + self.assertFalse(production_spawn_owners(Path("crate/src/tests/helper.rs"), source)) + + def test_mask_preserves_offsets_and_braces(self) -> None: + source = 'fn f() { let _ = "}"; /* { nested /* } */ } */ call(); }' + masked = mask_non_code(source) + self.assertEqual(len(source), len(masked)) + braces = matching_braces(masked) + opening = masked.index("{") + self.assertEqual(source[braces[opening]], "}") + + +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--repo", + type=Path, + help="workspace root (defaults to the parent of this script directory)", + ) + parser.add_argument( + "--list-spawns", + action="store_true", + help="print detected production spawn owner/count anchors", + ) + parser.add_argument( + "--self-test", + action="store_true", + help="run focused scanner tests", + ) + return parser.parse_args() + + +def main() -> None: + args = parse_args() + if args.self_test: + suite = unittest.defaultTestLoader.loadTestsFromTestCase(ScannerTests) + result = unittest.TextTestRunner(verbosity=2).run(suite) + raise SystemExit(0 if result.wasSuccessful() else 1) + + root = (args.repo or Path(__file__).resolve().parent.parent).resolve() + if not (root / "Cargo.toml").is_file(): + raise SystemExit(f"error: no Cargo.toml at workspace root: {root}") + + if args.list_spawns: + analyses = workspace_analyses(root) + for (path, owner), count in sorted( + production_spawns_in_workspace(analyses).items() + ): + print(f"{path}\t{owner}\t{count}\tUNCLASSIFIED") + return + + analyses = workspace_analyses(root) + spawns = production_spawns_in_workspace(analyses) + checks = [ + report_counter_difference( + "string-typed permission_mode boundaries", + EXPECTED_PERMISSION_STRINGS, + line_matches_in(analyses, PERMISSION_STRING_RE), + ), + report_counter_difference( + "raw nested-option boundaries", + EXPECTED_NESTED_OPTIONS, + line_matches_in(analyses, NESTED_OPTION_RE), + ), + audit_spawns(root, spawns), + ] + if not all(checks): + raise SystemExit(1) + print("Rust maintenance boundary audit passed.") + + +if __name__ == "__main__": + main() diff --git a/scripts/check.sh b/scripts/check.sh index a7c6e00..aeb397d 100755 --- a/scripts/check.sh +++ b/scripts/check.sh @@ -4,8 +4,8 @@ # Runs the same checks as CI so local failures surface before push. # # Usage: -# scripts/check.sh # full: fmt → clippy → check → test -# scripts/check.sh --quick # fast: fmt → clippy → check (no tests) +# scripts/check.sh # full: fmt → clippy → check → audits → test +# scripts/check.sh --quick # fast: fmt → clippy → check → audits # scripts/check.sh --release # full pipeline in release profile # scripts/check.sh --crate orbcode-config # test only the specified crate # scripts/check.sh --pty-e2e # ONLY the #[ignore]d PTY e2e tests (serial) @@ -26,7 +26,7 @@ usage() { Usage: scripts/check.sh [OPTIONS] Options: - --quick Skip tests (fmt + clippy + check only) + --quick Skip tests (fmt + clippy + check + audits only) --release Use the release profile --crate Run tests for a single crate only (e.g. orbcode-config) --pty-e2e Run ONLY the load-sensitive #[ignore]d PTY e2e tests, serially @@ -144,6 +144,9 @@ cargo check --workspace ${PROFILE_FLAG} step "cargo check (no default features)" cargo check --workspace --no-default-features ${PROFILE_FLAG} +step "Rust maintenance boundary audit" +"${REPO_ROOT}/scripts/audit-rust-maintenance.py" + step "public API surface audit" "${REPO_ROOT}/scripts/audit-public-surface.sh" @@ -151,7 +154,7 @@ step "brand audit" "${REPO_ROOT}/scripts/audit-brand.sh" if [ "$QUICK" = true ]; then - printf '\n\033[1;32m✓ Quick check passed (fmt + clippy + check).\033[0m\n' + printf '\n\033[1;32m✓ Quick check passed (fmt + clippy + check + audits).\033[0m\n' exit 0 fi diff --git a/scripts/rust-maintenance-spawn-allow-list.txt b/scripts/rust-maintenance-spawn-allow-list.txt new file mode 100644 index 0000000..b384fcb --- /dev/null +++ b/scripts/rust-maintenance-spawn-allow-list.txt @@ -0,0 +1,46 @@ +# Production tokio::spawn owner allow-list for the Slice 8 maintenance audit. +# +# Generated anchors come from: +# scripts/audit-rust-maintenance.py --list-spawns +# +# The disposition is a compact pointer to the semantic classification in +# docs/plans/rust-maintenance-current-head-inventory.md and the Slice 8 closeout. +# A changed count or a new owner must be reviewed before this file is updated. +# +# Format: pathowning-functioncountdisposition + +app-server-client/src/child_stdio_transport.rs spawn 4 deferred:client-stdio-lifecycle +app-server-client/src/in_process.rs new 2 deferred:in-process-drop-panic +app-server-client/src/lib.rs start_router_tasks 2 best-effort:app-client-routing +app-server-client/src/ndjson_transport.rs connect 1 deferred:ndjson-reader-drop +app-server-client/src/websocket_transport.rs connect 2 deferred:websocket-client-transport +app-server-transport/src/stdio.rs run_transport 2 complete:await-peer-abort +app-server-transport/src/websocket.rs handle_ws_connection 2 complete:await-peer-abort +app-server/src/background_api.rs background_task_progress_stream 1 best-effort:progress-forwarder +app-server/src/background_api.rs submit_background_turn 1 complete:terminal-cancel-owner +app-server/src/message_processor.rs pump_events 3 deferred:pending-request-waiters +app-server/src/message_processor.rs register_subscription_pump 1 complete:subscription-pump-joinset +app-server/src/message_processor.rs resolve_server_request 1 best-effort:pending-response-resolver +cli/src/acp_sdk/mod.rs start_server_request_pump 1 deferred:acp-request-pump +cli/src/acp_sdk/server_requests.rs server_request_pump 3 deferred:acp-request-handlers +cli/src/headless.rs run_control_input_loop 1 best-effort:stdin-control-reader +cli/src/main.rs main 2 complete:serve-readiness-abort +core/src/hook_runner/command.rs run_command_hook_capture 1 best-effort:bounded-hook-stdin +core/src/hook_runner/command.rs run_nonblocking_command_hook 1 best-effort:bounded-hook-stdin +core/src/session_manager/mod.rs submit_turn_with_interaction 1 deferred:core-turn-driver-panic +core/src/session_manager/session_background_agent.rs start_background_agent_task 2 deferred:background-agent-supervisor +core/src/session_manager/session_goal.rs spawn_supervised_goal_turn 2 deferred:persistent-goal-supervisor +core/src/session_manager/session_response.rs start_streamed_tool_execution 2 complete:streamed-tool-owner +core/src/session_manager/session_workflows.rs invoke_agent 1 deferred:workflow-child-drain +core/src/session_manager/session_workflows.rs spawn_workflow_run 1 deferred:workflow-run-supervisor +core/src/tool_runtime.rs attach_ask_user_channel 1 complete:ask-user-forwarder +mcp/src/transport/stdio.rs spawn_stderr_capture 1 deferred:mcp-stdio-shutdown-join +tools/src/bash.rs run_bash 2 complete:tool-process-join +tools/src/process.rs run_command_output 2 complete:tool-process-join +tools/src/skills.rs discover_mcp_skill_prompts_with_timeout 2 complete:mcp-skill-observe-log +tui/src/app.rs run_tui 2 best-effort:tui-statusline +tui/src/app.rs subscribe_pending_transcript_task_cards 1 best-effort:tui-task-stream +tui/src/commands/async_local.rs start_async_local_slash_command 1 best-effort:tui-command-operation +tui/src/commands/async_local.rs start_context_slash_command 1 best-effort:tui-command-operation +tui/src/commands/compact.rs start_compact_slash_command 1 best-effort:tui-command-operation +tui/src/commands/tui_local.rs run_tui_local_slash_command 1 best-effort:tui-command-operation diff --git a/tools/src/skills.rs b/tools/src/skills.rs index e5f2dab..7f01b46 100644 --- a/tools/src/skills.rs +++ b/tools/src/skills.rs @@ -744,6 +744,7 @@ while IFS= read -r line; do printf '{"jsonrpc":"2.0","id":%s,"result":{"protocolVersion":"2024-11-05","capabilities":{"tools":{},"prompts":{}},"serverInfo":{"name":"slow-stdio","version":"0.1.0"}}}\n' "$id" ;; *\"method\":\"prompts/list\"*) + printf 'prompts/list\n' >> "$MARKER_PATH" sleep 0.7 printf '{"jsonrpc":"2.0","id":%s,"result":{"prompts":[]}}\n' "$id" ;; @@ -797,7 +798,17 @@ done .expect("load skills"); assert!(skills.is_empty()); - tokio::time::sleep(Duration::from_millis(1000)).await; + tokio::time::timeout(Duration::from_secs(10), async { + loop { + let events = tokio::fs::read_to_string(&marker).await.unwrap_or_default(); + if events.lines().any(|event| event == "prompts/list") { + break; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("slow prompts/list request should start"); registry .list_tools("slow") .await @@ -805,7 +816,7 @@ done let starts = tokio::fs::read_to_string(&marker).await.expect("starts"); assert_eq!( - starts.lines().count(), + starts.lines().filter(|event| *event == "started").count(), 1, "timed-out skill discovery must not kill and restart the stdio MCP server" );