Trace Commons: instance-wide enrollment, per-user profiles, and trace inspection - #5280
Merged
Merged
Conversation
… inspection Cross-repo design (ironclaw + trace-commons-server) for three coexisting capabilities: instance-wide enrollment, per-user contributor accounts via login-links, and submitted-trace inspection. Introduces a trace-credential resolver so the existing user-invite model and the new instance-wide model both function on one instance, with personal-invite enrollment taking precedence. Server change is additive (optional per-user subject through claim issuance + login-link + account resolution). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TDD plan for the one server change the whole effort depends on: accept an optional opaque subject in the upload-claim request and derive a per-user, tenant-namespaced principal at device-key issuance. Submission attribution, login-link account resolution, and trace readback all become per-user automatically from the shared bearer principal; absent subject reproduces today's behavior. Targets trace-commons-server (contributor-account-slice1). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Slice 1: trace-credential resolver (personal-invite wins, instance fallback with per-user subject) + admin-gated instance enrollment. Slice 2: per-user subject plumbing through upload-claim request + submission. Slice 3: trace_commons.account_login_link first-party capability (profiles). Slice 4: per-user submitted-trace inspection across reborn_traces → product_workflow facade → webui_v2 handler → frontend. Each plan is bite-sized TDD against verbatim-extracted current code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ce fallback w/ subject)
…esolver duplication) Extract `trace_contribution_dir_for_scope_at`, `trace_policy_path_at`, `read_trace_policy_for_scope_at`, and `write_trace_policy_for_scope_at` as the canonical base-dir-parameterized path helpers. All public functions (`trace_contribution_dir_for_scope`, `read_trace_policy_for_scope`, `write_trace_policy_for_scope`) now delegate to the `_at` variants with `ironclaw_base_dir()` — signatures unchanged. The inline `read_policy` closure in `resolve_trace_credentials_at` that re-implemented path layout is deleted; it now calls `read_trace_policy_for_scope_at` directly. The test `write_policy_at` helper's bespoke path construction is replaced with a call to `write_trace_policy_for_scope_at`. The now-dead `trace_policy_path` function is removed. Path layout is encoded in exactly one place. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…bal base)
Rework `instance_onboard_writes_instance_level_policy` to operate entirely
under a `tempfile::tempdir()`:
- Compute instance_dir as base.path().join("trace_contributions") (scope=None
layout, no users/<hash> segment) rather than calling the global LazyLock.
- Call `onboard_at_dir_with_sink` directly against the tempdir so the test
never touches the real ~/.ironclaw tree.
- Assert policy.json by reading and deserializing it from the tempdir.
- Remove all manual std::fs::remove_* cleanup lines; tempdir drops automatically.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nstance enrollment) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n-links)
Add `mint_account_login_link_via_sink` to ironclaw_reborn_traces:
- `TraceUploadClaimContext::for_account(subject)` constructor for
account-management call contexts (no trace/submission ids, no
consent scopes).
- `AccountLoginLink { account_id, url }` return type.
- `account_login_links_url(policy)` helper that derives the login-links
URL from the upload-claim issuer URL (strip /v1/trace-upload-claim,
append /v1/account/login-links).
- `mint_account_login_link_inner(base_dir, ...)` private dir-parameterised
core: resolves credentials, selects correct scope_dir for DeviceKey
auth (instance enrollment → instance scope dir; personal → user scope
dir), mints bearer, POSTs subject, parses response.
- `mint_account_login_link_via_sink(tenant_id, user_id, sink)` public
entry point wrapping the inner function with the real base dir.
Tests (hermetic, tempdir-isolated):
- `mint_account_login_link_posts_subject_and_returns_url`: verifies the
posted subject equals `local_pseudonymous_contributor_id(trace_scope_key(...))`
for instance-enrolled users via an axum mock serving both the
upload-claim issuer and the login-links endpoint.
- `mint_account_login_link_errors_when_not_enrolled`: verifies error path.
- `ReqwestContributionSink` test helper added to the test module.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the unwrap_or_else fallback (which silently used the full issuer URL as a base when the /v1/trace-upload-claim suffix was absent) with an explicit anyhow error. Add two unit tests: one asserting an Err on a wrong-suffix URL, one asserting the correct .../v1/account/login-links URL on a valid issuer. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…k capability Mints a Trace Commons browser login URL via host network egress, mirroring dispatch_profile_token. Includes consent gate, enrollment pre-check, HostEgressContributionSink routing, and two e2e tests. Also fixes a sanitizer bug: validate_runtime_request was rejecting authorization headers on all requests, including RuntimeKind::FirstParty. FirstParty requests are host-internal and trusted to carry bearer tokens; the sensitive-header and manual-credentials guards now only apply to untrusted plugin runtimes (WASM/MCP/Script). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…re FirstParty sensitive-header guard Commit 9e25d99 blanket-exempted all RuntimeKind::FirstParty requests from the egress sensitive-header and manual-credentials guards so the host-minted Trace Commons bearer could pass. builtin.http is also FirstParty but forwards model-supplied headers, so this let the model smuggle Authorization/Cookie/ x-api-key headers (or user:pass@ URLs) to allowlisted hosts. Revert the sanitize.rs exemption (guards now apply to ALL runtimes again) and deliver the trace bearer through the staged credential-injection path instead: the HostEgressContributionSink stages the minted token one-shot via RuntimeSecretMaterialStager and declares a StagedObligation Authorization-header injection, mirroring the SlackProtocolHttpEgress pattern. The stager is now exposed to first-party handlers via InvocationServices. Covers the profile_token, profile_set/community-profile, and account_login_link bearer paths. Regression tests: FirstParty + raw authorization header -> denied; FirstParty + user:pass@ URL -> denied. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…per-user) - Add ContributionHttpMethod::Get variant; update all exhaustive match sites in ironclaw_reborn_traces and HostEgressContributionSink in ironclaw_host_runtime. - Extract account_api_base_url() shared helper; account_login_links_url and new account_traces_url both delegate to it (DRY). - Add AccountTraceItem (Debug, Clone, Serialize, Deserialize; serde defaults). - Add fetch_account_traces_via_sink / fetch_account_traces_inner mirroring mint_account_login_link pattern: unenrolled -> Ok(vec![]), non-2xx -> Ok(vec![]), transport error -> Err. - Tests: hermetic axum mock (GET /v1/account/traces), unenrolled empty-list, URL shape with/without limit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds RebornAccountTrace / RebornAccountTracesResponse wire types and a trace_account_traces default method on RebornServicesApi, mirroring the trace_credits egress pattern (crate-local hardened reqwest, no host-egress sink). Also adds fetch_account_traces (direct path) to ironclaw_reborn_traces::contribution so the facade can fetch server traces without coupling to RuntimeHttpEgress. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…s contract test, annotate sink scaffold Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…contribute) The autonomous trace-flush gate read only the per-scope (personal-invite) policy and aborted when it was disabled, so instance-enrolled users (whose enrollment lives at scope None) could never contribute traces — and the per-user scope_dir would also fail to load the instance device key. Introduce a single EffectiveFlushTarget resolver (resolve_effective_flush_target, mirroring resolve_trace_credentials but keyed on the already-composed scope string) that returns the policy, device-key dir, and per-user subject in one policy-read/path pass. The flush gate now proceeds for instance-only enrollment, loads the device key from the instance (None) dir, and attributes uploads via the per-user pseudonymous subject. The redundant subject_for_scope helper (which re-read the same policies with silent .ok() error swallowing) is removed and its logic folded into the new helper with proper error propagation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⛔ Files ignored due to path filters (1)
⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughTrace Commons now resolves enrollment through a shared credential resolver, stages bearer credentials for network egress, adds a consent-gated account-login-link capability, exposes account-trace reads through WebUI v2, and renders submitted traces in settings with new tests and docs. ChangesTrace Commons rollout
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant HostRuntime
participant trace_commons
participant ContributionHttpSink
participant AccountLoginLinkService
participant LocalFilesystem
HostRuntime->>trace_commons: dispatch_account_login_link(confirmed=true)
trace_commons->>trace_commons: resolve_trace_credentials()
trace_commons->>ContributionHttpSink: mint_account_login_link_via_sink(...)
ContributionHttpSink->>AccountLoginLinkService: POST /v1/account/login-links
AccountLoginLinkService-->>trace_commons: { account_id, url }
trace_commons->>LocalFilesystem: persist account_login_link.url
trace_commons-->>HostRuntime: { account_id, link_delivery }
sequenceDiagram
participant Browser
participant WebUIv2Router
participant WebUiV2Handlers
participant RebornServicesApi
Browser->>WebUIv2Router: GET /api/webchat/v2/traces/account
WebUIv2Router->>WebUiV2Handlers: trace_account_traces(...)
WebUiV2Handlers->>RebornServicesApi: trace_account_traces(caller)
RebornServicesApi-->>WebUiV2Handlers: RebornAccountTracesResponse
WebUiV2Handlers-->>Browser: Json<RebornAccountTracesResponse>
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
scope: agent
…h tests - Explicit per-user opt-out (scoped policy present with enabled=false, as written by 'traces opt-out') now blocks the instance-enrollment fallback in resolve_trace_credentials and resolve_effective_flush_target (and thus capture) — only a never-configured scope falls through to the instance policy. Regression test covers all three resolution surfaces. - Instance-enrollment subjects are now salted: a per-instance random salt (persisted 0600 at the instance trace dir, create_new race-safe) feeds sha256(salt:scope), so the server or ledger holders cannot dictionary-match guessable tenant/user ids against an unsalted scope hash. Unsalted local_pseudonymous_contributor_id remains for local state keying/log refs. - contribution.rs carries the architecture-rule file-size justification referencing decomposition tracking issue #4088; state_scope field docs now say which state it does (and does not) locate. - Submitted-traces UI: extracted the pure tracesSectionMode decision (error wins over list; list needs enrolled + non-empty) and covered it plus the row formatters in trace-commons-tab.test.mjs. - Docs: slice4 plan points at crates/ironclaw_webui_v2 (static crate was folded in), slice3 signature snippet matches the typed contract, and the webui_v2 CLAUDE.md route table gains the three trace routes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Round-2 review feedback addressed in ee46317 — all seven inline threads replied to:
Verified: full-workspace clippy (zero warnings), |
Collaborator
Author
|
@IronLoop review --agent reviewer |
henrypark133
previously approved these changes
Jul 6, 2026
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Verdict: ✅ Approved
Findings: 0 blocking / 0 notes
Next: No reviewer action needed.
Head: ee463174263dfdc1d3c0df9171e42b5b7a3545a2
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete blocking issues found in the PR diff. The changes add Trace Commons account-login/account-trace plumbing with caller-scoped routing, host-egress credential injection for agent paths, bounded account trace fetches, and WebUI coverage.
Findings
None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloop review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloop reviewwhen the fix may affect multiple areas. - Use
@ironloop statusto check queued/running/completed/stale/stalled state while reviewers run.
ironloopai
Bot
dismissed
their stale review
July 6, 2026 22:57
Superseded by a later IronLoop approved review for this reviewer.
Contributor
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Verdict: ✅ Approved
Findings: 0 blocking / 0 notes
Next: No reviewer action needed.
Head: ee463174263dfdc1d3c0df9171e42b5b7a3545a2
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete blocking issues found in the PR diff. The changes add Trace Commons account-login/account-trace plumbing with caller-scoped routing, host-egress credential injection for agent paths, bounded account trace fetches, and WebUI coverage.
Findings
None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloop review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloop reviewwhen the fix may affect multiple areas. - Use
@ironloop statusto check queued/running/completed/stale/stalled state while reviewers run.
Lockfile-only patch bump of a transitive dep (via termimad/crossbeam) to clear the new advisory failing cargo-deny; verified locally with cargo deny check advisories. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Verdict: ❌ Changes requested
Findings: 1 blocking / 0 notes
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Head: 951ee80d9c865fb9de5c6e3eff53d0fad752c147
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
Found one blocking issue: the new Trace Commons account login-link path bypasses host RuntimeHttpEgress for the upload-claim mint before using the sink for the login-link POST.
Findings
1. ❌ [HIGH] Route the login-link claim mint through host egress
Location: crates/ironclaw_reborn_traces/src/contribution.rs:6661-6664
mint_account_login_link_via_sink receives a ContributionHttpSink, but it mints the bearer with DefaultTraceUploadCredentialProvider::bearer_token, whose issuer request uses the direct reqwest path (fetch_trace_upload_claim_from_issuer(..., None)). That means an agent-invoked builtin.trace_commons.account_login_link performs a network call to the upload-claim issuer outside RuntimeHttpEgress, bypassing the deployment's egress policy/approval/accounting path that this PR explicitly requires for agent-driven network effects. The profile token/profile set flows pass Some(sink) into the claim mint; this path should do the same or otherwise route the upload-claim request through the supplied sink before posting the login-link request.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloop review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloop reviewwhen the fix may affect multiple areas. - Use
@ironloop statusto check queued/running/completed/stale/stalled state while reviewers run.
| let context = | ||
| TraceUploadClaimContext::for_account(resolution.subject.clone()).with_scope_dir(scope_dir); | ||
| let provider = DefaultTraceUploadCredentialProvider; | ||
| let bearer = provider |
Contributor
There was a problem hiding this comment.
This bearer mint bypasses the supplied ContributionHttpSink: DefaultTraceUploadCredentialProvider::bearer_token calls the direct reqwest issuer path, so the agent-invoked login-link tool performs the upload-claim network request outside RuntimeHttpEgress. Please route this claim mint through the sink as the profile token/profile set flows do, so deployment egress policy/approval/accounting is enforced before the login-link POST.
Collaborator
Author
There was a problem hiding this comment.
Fixed in b2efc85. Added trace_upload_bearer_token_via(policy, context, force_refresh, sink) — the sink-aware mint that threads Option<&dyn ContributionHttpSink> down into fetch_trace_upload_claim_from_issuer (claim caching unchanged; DefaultTraceUploadCredentialProvider now delegates with None). Both mint_account_login_link_inner and fetch_account_traces_inner pass Some(sink), matching the profile-token/profile-set flows, so the agent-invoked upload-claim mint routes through RuntimeHttpEgress like the follow-up request. Tests now use a RecordingSink to pin the invariant: both entry points assert exactly two sink executions (claim mint first, then the login-link POST / traces GET).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/superpowers/plans/2026-06-25-trace-commons-slice4-trace-inspection.md (2)
352-355: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winScope the React Query cache to the authenticated caller.
["account-traces"]is shared across sessions, so a logout/login or tenant switch can reuse a previous user's traces until the cache is invalidated. This endpoint is caller-scoped; include tenant/user identity in the key or clear the cache on auth changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-06-25-trace-commons-slice4-trace-inspection.md` around lines 352 - 355, The React Query cache for useAccountTraces is currently keyed too broadly with ["account-traces"], which can leak caller-scoped data across auth sessions. Update the queryKey in useAccountTraces to include authenticated identity such as tenant/user scope, or ensure the cache is invalidated on login/logout and tenant switch so fetchAccountTraces never reuses a previous caller’s traces.
352-361: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDon't flatten failures into the unenrolled empty state.
query.data?.traces || []and!!query.data?.enrolledmake loading and backend errors look identical to the zero-state. That hides outages and violates the slice contract: only explicit unenrolled/404 should render empty.Also applies to: 366-369
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-06-25-trace-commons-slice4-trace-inspection.md` around lines 352 - 361, The useAccountTraces hook is flattening missing data, loading, and backend errors into the same empty unenrolled state via the query result mapping. Update useAccountTraces to preserve query state and error information from useQuery, and only derive an empty traces/enrolled=false state for explicit unenrolled or 404 responses; otherwise let callers distinguish loading and failure from the zero-state.
♻️ Duplicate comments (1)
crates/ironclaw_webui_v2/static/js/pages/settings/components/trace-commons-tab.js (1)
254-256: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNull
final_credit"—" fallback still untested.Past review asked for coverage of "null
final_creditdisplay" among other branches.tracesSectionModewas extracted and tested, but this ternary (the actual "—" rendering decision) remains inline in JSX and isn't covered bytrace-commons-tab.test.mjs— onlyformatCredit(null)(a different code path) is tested there.♻️ Extract as a testable helper
+export function formatFinalCredit(value) { + return value != null ? formatCredit(value) : "—"; +}- ${trace.final_credit != null - ? formatCredit(trace.final_credit) - : "—"} + ${formatFinalCredit(trace.final_credit)}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_webui_v2/static/js/pages/settings/components/trace-commons-tab.js` around lines 254 - 256, The null final_credit fallback is still only handled inline in the JSX ternary, so the “—” branch is not directly covered by tests. Extract the display decision from trace-commons-tab.js into a small testable helper used by the Trace Commons tab rendering, then add/adjust trace-commons-tab.test.mjs to assert the helper’s null path renders “—” and the non-null path still uses formatCredit(trace.final_credit).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_webui_v2/CLAUDE.md`:
- Around line 93-95: Update the documentation entry for trace_account_traces in
CLAUDE.md to explicitly state the limit clamp behavior: document that the
optional limit parameter defaults to 200 and is clamped to the [1, 500] range.
Keep the existing table row, but add a nearby prose note or section consistent
with list_threads, get_timeline, and logs so the bounds are discoverable from
the ProductWorkflow API docs.
---
Outside diff comments:
In `@docs/superpowers/plans/2026-06-25-trace-commons-slice4-trace-inspection.md`:
- Around line 352-355: The React Query cache for useAccountTraces is currently
keyed too broadly with ["account-traces"], which can leak caller-scoped data
across auth sessions. Update the queryKey in useAccountTraces to include
authenticated identity such as tenant/user scope, or ensure the cache is
invalidated on login/logout and tenant switch so fetchAccountTraces never reuses
a previous caller’s traces.
- Around line 352-361: The useAccountTraces hook is flattening missing data,
loading, and backend errors into the same empty unenrolled state via the query
result mapping. Update useAccountTraces to preserve query state and error
information from useQuery, and only derive an empty traces/enrolled=false state
for explicit unenrolled or 404 responses; otherwise let callers distinguish
loading and failure from the zero-state.
---
Duplicate comments:
In
`@crates/ironclaw_webui_v2/static/js/pages/settings/components/trace-commons-tab.js`:
- Around line 254-256: The null final_credit fallback is still only handled
inline in the JSX ternary, so the “—” branch is not directly covered by tests.
Extract the display decision from trace-commons-tab.js into a small testable
helper used by the Trace Commons tab rendering, then add/adjust
trace-commons-tab.test.mjs to assert the helper’s null path renders “—” and the
non-null path still uses formatCredit(trace.final_credit).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d26cfa78-ad01-4572-98ee-e0992df78afc
📒 Files selected for processing (6)
crates/ironclaw_reborn_traces/src/contribution.rscrates/ironclaw_webui_v2/CLAUDE.mdcrates/ironclaw_webui_v2/static/js/pages/settings/components/trace-commons-tab.jscrates/ironclaw_webui_v2/static/js/pages/settings/components/trace-commons-tab.test.mjsdocs/superpowers/plans/2026-06-25-trace-commons-slice3-login-link-capability.mddocs/superpowers/plans/2026-06-25-trace-commons-slice4-trace-inspection.md
Comment on lines
+93
to
+95
| | `webui.v2.trace_credits` | GET | `/api/webchat/v2/traces/credit` | None | `ProductWorkflow` | | ||
| | `webui.v2.trace_account_traces` | GET | `/api/webchat/v2/traces/account` (optional `?limit=N`) | None | `ProductWorkflow` | | ||
| | `webui.v2.authorize_trace_hold` | POST | `/api/webchat/v2/traces/holds/{submission_id}/authorize` | None | `ProductWorkflow` | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Document the account-traces limit clamp.
Other paginated routes (list_threads, get_timeline, logs) get a dedicated prose section spelling out clamp bounds. Per the PR objective, trace_account_traces clamps limit to [1, 500] with default 200, but the table only notes (optional ?limit=N) with no bounds documented.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_webui_v2/CLAUDE.md` around lines 93 - 95, Update the
documentation entry for trace_account_traces in CLAUDE.md to explicitly state
the limit clamp behavior: document that the optional limit parameter defaults to
200 and is clamped to the [1, 500] range. Keep the existing table row, but add a
nearby prose note or section consistent with list_threads, get_timeline, and
logs so the bounds are discoverable from the ProductWorkflow API docs.
The sink-based entry points (mint_account_login_link_via_sink, fetch_account_traces_via_sink) used the sink for the final POST/GET but minted the upload-claim bearer via DefaultTraceUploadCredentialProvider, whose issuer request takes the direct reqwest path — so an agent-invoked account_login_link performed a network call outside RuntimeHttpEgress. New trace_upload_bearer_token_via threads Option<sink> into the claim mint (cache behavior unchanged; the default provider passes None), and both sink paths pass Some(sink), matching the profile-token/profile-set flows. Tests now use a RecordingSink to pin the invariant that both the claim mint and the follow-up request route through the sink. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
@IronLoop review --agent reviewer |
Collaborator
Author
|
@IronLoop status |
Collaborator
Author
|
@IronLoop review |
zmanian
enabled auto-merge
July 8, 2026 01:29
zmanian
dismissed
ironloopai[bot]’s stale review
July 8, 2026 02:47
Dismissing stale bot review: the sole blocking finding (login-link/account-traces claim mint bypassing RuntimeHttpEgress) was fixed in b2efc85 — trace_upload_bearer_token_via threads the sink into the claim mint, with RecordingSink regression tests pinning that both the claim mint and follow-up request route through the sink. ironloop has been unresponsive to three re-review triggers over ~4h (GitHub incident); CI is green on the fixed head.
zmanian
added a commit
that referenced
this pull request
Jul 9, 2026
…ne user Codex review on c6466d6 caught that 'traces opt-out' always flips the ROOT policy before the scoped one — and since #5280 that root file IS the instance-wide enrollment, so opting out one user (even with --user-scope) silently disenrolled the entire instance. The enroll-instance guidance text pointed users at exactly that command. - New primitive ironclaw_reborn_traces::contribution::opt_out_user_scope[_at]: writes ONLY the scoped policy with enabled=false (the resolver's explicit opt-out signal). Test pins: instance policy untouched on disk, the opted-out user stops resolving, other users keep inheriting. - CLI: --user-scope now performs a scoped-only opt-out; bare opt-out keeps the legacy full-disable semantics and prints a note distinguishing the two. enroll-instance guidance names the scoped form. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
zmanian
added a commit
that referenced
this pull request
Jul 9, 2026
…ount login links (#5858) * feat(traces): ironclaw-reborn traces enroll-instance — admin instance enrollment CLI Closes the Path B gap from PR #5280: onboard_instance_with_sink existed but had no product entry point, so invite-based (DeviceKey, per-user-attributed) instance enrollment was unreachable for admins. - onboarding::onboard_instance_at_base — base-dir-parameterised instance enrollment with the default direct sink; targets the scope-None location so every user without a personal enrollment inherits it via resolve_trace_credentials. Test pins the instance-dir targeting (policy + promoted device key at trace_contributions/, no users/<hash> dir). - New CLI subcommand: traces enroll-instance --invite <url> [--include-message-text] [--include-tool-payloads] [--json]. Host-shell possession is the admin gate, matching traces opt-in's trust boundary for the global policy. Parse tests cover flags and required --invite. - slice1 plan note updated: the deferred admin surface now exists. Also in this commit (same file): mint_account_login_link direct variant and DirectPinnedContributionSink used by the WebUI surface in the next commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(webui): Open Trace Commons account from the WebUI — hosted-user login links Hosted multi-tenant users have no host shell, and the agent capability delivers the one-time login URL to a local file they cannot read. This adds the product path: an authenticated WebUI action that mints the link and returns it directly to the caller's browser — the URL never touches a local file, a log line, or any model-visible surface. - product_workflow: account_login_link_for_user + RebornServicesApi::trace_account_login_link (caller-derived scope; unenrolled → zero-state, not an error). - webui_v2: POST /api/webchat/v2/traces/account-login-link (webui.v2.trace_account_login_link), NoBody, per-caller 10/min rate limit (each call mints a credential). Descriptor + handler contract tests; route table row in CLAUDE.md. - frontend: 'Open Trace Commons account' button on the Trace Commons settings tab. openAccountLoginLink opens a blank tab synchronously (popup-blocker attribution) with noopener,noreferrer, then navigates it to the minted URL; closes the placeholder on unenrolled/error. i18n for all 11 languages. - test-runner fix: trace-commons-tab.test.mjs had silently stopped running after the frontend migration (vitest include is *.{test,spec}.{ts,tsx}); converted to trace-commons-tab.test.ts with the window.open fake capturing every argument the production caller passes. Suite: 582 → 590. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address automated review: absolutize login-link URL, fix noopener handle, mint after popup check - mint_account_login_link_inner resolves a relative response url against the login-links endpoint (trust-anchored issuer origin); a relative url would otherwise resolve against the consuming surface's own origin (e.g. the IronClaw WebUI host). Tests assert the absolutized form on both paths. - openAccountLoginLink no longer passes the noopener feature to window.open — modern browsers return null for noopener opens, which made navigation impossible; the handle's opener is severed manually instead (reverse-tabnabbing protection preserved). The test fake now models the real browser contract (noopener feature → null handle). - Popup-blocked opens short-circuit BEFORE the mint call so a blocked tab no longer burns a single-use login link server-side; test asserts zero mints. - DirectPinnedContributionSink is pub(crate) with a documented invariant: request URLs must come from the enrolled policy's trust-anchored endpoint derivations, never caller input. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address local Codex review: pin login-link URL to issuer origin, sink transport tests, UI guards - SECURITY: mint_account_login_link_inner now rejects any response url whose final resolved form leaves the trust-anchored issuer origin (scheme + host + port must match the login-links endpoint; userinfo rejected) — a hostile or compromised issuer response could otherwise steer the authenticated browser popup to an arbitrary origin or a javascript: URL. Tests cover relative absolutization, cross-origin rejection, javascript: rejection, and userinfo rejection. - DirectPinnedContributionSink transport hardening is now regression-tested: link-local/metadata hosts rejected at resolution, 3xx surfaced without contacting the Location target, oversized bodies rejected during the streamed read. - Frontend: handleOpenAccount gains an in-flight ref (double-click burns a one-time link) and a mounted ref (no setState after unmount). - Handler contract test now asserts the forwarded caller carries the authenticated tenant AND user id, not just call count. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Client-side defense in depth: refuse non-http(s) minted login URLs The about:blank tab inherits the WebUI origin, so a javascript: URL would execute with WebUI-origin access (e.g. session storage). The backend now origin-pins the minted URL; the client additionally refuses to navigate to anything that is not absolute http(s), independently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Scoped-only user opt-out: never disable the instance enrollment for one user Codex review on c6466d6 caught that 'traces opt-out' always flips the ROOT policy before the scoped one — and since #5280 that root file IS the instance-wide enrollment, so opting out one user (even with --user-scope) silently disenrolled the entire instance. The enroll-instance guidance text pointed users at exactly that command. - New primitive ironclaw_reborn_traces::contribution::opt_out_user_scope[_at]: writes ONLY the scoped policy with enabled=false (the resolver's explicit opt-out signal). Test pins: instance policy untouched on disk, the opted-out user stops resolving, other users keep inheriting. - CLI: --user-scope now performs a scoped-only opt-out; bare opt-out keeps the legacy full-disable semantics and prints a note distinguishing the two. enroll-instance guidance names the scoped form. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Align enroll-instance clap help with scoped-only opt-out semantics The doc comment still pointed users at bare 'traces opt-out' for self-exclusion, which is now the instance-wide off switch; name the --user-scope form and warn about the bare form, matching the runtime output. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Merged
22 tasks
21 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the IronClaw client side of Trace Commons instance-wide enrollment, per-user contributor accounts/profiles, and submitted-trace inspection, alongside (not replacing) the existing per-scope personal-invite flow. The matching server change ships as trace-commons-server PR #152 (optional per-user
subject→ namespaced principal).Built design-first: spec + 5 plans under
docs/superpowers/, executed subagent-driven with per-task review.What's here
None) with a per-user pseudonymoussubject(local_pseudonymous_contributor_id). One construction site for policy paths (dir-parameterized_athelpers).AdminScope::enroll_instance_trace_commons(unconstructable for non-admins).subjectplumbing — threaded through the upload-claim request + submission; omitted (byte-identical) for personal-invite and CLI/worker paths.trace_commons.account_login_link— consent-gated, model-visible first-party capability that mints a one-time browser login URL.fetch_account_traces→product_workflowfacadetrace_account_traces→GET /api/webchat/v2/traces/account→ settings UI.Security
A mid-stream attempt to let the host-minted trace bearer through the egress guard by exempting all
RuntimeKind::FirstPartyrequests was caught in review (it would have let the model-controlledbuiltin.httpsmuggle arbitraryAuthorization/Cookieheaders to allowlisted hosts). It was reverted; the trace bearer now flows through the staged credential-injection path (zeroizedSecretMaterial), applied after sanitization — which also fixed the same latent issue in the pre-existingprofile_set/profile_tokenpaths. The sensitive-header + manual-credential guards are restored for all runtimes and regression-tested. Verified by an opus whole-branch review.Verification
cargo fmt --all --checkclean;cargo clippyclean across touched crates; per-crate test suites green.Known follow-ups (non-blocking, tracked in the branch ledger)
fetch_account_tracesnon-2xx silently returns empty — add adebug!+ tests for the non-2xx/transport branches.account_login_link_through_dispatchmock should assert it received theAuthorizationheader (currently inferential).fetch_account_tracesGET lacksresolve_to_addrspinning (operator-configured host; consistent with the submission lane).traceStatus,traceReceivedAt);final_creditNone renders as0.00.fetch_account_traces_via_sinkretained as annotated agent-path scaffold (no capability wired yet).🤖 Generated with Claude Code