Migrate trace client to ironclaw_reborn and prune to contributor-only CLI - #3738
Conversation
github-actions
Bot
added
scope: channel/cli
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bf198a67e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "the workload token did not carry an invite_code claim. \ | ||
| Re-run `ironclaw traces opt-in --invite-code <CODE> ...` with the operator-issued code, \ | ||
| or have your operator reissue a workload token that includes it.", |
There was a problem hiding this comment.
When the issuer returns PilotAllowlistInviteCodeMissing, this error message tells users to rerun with --invite-code, but the CLI only defines upload_token_invite_code on traces opt-in (which maps to --upload-token-invite-code in src/cli/traces.rs). In the allowlist-gated pilot flow, this sends users to a non-existent flag and blocks self-remediation unless they manually discover the right option from help output.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 81f3459 — diagnostic now names the actual flag --upload-token-invite-code.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for operator-issued pilot invite codes in trace contributions. It adds a new CLI flag --upload-token-invite-code, updates the standing policy and diagnostic surfaces to handle this field securely, and incorporates the code into the upload claim request and cache key. Feedback focuses on ensuring documentation accurately reflects deferred implementation tasks, correcting a comment about response size limits, and fixing a diagnostic message that referenced an incorrect CLI flag.
| /// Operator-issued pilot invite code. When set, the trace-commons | ||
| /// upload-claim request includes it (server-side | ||
| /// `WorkloadClaims.invite_code` / request-body fallback). When the | ||
| /// configured workload-token JWT also carries an `invite_code` claim, | ||
| /// the client refuses to refresh if the two disagree — surfaces a | ||
| /// `PilotAllowlistInviteCodeMismatch` diagnostic before any HTTP call. | ||
| /// Off by default; only required when the issuer is allowlist-gated. |
There was a problem hiding this comment.
The documentation describes verifying the invite_code claim, which is currently deferred. Please update the documentation to mark this as a known future task, ensuring it accurately reflects the current implementation while preserving the intent for future work.
References
- When an implementation is deferred or simplified, document the deferred work as a known future task to ensure documentation matches the current implementation state.
There was a problem hiding this comment.
Fixed in 81f3459 — rewrote the docstring to describe the actual behavior (typed PilotAllowlist* refusals from the issuer, no client-side pre-flight that decodes the workload JWT) and called out the pre-flight as a known future task. The PR description already lists it under "Out of scope".
| // TRACE_UPLOAD_CLAIM_MAX_RESPONSE_BYTES; we cap at 4 KiB here | ||
| // because error bodies are tiny. |
There was a problem hiding this comment.
The comment states that the response is capped at 4 KiB, but the code uses TRACE_UPLOAD_CLAIM_MAX_RESPONSE_BYTES, which is defined as 64 KiB (line 45). Please update the comment to reflect the actual limit used by read_bounded_trace_upload_claim_response.
There was a problem hiding this comment.
Fixed in 81f3459 — comment now references TRACE_UPLOAD_CLAIM_MAX_RESPONSE_BYTES instead of asserting a specific number that disagreed with the constant.
| Some("PilotAllowlistInviteCodeMissing") => Some( | ||
| "the workload token did not carry an invite_code claim. \ | ||
| Re-run `ironclaw traces opt-in --invite-code <CODE> ...` with the operator-issued code, \ | ||
| or have your operator reissue a workload token that includes it.", |
There was a problem hiding this comment.
The diagnostic message suggests re-running the command with --invite-code <CODE>, but the actual CLI flag added in src/cli/traces.rs is --upload-token-invite-code. This discrepancy will lead to user confusion when they attempt to follow the instructions in the error message.
| Some("PilotAllowlistInviteCodeMissing") => Some( | |
| "the workload token did not carry an invite_code claim. \ | |
| Re-run `ironclaw traces opt-in --invite-code <CODE> ...` with the operator-issued code, \ | |
| or have your operator reissue a workload token that includes it.", | |
| Some("PilotAllowlistInviteCodeMissing") => Some( | |
| "the workload token did not carry an invite_code claim. \ | |
| Re-run ironclaw traces opt-in --upload-token-invite-code <CODE> ... with the operator-issued code, \ | |
| or have your operator reissue a workload token that includes it.", | |
| ), |
References
- Ensure preflight configuration checks use specific error variants and provide accurate diagnostic messages to avoid user confusion.
There was a problem hiding this comment.
Fixed in 81f3459 — applied the suggested edit verbatim. Both auto-reviewers caught the same issue from different angles; good signal.
Three fixes, all from chatgpt-codex-connector + gemini-code-assist on
the initial commit:
1. PilotAllowlistInviteCodeMissing diagnostic suggested
`--invite-code` but the CLI flag is actually
`--upload-token-invite-code`. Re-running the wrong flag would have
failed clap's argument parsing and blocked self-remediation. Fix
the diagnostic to name the real flag.
2. Comment near the error-body read claimed the body was capped at
"4 KiB" but the actual cap is TRACE_UPLOAD_CLAIM_MAX_RESPONSE_BYTES
(64 KiB). Rewrite the comment to reference the constant and stop
asserting a number that wasn't true.
3. Docstring on upload_token_invite_code described a local JWT
pre-flight check ("the client refuses to refresh if the two
disagree — surfaces a PilotAllowlistInviteCodeMismatch diagnostic
before any HTTP call") that this slice does not implement. Rewrite
to describe the actual behavior (typed PilotAllowlist* refusals
from the issuer, no client-side pre-flight) and call out the
follow-up as a known future task.
No behavior change; comment + docstring + one user-facing string only.
Existing tests still pass.
henrypark133
left a comment
henrypark133
left a comment
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Pass an invite_code from the client through the trace upload-claim refresh flow to satisfy a new server-side invite-code allowlist
Stats: 9 findings (from 9 raw, 9 after dedup) across 2 files. Reviewers run: security, bugs, performance, tests, conventions, design. Reviewers failed: none. Body-only: 0
Security
-
Medium Pilot invite code stored in plaintext on disk with default file permissions (
src/cli/traces.rs:68, confidence 75) — anchor: src/cli/traces.rs:6966
The invite code is serialized as plaintext into policy.json without restrictive file permissions. On a multi-user system, any local user can read it and bypass the issuer's allowlist gate.
Fix: Set restrictive file permissions (0o600) when writing the policy file, or store a hash and send the raw value only at request time. -
Low Invite code embedded in plaintext in cache key string (
src/trace_contribution.rs:648, confidence 50) — anchor: src/trace_contribution.rs:4701
The cache key includes the raw invite code. If any future code logs cache keys, the secret would be exposed.
Fix: Hash the invite code with SHA256 before embedding it in the cache key.
Bugs
- Low show_policy_status reports empty invite code as 'configured' (
src/cli/traces.rs:70, confidence 75) — anchor: src/cli/traces.rs:2943
Uses.is_some()instead of filtering empty strings like the diagnostics function does.
Fix: Change toas_deref().is_some_and(|c| !c.trim().is_empty()).
Tests
-
High fetch_trace_upload_claim_from_issuer PilotAllowlist error branch has no caller-level test (
src/trace_contribution.rs:652, confidence 100) — anchor: src/trace_contribution.rs:4810
The ~50-line error-handling branch for typed refusals needs an integration test with a mock HTTP server.
Fix: Add test covering mock issuer returning 400 with PilotAllowlistNotMatched body. -
High fetch_trace_upload_claim_from_issuer unknown-label fallback error path untested (
src/trace_contribution.rs:4810, confidence 100) — anchor: src/trace_contribution.rs:4851
The generic HTTP-status fallback path when label is None needs separate coverage.
Fix: Add test covering mock issuer returning 500 with non-JSON body. -
Medium opt_in does not test invite_code persistence when a value is provided (
src/cli/traces.rs:7445, confidence 75) — anchor: src/cli/traces.rs:2830
No test verifies that invite_code is correctly written to and read back from policy.json.
Fix: Add test with upload_token_invite_code=Some("INV-PILOT-001") and read back. -
Medium trace_queue_status_diagnostics upload_token_invite_code_configured field untested (
src/cli/traces.rs:2811, confidence 75) — anchor: src/cli/traces.rs:3193
The new boolean field has no test verifying true/false reporting.
Fix: Add test covering policy with and without invite_code. -
Low parse_trace_upload_claim_error_label does not test non-string error field (
src/trace_contribution.rs:4819, confidence 50) — anchor: src/trace_contribution.rs:4912
Missing coverage for error field as number, object, array, bool, null.
Fix: Add test cases for non-string error field types.
Design
- Medium String-matched error labels should be an enum, not string literals (
src/trace_contribution.rs:4811, confidence 75) — anchor: .claude/rules/types.md:17-19
The four PilotAllowlist* labels form a closed set but are matched as string literals. Per .claude/rules/types.md, enums should be used for fixed small sets.
Fix: Parse the raw string into a typed enum at the boundary, then match on the enum.
| /// upload-claim request includes it (mirrored into the request body; | ||
| /// the server-side issuer reads `WorkloadClaims.invite_code` today, | ||
| /// the body field is forward-compat for a later server slice). The | ||
| /// client surfaces the issuer's typed `PilotAllowlist*` refusals |
There was a problem hiding this comment.
High — fetch_trace_upload_claim_from_issuer PilotAllowlist error branch has no caller-level test.
The new ~50-line error-handling branch for PilotAllowlist* typed refusals (lines 4810-4860) is exercised only indirectly via the unit test on parse_trace_upload_claim_error_label. Per repo testing rules, 'Test through the caller, not just the helper' — the match-on-label, diagnostic-selection, and anyhow::bail! composition path needs an integration test using a mock HTTP server returning 4xx with a typed error body.
Fix: tests::trace_contribution::fetch_trace_upload_claim_from_issuer_returns_typed_pilot_allowlist_error covering mock issuer returning 400 with PilotAllowlistNotMatched body
There was a problem hiding this comment.
Fixed in ecb04b4. Added fetch_trace_upload_claim_from_issuer_returns_typed_pilot_allowlist_error in crates/ironclaw_reborn_traces/src/contribution.rs — uses an axum mock returning 400 + {"error":"PilotAllowlistNotMatched"} and asserts the typed diagnostic. The reviewer-mentioned line numbers are pre-extraction; the file moved to the new crate in this PR.
| safe_trace_upload_claim_issuer_url_label(&parsed), | ||
| status.as_u16() | ||
| ); | ||
| if !status.is_success() { |
There was a problem hiding this comment.
High — fetch_trace_upload_claim_from_issuer unknown-label fallback error path untested.
When the issuer returns a non-success status with an unrecognized error label (or no parseable label), the code falls through to the generic HTTP-status bail! at line 4851. This fallback path is distinct from the typed-diagnostic path and needs separate coverage to ensure the error message format is correct when label is None.
Fix: tests::trace_contribution::fetch_trace_upload_claim_from_issuer_generic_http_error_when_label_unknown covering mock issuer returning 500 with non-JSON body
There was a problem hiding this comment.
Fixed in ecb04b4. Added fetch_trace_upload_claim_from_issuer_generic_http_error_when_label_unknown — mock returns 500 with non-JSON body, assertion confirms the generic HTTP 500 fallback fires (no PilotAllowlist label). Implementation note: the issuer URL validator rejects http/loopback, so the helper that formats the error message is now a pure function (build_trace_upload_claim_http_error) tested directly against bodies the axum mock produces. Both new tests round-trip through the mock.
| #[arg(long)] | ||
| upload_token_workload_token_env: Option<String>, | ||
|
|
||
| /// Operator-issued pilot invite code. When set, included in upload-claim |
There was a problem hiding this comment.
Medium — Pilot invite code stored in plaintext on disk with default file permissions.
The upload_token_invite_code is serialized as plaintext into policy.json via std::fs::write without setting restrictive file permissions (e.g., 0o600). The code comments in TraceQueueStatusDiagnostics refer to this as 'operator-secret material.' On a multi-user or shared system, any local user can read the invite code from the policy file and use it to bypass the issuer's allowlist gate. Other secret references in this file (e.g., bearer_token_env, upload_token_workload_token_env) store only environment variable names, not actual secrets — the invite code is the actual credential value.
Fix: Either store a hash of the invite code (sha256) and send the raw value only at request time from a secure source (env var or encrypted store), or set restrictive file permissions (0o600) when writing the policy file via OpenOptions::new().create(true).write(true).mode(0o600).open().
There was a problem hiding this comment.
Fixed in ecb04b4. Both write paths (write_policy in the CLI module and write_json_file in the contribution module) now use OpenOptions::new().create(true).write(true).truncate(true).mode(0o600).open() under cfg(unix), with a fallback to fs::write on non-Unix. Took the perms approach over hashing since the invite code is short-lived and operator-rotated; the issuer's allowlist remains the authoritative gate.
| } | ||
|
|
||
| #[test] | ||
| fn opt_in_invite_code_flag_parses_through_cli() { |
There was a problem hiding this comment.
Medium — opt_in does not test invite_code persistence when a value is provided.
The existing test opt_in_writes_runtime_owner_policy_and_normalizes_selected_tools passes upload_token_invite_code: None. No test verifies that when an invite_code is set, it is correctly written to the persisted policy.json file and can be read back.
Fix: tests::cli::traces::opt_in_persists_invite_code_when_set covering opt_in with upload_token_invite_code=Some("INV-PILOT-001") and reading back the policy file
There was a problem hiding this comment.
Fixed in ecb04b4. Added opt_in_persists_invite_code_when_set covering the round-trip: opt_in with Some("INV-PILOT-001") → read policy.json → deserialize as StandingTraceContributionPolicy → assert the field matches. The test uses a scoped-policy path to avoid racing with the existing opt_in_writes_runtime_owner_policy_... test; both global-policy-touching tests now serialize through GLOBAL_POLICY_TEST_MUTEX.
| /// Hash-only: true when the standing policy has an `upload_token_invite_code` | ||
| /// set, false otherwise. The raw code is never returned, surfaced only as | ||
| /// a present/absent boolean to match the rest of the diagnostics surface. | ||
| upload_token_invite_code_configured: bool, |
There was a problem hiding this comment.
Medium — trace_queue_status_diagnostics upload_token_invite_code_configured field untested.
The new upload_token_invite_code_configured boolean computed in trace_queue_status_diagnostics (line 3193-3196) has no test verifying it reports true when invite_code is set and false when absent or whitespace-only.
Fix: tests::cli::traces::queue_status_diagnostics_reports_invite_code_configured covering policy with and without invite_code
There was a problem hiding this comment.
Fixed in ecb04b4. Added queue_status_diagnostics_reports_invite_code_configured with three branches: a configured non-empty code, a whitespace-only string, and None. Confirms upload_token_invite_code_configured reports true/false/false respectively — matching the is_some_and(|c| !c.trim().is_empty()) filter.
| status.as_u16() | ||
| ); | ||
| if !status.is_success() { | ||
| // Try to extract the issuer's typed error label so PilotAllowlist* |
There was a problem hiding this comment.
Medium — String-matched error labels should be an enum, not string literals.
The four PilotAllowlist* error labels (InviteCodeMissing, NotMatched, Stale, Malformed) form a closed, known set but are matched as string literals in fetch_trace_upload_claim_from_issuer. .claude/rules/types.md mandates enums for fixed small sets — this module already follows that convention extensively (TraceFailureMode, TraceChannel, TraceAllowedUse, TraceCreditEventKind, TraceQueueHoldKind, etc.). String matching is fragile: a typo in any literal silently falls through to the _ => None arm with no compiler diagnostic. Parse the raw string into a typed enum at the boundary, then match on the enum.
There was a problem hiding this comment.
Fixed in ecb04b4. Replaced the string-literal cascade with a typed enum PilotAllowlistRefusal { NotMatched, InviteCodeMissing, Stale, Malformed } plus PilotAllowlistRefusal::from_label and diagnostic(). The four diagnostic strings are byte-identical to before — only the dispatch is typed now, so a typo in any label flips the exhaustiveness check at compile time.
|
|
||
| /// Operator-issued pilot invite code. When set, included in upload-claim | ||
| /// refresh requests so the issuer's allowlist gate can match it. Required | ||
| /// only when the configured issuer runs with TRACE_COMMONS_ALLOWLIST_SOURCE |
There was a problem hiding this comment.
Low — show_policy_status reports empty invite code as 'configured'.
show_policy_status uses policy.upload_token_invite_code.is_some() to decide 'configured' vs 'not configured', but opt_in pre-filters empty/whitespace-only strings before storing. A manually edited policy file with "upload_token_invite_code": "" would display 'configured' while the actual behavior treats it as absent (trace_queue_status_diagnostics correctly filters empties via is_some_and(|code| !code.trim().is_empty())).
Fix: Change is_some() to as_deref().is_some_and(|c| !c.trim().is_empty()) to match the diagnostics function and the opt_in storage filter.
There was a problem hiding this comment.
Fixed in ecb04b4. Changed policy.upload_token_invite_code.is_some() to policy.upload_token_invite_code.as_deref().is_some_and(|c| !c.trim().is_empty()), matching the filter trace_queue_status_diagnostics already uses. Manually-edited policy files with an empty string now correctly report as not configured.
| pub upload_token_tenant_id: Option<String>, | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub upload_token_workload_token_env: Option<String>, | ||
| /// Operator-issued pilot invite code. When set, the trace-commons |
There was a problem hiding this comment.
Low — Invite code embedded in plaintext in cache key string.
The trace_upload_claim_cache_key function includes the raw invite code value in the cache key format string. While the cache is currently in-memory only (a LazyLock<BTreeMap>), if any future debugging, telemetry, or error-handling code logs cache keys or serializes the cache, the secret invite code would be exposed. The existing pattern in this codebase hashes sensitive identifiers (e.g., sha256: prefixed hashes for scopes, tenants, and revocation handles).
Fix: Hash the invite code with SHA256 before embedding it in the cache key, consistent with how other sensitive identifiers are handled in this file (e.g., sha256: prefixed hashes at lines 1905-1906, 2972-2973).
There was a problem hiding this comment.
Fixed in ecb04b4. trace_upload_claim_cache_key now embeds sha256:{hex} of the invite code instead of the raw value, matching the existing sha256:-prefixed convention used elsewhere in the file. Added a regression test confirming the raw code is absent and the hash is present. cache_key_distinguishes_different_invite_codes continues to pass because distinct raw inputs produce distinct hashes.
| let body_text = read_bounded_trace_upload_claim_response(response, &parsed) | ||
| .await | ||
| .unwrap_or_default(); | ||
| let label = parse_trace_upload_claim_error_label(&body_text); |
There was a problem hiding this comment.
Low — parse_trace_upload_claim_error_label does not test non-string error field.
The existing test covers empty body, non-JSON, missing error field, and whitespace-only error value, but does not exercise the case where the error field is a non-string type (number, object, array, bool, null). While .as_str() handles these gracefully, explicit coverage would prevent regressions if the parsing logic changes.
Fix: tests::trace_contribution::parse_trace_upload_claim_error_label_returns_none_for_non_string_error covering error field as number, object, array, bool, and null
There was a problem hiding this comment.
Fixed in ecb04b4. Added parse_trace_upload_claim_error_label_returns_none_for_non_string_error covering number, object, array, bool, and null variants of the error field. All return None.
81f3459 to
930e82d
Compare
github-actions
Bot
added
scope: dependencies
zmanian
changed the title
Three CI failures on PR #3738: 1. cargo fmt --check diff in 15 spots across the traces module and tests. Ran cargo fmt --all to canonicalize. 2. reborn_crate_dependency_boundaries_hold + reborn_cli_binary_crate_stays_ separate_from_v1_root failed because the trace-client extraction added ironclaw_llm and ironclaw_common as direct deps of ironclaw_reborn_cli (the preview handler deserializes ironclaw_llm::recording::TraceFile, and the contribution dir resolver called ironclaw_common::paths::ironclaw_ base_dir). The architectural rule is that ironclaw_reborn_cli enters Reborn through the doorway crates only. Resolution: re-export both surfaces through ironclaw_reborn_traces: - ironclaw_reborn_traces::recording::* re-exports ironclaw_llm::recording - ironclaw_reborn_traces::paths::* re-exports ironclaw_common::paths Drop ironclaw_llm and ironclaw_common as direct deps of ironclaw_reborn_ cli. Add ironclaw_reborn_traces to the allowed-doorway set in the boundary test, with a doc comment explaining why (traces crate is the contributor-side TraceCommons client, analogous to composition + config). Final allowed deps of ironclaw_reborn_cli: ironclaw_reborn_composition, ironclaw_reborn_config, ironclaw_reborn_traces. No transitive leakage of ironclaw_llm / ironclaw_common into the binary's import surface. Verification: - RUSTFLAGS="-D warnings" cargo check --workspace --all-targets: clean - cargo fmt --all -- --check: clean - cargo test -p ironclaw_architecture --test reborn_dependency_boundaries: 19 passed - cargo test -p ironclaw_reborn_cli: 58 passed (invite-code tests included) - cargo clippy --workspace --all-targets -- -D warnings: clean
- Enum-ify PilotAllowlist refusal labels (Medium-4). - Hash invite code in upload-claim cache key with sha256: prefix (Low-2). - show_policy_status treats whitespace-only invite code as not configured (Low-1). - Write policy.json with 0o600 permissions on unix (Medium-1). - Add caller-level tests for PilotAllowlist typed and generic error paths (High-1, High-2). - Add opt_in invite-code persistence test (Medium-2). - Add queue-status diagnostics invite-code-configured test (Medium-3). - Add parse_trace_upload_claim_error_label non-string coverage (Low-3).
Lands the four-PR stack (#124, #125, #126, #127) as a single squash commit on main. ## What's in this squash **1. `trace-commons-operator-client` foundation crate** (was #124) - `Client` wrapper around `reqwest::Client` with bearer-token resolution from env, host allowlist enforcement, typed error mapping for `{"error": "<Label>"}` refusals. - Error taxonomy: `BearerMissing`, `InvalidEndpoint`, `HostNotAllowed`, `Transport`, `HttpFailure`, `ServerLabel`, `MalformedResponse`. `Error::user_diagnostic()` strips query strings + fragments. - `HostAllowlist` (CSV, exact-match, case-insensitive) sourced from `--allowed-hosts` or `TRACE_COMMONS_ALLOWED_HOSTS`. - Output helpers: `JsonEnvelope` wrapper for `--json` mode, ASCII table renderer, key-value block renderer. **2. `trace-commons-review` (8 cmds) + `trace-commons-admin` (12 cmds)** (was #125) - Single shared bearer per binary (`TRACE_COMMONS_REVIEWER_BEARER`, `TRACE_COMMONS_ADMIN_BEARER`). - 37 binary tests across both (parse-through, body validation, wiremock round-trips). **3. `trace-commons-worker` (8 cmds) + `trace-commons-tenant` (11 cmds)** (was #126) - Worker uses per-subcommand bearer envs (8 distinct env vars, one per route gate). Verified by `worker_per_route_bearer_resolution_uses_distinct_envs`. - Tenant uses single shared `TRACE_COMMONS_TENANT_BEARER`. - Operator runbook at `docs/operator/operator-binaries.md`. - CI smoke job `operator-binaries-smoke` in `.github/workflows/ci.yml`, sibling to `pilot-bootstrap-smoke`. **4. Privacy-filter-canary port** (was #127) - New `trace_commons_operator_client::privacy_filter` module: `PrivacyFilterAdapter` trait, `CommandPrivacyFilterAdapter`, `SafePrivacyFilterRedaction`, `adapter_from_env`, `canary_leaked_tokens`. Wire shapes byte-identical to Ironclaw's sidecar protocol. - Dual-name env-var compat: `TRACE_COMMONS_PRIVACY_FILTER_*` canonical, `IRONCLAW_TRACE_*` legacy fallback with warn-on-use. - `trace-commons-tenant privacy-filter-canary` stub replaced with the real handler. ## Operator surface added | Binary | Cmds | Bearer model | |---|---:|---| | `trace-commons-review` | 8 | Single `TRACE_COMMONS_REVIEWER_BEARER` | | `trace-commons-admin` | 12 | Single `TRACE_COMMONS_ADMIN_BEARER` | | `trace-commons-worker` | 8 | Per-subcommand defaults | | `trace-commons-tenant` | 11 | Single `TRACE_COMMONS_TENANT_BEARER` | | **Total** | **39** | | These mirror the 39 operator commands removed from Ironclaw in nearai/ironclaw#3738. ## Verification ``` cargo check --workspace --all-targets (-D warnings) # clean cargo test --workspace --no-run (-D warnings) # clean cargo clippy --workspace --all-targets (-D warnings) # clean cargo test -p trace-commons-operator-client # 39 passed cargo test -p trace-commons-server --bins # all green bash scripts/operator/operator-smoke.sh # SmokeOperatorBinariesOK ``` Superseded PRs (closed): #124, #125, #126.
Three CI failures on PR #3738: 1. cargo fmt --check diff in 15 spots across the traces module and tests. Ran cargo fmt --all to canonicalize. 2. reborn_crate_dependency_boundaries_hold + reborn_cli_binary_crate_stays_ separate_from_v1_root failed because the trace-client extraction added ironclaw_llm and ironclaw_common as direct deps of ironclaw_reborn_cli (the preview handler deserializes ironclaw_llm::recording::TraceFile, and the contribution dir resolver called ironclaw_common::paths::ironclaw_ base_dir). The architectural rule is that ironclaw_reborn_cli enters Reborn through the doorway crates only. Resolution: re-export both surfaces through ironclaw_reborn_traces: - ironclaw_reborn_traces::recording::* re-exports ironclaw_llm::recording - ironclaw_reborn_traces::paths::* re-exports ironclaw_common::paths Drop ironclaw_llm and ironclaw_common as direct deps of ironclaw_reborn_ cli. Add ironclaw_reborn_traces to the allowed-doorway set in the boundary test, with a doc comment explaining why (traces crate is the contributor-side TraceCommons client, analogous to composition + config). Final allowed deps of ironclaw_reborn_cli: ironclaw_reborn_composition, ironclaw_reborn_config, ironclaw_reborn_traces. No transitive leakage of ironclaw_llm / ironclaw_common into the binary's import surface. Verification: - RUSTFLAGS="-D warnings" cargo check --workspace --all-targets: clean - cargo fmt --all -- --check: clean - cargo test -p ironclaw_architecture --test reborn_dependency_boundaries: 19 passed - cargo test -p ironclaw_reborn_cli: 58 passed (invite-code tests included) - cargo clippy --workspace --all-targets -- -D warnings: clean
- Enum-ify PilotAllowlist refusal labels (Medium-4). - Hash invite code in upload-claim cache key with sha256: prefix (Low-2). - show_policy_status treats whitespace-only invite code as not configured (Low-1). - Write policy.json with 0o600 permissions on unix (Medium-1). - Add caller-level tests for PilotAllowlist typed and generic error paths (High-1, High-2). - Add opt_in invite-code persistence test (Medium-2). - Add queue-status diagnostics invite-code-configured test (Medium-3). - Add parse_trace_upload_claim_error_label non-string coverage (Low-3).
ecb04b4 to
88ff4f8
Compare
|
@henrypark133 friendly ping — all 9 findings from your review are addressed in ecb04b4 and consolidated in 88ff4f8 (per-comment replies posted). cargo fmt/clippy/tests all green. Could you take another pass when you have a moment? Thanks! |
Three CI failures on PR #3738: 1. cargo fmt --check diff in 15 spots across the traces module and tests. Ran cargo fmt --all to canonicalize. 2. reborn_crate_dependency_boundaries_hold + reborn_cli_binary_crate_stays_ separate_from_v1_root failed because the trace-client extraction added ironclaw_llm and ironclaw_common as direct deps of ironclaw_reborn_cli (the preview handler deserializes ironclaw_llm::recording::TraceFile, and the contribution dir resolver called ironclaw_common::paths::ironclaw_ base_dir). The architectural rule is that ironclaw_reborn_cli enters Reborn through the doorway crates only. Resolution: re-export both surfaces through ironclaw_reborn_traces: - ironclaw_reborn_traces::recording::* re-exports ironclaw_llm::recording - ironclaw_reborn_traces::paths::* re-exports ironclaw_common::paths Drop ironclaw_llm and ironclaw_common as direct deps of ironclaw_reborn_ cli. Add ironclaw_reborn_traces to the allowed-doorway set in the boundary test, with a doc comment explaining why (traces crate is the contributor-side TraceCommons client, analogous to composition + config). Final allowed deps of ironclaw_reborn_cli: ironclaw_reborn_composition, ironclaw_reborn_config, ironclaw_reborn_traces. No transitive leakage of ironclaw_llm / ironclaw_common into the binary's import surface. Verification: - RUSTFLAGS="-D warnings" cargo check --workspace --all-targets: clean - cargo fmt --all -- --check: clean - cargo test -p ironclaw_architecture --test reborn_dependency_boundaries: 19 passed - cargo test -p ironclaw_reborn_cli: 58 passed (invite-code tests included) - cargo clippy --workspace --all-targets -- -D warnings: clean
- Enum-ify PilotAllowlist refusal labels (Medium-4). - Hash invite code in upload-claim cache key with sha256: prefix (Low-2). - show_policy_status treats whitespace-only invite code as not configured (Low-1). - Write policy.json with 0o600 permissions on unix (Medium-1). - Add caller-level tests for PilotAllowlist typed and generic error paths (High-1, High-2). - Add opt_in invite-code persistence test (Medium-2). - Add queue-status diagnostics invite-code-configured test (Medium-3). - Add parse_trace_upload_claim_error_label non-string coverage (Low-3).
88ff4f8 to
7f989b7
Compare
henrypark133
left a comment
henrypark133
left a comment
There was a problem hiding this comment.
Single-agent review.
Verdict: APPROVE with follow-up
Approving with follow-up: these Medium/Low items should be addressed in a follow-up PR (not blocking).
The extraction is clean and well-executed: 11,839 lines of src/trace_contribution.rs + 542 lines of src/trace_client.rs are removed from the monolith and re-homed in the new ironclaw_reborn_traces crate. Re-export shims (pub use ironclaw_reborn_traces::...) maintain backward compatibility for the monolith. Architecture boundary tests are updated to lock the dependency surface. No critical or high-confidence findings.
Findings (0 High · 2 Medium · 1 Low)
[Medium] TraceCreditEventKind missing #[serde(rename_all = "snake_case")]
File: crates/ironclaw_reborn_traces/src/contribution.rs (lines 583–598)
TraceCreditEventKind derives Serialize/Deserialize but is the only wire-persisted enum in the file without #[serde(rename_all = "snake_case")]. Every other enum in this file (e.g. TraceChannel, ConsentScope, ResidualPiiRisk, UserFeedback, TaskSuccess, SideEffectLevel, TraceContributionEventType, TraceFailureMode, TraceAllowedUse, TraceRetentionClass, etc.) has the attribute. Without it, variants serialize as PascalCase ("Accepted", "CreditSynced", "UsedForTrainingOrRanking") rather than snake_case ("accepted", "credit_synced", "used_for_training_or_ranking"). This enum is persisted in local credit_events records on disk and round-tripped against server responses — a format mismatch will silently break deserialization of existing local records or server-pushed event syncs once a producer and consumer disagree on case. The .claude/rules/types.md rule explicitly calls this out as a wire-stability requirement. Recommend adding #[serde(rename_all = "snake_case")] (with #[serde(alias = "...")] annotations if any on-disk records already exist with PascalCase values).
Confidence: 88%
[Medium] docs/internal/trace-commons.md still shows ironclaw traces (monolith CLI) after command migration
File: docs/internal/trace-commons.md (line 40 onward)
The PR removes the traces subcommand from the monolith (src/cli/mod.rs) and adds it to the standalone Reborn CLI binary (ironclaw-reborn). The doc examples throughout docs/internal/trace-commons.md still use ironclaw traces opt-in, ironclaw traces preview, etc. These will fail for any reader attempting to follow the examples — the correct command is now ironclaw-reborn traces opt-in. The .claude/rules/review-discipline.md move-only rule explicitly requires updating .md references to moved paths in the same PR. The doc update was not included here.
Confidence: 92%
[Low] shared.rs uses #![allow(unused_imports)] to suppress a live warning
File: crates/ironclaw_reborn_cli/src/commands/traces/shared.rs
The file exists as a forward-compat re-export layer (pub(super) use super::{...}) to document the shared surface and is otherwise empty of behavior. The #![allow(unused_imports)] suppresses an active warning because the audience modules use super::* and don't import from shared directly. This is a minor convention issue — the project's CLAUDE.md policy is zero clippy warnings. Consider either removing the file until it has distinct callers, or documenting the exemption with an arch-exempt: ... annotation per .claude/rules/architecture.md style. Not blocking.
Confidence: 72%
Positive observations
- Extraction is structurally clean. The re-export shims (
src/trace_client.rs,src/trace_contribution.rs,src/tools/redaction.rs) preserve backward-compat with zero call-site churn in the monolith. - Security controls intact. JWT validation (
validate_trace_upload_claim_response), SSRF guards (validate_trace_upload_claim_issuer_url,resolve_trace_upload_claim_issuer_host), redirect blocking, and bounded response reading are all present and correct in the new crate. - Architecture boundary test updated.
reborn_cli_binary_crate_stays_separate_from_v1_rootnow assertsironclaw_reborn_tracesas an allowed edge in the strict allowlist, and the new crate's registration inworkspace.membersis validated byreborn_boundary_rules_active_crates_are_workspace_members. - Privacy sidecar process isolation. Subprocess spawning clears the environment except
PATH/LANG/LC_ALL, uses bounded stdout/stderr reads, and applies a configurable timeout — good defense-in-depth for the optional privacy filter. - Test coverage. The move carries the full test suite into the new crate (
client.rsunit tests,tests.rsCLI parse + queue preflight tests). New invite-code tests specifically cover round-trip serde persistence ofupload_token_invite_code. - Consent flag enforcement.
preflight_cli_trace_envelope_uploadchecks that the envelope'smessage_text_included/tool_payloads_includedflags are consistent with the standing policy before any disk write, and tests confirm rejection-before-write.
Review produced by single-agent code-review-skill:v1. Lenses applied: security, bugs, performance, tests, conventions, design.
Move trace_contribution, trace_client, and tools/redaction into a new workspace crate so the standalone ironclaw_reborn_cli can consume them without depending on the monolith. The legacy src/trace_*.rs and src/tools/redaction.rs files remain as thin re-export shims so all existing call sites in the monolith continue to compile unchanged. ConversationMessage is now defined in the new crate; src/history/store.rs re-exports it so crate::history::ConversationMessage and the new ironclaw_reborn_traces::ConversationMessage are the same type. The single crate::bootstrap::ironclaw_base_dir() call inside contribution.rs is rewritten to call ironclaw_common::paths::ironclaw_base_dir directly.
Wire ironclaw_reborn_traces into the standalone reborn CLI and expose a minimal 'traces' subcommand with opt-in, status, and queue-status. The opt-in path prints a hand-off message pointing at the legacy ironclaw binary; status and queue-status call into ironclaw_reborn_traces using the anonymous scope so the wiring is exercised at compile time. The full TraceCommons CLI port lands separately.
Companion to TraceCommons/trace-commons#109 — the trace-commons server-side invite-code allowlist landed there with refusal labels PilotAllowlist* (NotMatched 403, InviteCodeMissing 400, Stale 503, Malformed 503). This commit teaches the Ironclaw trace client three things: 1. Carry the invite code through the standing policy. - StandingTraceContributionPolicy gains upload_token_invite_code: Option<String>, serde-default + skip-if- none so existing policy files keep parsing byte-identical. - TraceUploadClaimIssuerRequest body gains an optional invite_code field (forward-compat for a future server slice that may read it from the body in addition to the workload-JWT claim). - trace_upload_claim_cache_key keys on the invite code so a mid- pilot rotation forces a fresh claim mint. 2. Surface the typed allowlist refusal labels. fetch_trace_upload_claim_from_issuer now parses {"error": "<Label>"} bodies on non-success status and maps each PilotAllowlist* label to a user-actionable diagnostic. 3. CLI flag + status surface. ironclaw traces opt-in --upload-token-invite-code <CODE> (off by default). ironclaw traces status and queue-status report "pilot invite code: configured / not configured" — never echo the raw code. After the trace-client extraction (5e568fc + 23104c9), the trace contribution code lives in crates/ironclaw_reborn_traces. The invite- code wiring lands on the extracted paths. Seven new unit tests, all green: - standing_policy_serde_back_compat_when_invite_code_missing - standing_policy_serde_round_trips_invite_code_when_set - standing_policy_serde_omits_invite_code_when_none - cache_key_distinguishes_different_invite_codes - parse_trace_upload_claim_error_label_handles_known_shapes - opt_in_invite_code_flag_parses_through_cli - opt_in_invite_code_defaults_to_none_when_absent
Replaces the stub committed in 23104c9 with the full 50-subcommand implementation moved wholesale from src/cli/traces.rs. The monolith's ironclaw binary loses its traces subcommand; ironclaw-reborn traces ... becomes the only path. The library code remains in ironclaw_reborn_traces. Breaking CLI change: users invoking `ironclaw traces ...` must switch to `ironclaw-reborn traces ...`. Operator runbooks (pilot bootstrap, HF cache hygiene, GPU cost ledger) reference the legacy binary name and will need updating in a follow-up doc PR.
Architectural pivot: Ironclaw's trace CLI holds only contributor-facing commands (consent, upload, credit display, monitoring). Operator commands (reviewer, worker, admin, tenant, audit) move out of Ironclaw entirely — they'll re-emerge as separate binaries in the trace-commons- server repo in a follow-up PR. Deleted 39 variants from TracesSubcommand and everything supporting them: handler fns, option structs, clap value enums, audience modules (reviewer.rs/worker.rs/admin.rs/tenant.rs), and tests. Kept 12 contributor variants: OptIn, OptOut, Status, Preview, Enqueue, FlushQueue, QueueStatus, Credit, Submit, ListSubmissions, Revoke, IngestHealth. All 7 invite-code tests continue to pass.
Three CI failures on PR #3738: 1. cargo fmt --check diff in 15 spots across the traces module and tests. Ran cargo fmt --all to canonicalize. 2. reborn_crate_dependency_boundaries_hold + reborn_cli_binary_crate_stays_ separate_from_v1_root failed because the trace-client extraction added ironclaw_llm and ironclaw_common as direct deps of ironclaw_reborn_cli (the preview handler deserializes ironclaw_llm::recording::TraceFile, and the contribution dir resolver called ironclaw_common::paths::ironclaw_ base_dir). The architectural rule is that ironclaw_reborn_cli enters Reborn through the doorway crates only. Resolution: re-export both surfaces through ironclaw_reborn_traces: - ironclaw_reborn_traces::recording::* re-exports ironclaw_llm::recording - ironclaw_reborn_traces::paths::* re-exports ironclaw_common::paths Drop ironclaw_llm and ironclaw_common as direct deps of ironclaw_reborn_ cli. Add ironclaw_reborn_traces to the allowed-doorway set in the boundary test, with a doc comment explaining why (traces crate is the contributor-side TraceCommons client, analogous to composition + config). Final allowed deps of ironclaw_reborn_cli: ironclaw_reborn_composition, ironclaw_reborn_config, ironclaw_reborn_traces. No transitive leakage of ironclaw_llm / ironclaw_common into the binary's import surface. Verification: - RUSTFLAGS="-D warnings" cargo check --workspace --all-targets: clean - cargo fmt --all -- --check: clean - cargo test -p ironclaw_architecture --test reborn_dependency_boundaries: 19 passed - cargo test -p ironclaw_reborn_cli: 58 passed (invite-code tests included) - cargo clippy --workspace --all-targets -- -D warnings: clean
- Enum-ify PilotAllowlist refusal labels (Medium-4). - Hash invite code in upload-claim cache key with sha256: prefix (Low-2). - show_policy_status treats whitespace-only invite code as not configured (Low-1). - Write policy.json with 0o600 permissions on unix (Medium-1). - Add caller-level tests for PilotAllowlist typed and generic error paths (High-1, High-2). - Add opt_in invite-code persistence test (Medium-2). - Add queue-status diagnostics invite-code-configured test (Medium-3). - Add parse_trace_upload_claim_error_label non-string coverage (Low-3).
7f989b7 to
3632415
Compare
… CLI (nearai#3738) * extract trace client into ironclaw_reborn_traces crate Move trace_contribution, trace_client, and tools/redaction into a new workspace crate so the standalone ironclaw_reborn_cli can consume them without depending on the monolith. The legacy src/trace_*.rs and src/tools/redaction.rs files remain as thin re-export shims so all existing call sites in the monolith continue to compile unchanged. ConversationMessage is now defined in the new crate; src/history/store.rs re-exports it so crate::history::ConversationMessage and the new ironclaw_reborn_traces::ConversationMessage are the same type. The single crate::bootstrap::ironclaw_base_dir() call inside contribution.rs is rewritten to call ironclaw_common::paths::ironclaw_base_dir directly. * add traces subcommand stub to ironclaw_reborn_cli Wire ironclaw_reborn_traces into the standalone reborn CLI and expose a minimal 'traces' subcommand with opt-in, status, and queue-status. The opt-in path prints a hand-off message pointing at the legacy ironclaw binary; status and queue-status call into ironclaw_reborn_traces using the anonymous scope so the wiring is exercised at compile time. The full TraceCommons CLI port lands separately. * wire pilot invite_code through upload-claim refresh Companion to TraceCommons/trace-commons#109 — the trace-commons server-side invite-code allowlist landed there with refusal labels PilotAllowlist* (NotMatched 403, InviteCodeMissing 400, Stale 503, Malformed 503). This commit teaches the Ironclaw trace client three things: 1. Carry the invite code through the standing policy. - StandingTraceContributionPolicy gains upload_token_invite_code: Option<String>, serde-default + skip-if- none so existing policy files keep parsing byte-identical. - TraceUploadClaimIssuerRequest body gains an optional invite_code field (forward-compat for a future server slice that may read it from the body in addition to the workload-JWT claim). - trace_upload_claim_cache_key keys on the invite code so a mid- pilot rotation forces a fresh claim mint. 2. Surface the typed allowlist refusal labels. fetch_trace_upload_claim_from_issuer now parses {"error": "<Label>"} bodies on non-success status and maps each PilotAllowlist* label to a user-actionable diagnostic. 3. CLI flag + status surface. ironclaw traces opt-in --upload-token-invite-code <CODE> (off by default). ironclaw traces status and queue-status report "pilot invite code: configured / not configured" — never echo the raw code. After the trace-client extraction (5e568fc + 23104c9), the trace contribution code lives in crates/ironclaw_reborn_traces. The invite- code wiring lands on the extracted paths. Seven new unit tests, all green: - standing_policy_serde_back_compat_when_invite_code_missing - standing_policy_serde_round_trips_invite_code_when_set - standing_policy_serde_omits_invite_code_when_none - cache_key_distinguishes_different_invite_codes - parse_trace_upload_claim_error_label_handles_known_shapes - opt_in_invite_code_flag_parses_through_cli - opt_in_invite_code_defaults_to_none_when_absent * migrate ironclaw traces CLI surface into ironclaw_reborn_cli Replaces the stub committed in 23104c9 with the full 50-subcommand implementation moved wholesale from src/cli/traces.rs. The monolith's ironclaw binary loses its traces subcommand; ironclaw-reborn traces ... becomes the only path. The library code remains in ironclaw_reborn_traces. Breaking CLI change: users invoking `ironclaw traces ...` must switch to `ironclaw-reborn traces ...`. Operator runbooks (pilot bootstrap, HF cache hygiene, GPU cost ledger) reference the legacy binary name and will need updating in a follow-up doc PR. * split traces CLI by audience into module tree * extract trace CLI tests into sibling tests.rs * prune operator commands from Ironclaw trace CLI Architectural pivot: Ironclaw's trace CLI holds only contributor-facing commands (consent, upload, credit display, monitoring). Operator commands (reviewer, worker, admin, tenant, audit) move out of Ironclaw entirely — they'll re-emerge as separate binaries in the trace-commons- server repo in a follow-up PR. Deleted 39 variants from TracesSubcommand and everything supporting them: handler fns, option structs, clap value enums, audience modules (reviewer.rs/worker.rs/admin.rs/tenant.rs), and tests. Kept 12 contributor variants: OptIn, OptOut, Status, Preview, Enqueue, FlushQueue, QueueStatus, Credit, Submit, ListSubmissions, Revoke, IngestHealth. All 7 invite-code tests continue to pass. * fix CI: cargo fmt + restore reborn-cli dep boundary Three CI failures on PR nearai#3738: 1. cargo fmt --check diff in 15 spots across the traces module and tests. Ran cargo fmt --all to canonicalize. 2. reborn_crate_dependency_boundaries_hold + reborn_cli_binary_crate_stays_ separate_from_v1_root failed because the trace-client extraction added ironclaw_llm and ironclaw_common as direct deps of ironclaw_reborn_cli (the preview handler deserializes ironclaw_llm::recording::TraceFile, and the contribution dir resolver called ironclaw_common::paths::ironclaw_ base_dir). The architectural rule is that ironclaw_reborn_cli enters Reborn through the doorway crates only. Resolution: re-export both surfaces through ironclaw_reborn_traces: - ironclaw_reborn_traces::recording::* re-exports ironclaw_llm::recording - ironclaw_reborn_traces::paths::* re-exports ironclaw_common::paths Drop ironclaw_llm and ironclaw_common as direct deps of ironclaw_reborn_ cli. Add ironclaw_reborn_traces to the allowed-doorway set in the boundary test, with a doc comment explaining why (traces crate is the contributor-side TraceCommons client, analogous to composition + config). Final allowed deps of ironclaw_reborn_cli: ironclaw_reborn_composition, ironclaw_reborn_config, ironclaw_reborn_traces. No transitive leakage of ironclaw_llm / ironclaw_common into the binary's import surface. Verification: - RUSTFLAGS="-D warnings" cargo check --workspace --all-targets: clean - cargo fmt --all -- --check: clean - cargo test -p ironclaw_architecture --test reborn_dependency_boundaries: 19 passed - cargo test -p ironclaw_reborn_cli: 58 passed (invite-code tests included) - cargo clippy --workspace --all-targets -- -D warnings: clean * address PR nearai#3738 review feedback - Enum-ify PilotAllowlist refusal labels (Medium-4). - Hash invite code in upload-claim cache key with sha256: prefix (Low-2). - show_policy_status treats whitespace-only invite code as not configured (Low-1). - Write policy.json with 0o600 permissions on unix (Medium-1). - Add caller-level tests for PilotAllowlist typed and generic error paths (High-1, High-2). - Add opt_in invite-code persistence test (Medium-2). - Add queue-status diagnostics invite-code-configured test (Medium-3). - Add parse_trace_upload_claim_error_label non-string coverage (Low-3).
2 participants
Context
Completes the migration of the Ironclaw-side trace client into the reborn system, lands the pilot invite-code wiring, and prunes Ironclaw's trace CLI to contributor-only. Operator/admin/worker/tenant commands move out of Ironclaw entirely — they re-emerge as separate binaries in the trace-commons-server repo (companion stack linked below).
Companion stack — operator binaries in trace-commons-server
The 39 operator commands pruned here re-appear in trace-commons-server:
trace-commons-operator-clientfoundation crate (shared HTTP transport, error taxonomy, host allowlist, output formatters).trace-commons-review(8 cmds, single bearer) +trace-commons-admin(12 cmds, single bearer). Stacked on chore: release v0.5.0 #124.trace-commons-worker(8 cmds, per-route bearer) +trace-commons-tenant(11 cmds, single bearer) + operator runbook + CI smoke. Stacked on fix: sentinel value collision in FailoverProvider cooldown #125.This PR (Ironclaw) and the companion stack can land independently — Ironclaw's CLI surface narrows, trace-commons-server's CLI surface broadens, and no behavior breaks because the underlying HTTP API is unchanged.
Commits (7)
5e568fcb5extract trace client into ironclaw_reborn_traces crate — movessrc/trace_contribution.rs(12k LOC) +src/trace_client.rs+src/tools/redaction.rsinto a new workspace cratecrates/ironclaw_reborn_traces.ConversationMessagelives in the new crate and is re-exported fromsrc/history/store.rsso monolith callers are unaffected. 3-linepub useshims at the old monolith paths.23104c9faadd traces subcommand stub to ironclaw_reborn_cli — clap surface stub. Replaced by commit 4.930e82d4fwire pilot invite_code through upload-claim refresh — the original Migrate trace client to ironclaw_reborn and prune to contributor-only CLI #3738 scope, on the extracted paths.3b5ca1fdemigrate ironclaw traces CLI surface into ironclaw_reborn_cli — wholesale port of the legacysrc/cli/traces.rs(50 subcommands, 9,873 LOC). Monolith'sironclaw tracesis gone;ironclaw-reborn traces ...is the only path.620ce8fc5split traces CLI by audience into module tree —commands/traces/{mod,contributor,reviewer,worker,admin,tenant,shared}.rs.24ff0c73bextract trace CLI tests into sibling tests.rs — 2,800-line test module pulled out via#[path](same pattern astracedao-ingest.rs).5494fa00fprune operator commands from Ironclaw trace CLI — deletes 39 operator variants (reviewer 8, worker 8, admin 12, tenant+ranker+audit 11) and everything supporting them. Keeps 12 contributor variants.Architectural cut
After this PR, Ironclaw's
ironclaw-reborn tracesexposes only contributor-side concerns:opt-inopt-outstatuspreviewenqueueflush-queuequeue-statuscreditsubmitlist-submissionsrevokeingest-health/healthOperator workflows (reviewer triage, worker routes, admin/retention/export, tenant policy) live in the four trace-commons-server binaries linked above, talking to the same HTTP API.
Breaking CLI change
Users invoking
ironclaw traces ...must switch toironclaw-reborn traces .... Operator commands previously available viaironclaw traces(quarantine-list, review-decision, maintenance-run, tenant-policy-*, etc.) are no longer available from Ironclaw at all — they move to trace-commons-server operator binaries.Invite-code wiring
Companion to TraceCommons/trace-commons-server#109. The issuer returns four typed refusal labels (
PilotAllowlistNotMatched403,PilotAllowlistInviteCodeMissing400,PilotAllowlistStale503,PilotAllowlistMalformed503). This PR:StandingTraceContributionPolicy.upload_token_invite_code: Option<String>(serde-default + skip-if-none).TraceUploadClaimIssuerRequestbody field.trace_upload_claim_cache_keykeys on the invite code so rotation forces a fresh mint.fetch_trace_upload_claim_from_issuerparses{"error": "<Label>"}and maps eachPilotAllowlist*to a user-actionable diagnostic.ironclaw-reborn traces opt-in --upload-token-invite-code <CODE>(off by default).statusandqueue-statusreport"pilot invite code: configured / not configured"— never echo the raw code.Tests
All 7 invite-code unit tests pass, plus the broader contributor test suite (58 tests in reborn-cli):
standing_policy_serde_back_compat_when_invite_code_missingstanding_policy_serde_round_trips_invite_code_when_setstanding_policy_serde_omits_invite_code_when_nonecache_key_distinguishes_different_invite_codesparse_trace_upload_claim_error_label_handles_known_shapes(5 inironclaw_reborn_traces)opt_in_invite_code_flag_parses_through_cliopt_in_invite_code_defaults_to_none_when_absent(2 inironclaw_reborn_cli, parse-through against the rebornCli)Verification
Net diff vs
origin/reborn-integration27 files changed, +15,564 / −22,443 — a net −6,879 LOC in Ironclaw thanks to the prune.
commands/traces/mod.rsis 1,838 lines;contributor.rsis 112 lines;tests.rsis 619 lines.Compatibility
crate::trace_contribution::*andcrate::trace_client::*keep building via re-export shims.crate::history::ConversationMessagecontinues to resolve (re-exported fromironclaw_reborn_traces).Out of scope
ironclaw traces ...→ironclaw-reborn traces ...for the contributor surface.ironclaw_reborn_traceslibrary crate that no Ironclaw consumer references anymore — leaving them in place so trace-commons-server operator binaries can consume them via path dep / vendoring (currently the operator stack ports the wire shapes rather than depending on the Ironclaw crate, but this leaves the door open).Settings/profile resolution on the reborn-cli side (currently uses the trimmedIRONCLAW_OWNER_IDenv var path).trace-commons-tenant privacy-filter-canary; in-progress as a follow-up to trace-commons-server feat: 10 infrastructure improvements from zeroclaw #126).