refactor(composition): group product-auth cluster under product_auth/ (dissection n4) - #5686
Conversation
… (dissection n4) Step 4 of the composition dissection (docs/plans/2026-07-02-reborn-internal-module-refactor.md §3). Pure module move — no behavior change, crate public API unchanged (crate-root pub use re-pointed 1:1; composition-pubuse.snapshot updated for the re-points and to pick up AttachmentTestSupport, which landed on main after the baseline). Mapping: - product_auth::api <- auth.rs, auth_prompt.rs, auth_dcr_tests.rs - product_auth::oauth <- oauth_dcr(.rs/_protocol.rs), oauth_gate.rs, oauth_provider_client(+tests), google_oauth/, notion_oauth.rs - product_auth::durable <- product_auth_durable(+8 submodules) - product_auth::serve <- product_auth_serve/ (cfg-gated webui-v2-beta) - product_auth::credentials <- product_auth_runtime_credentials(+tests), product_auth_providers.rs, product_auth_refresh_lock.rs, credential_refresh_worker.rs, manual_token_flow.rs Deliberately NOT absorbed (other domains per the plan): profile_approval_authorization (runtime-profile policy), extension_activation_credentials + extension_credential_requirements (extension_host, n7), input.rs (build config), nearai_login_serve (llm_admin, n6). Architecture boundary tests: path updates only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
❌ IronLoop Review StatusHead:
Configuration errorMessage: Trusted agent config is invalid: Invalid type: Expected Object but received true
Available commands
Run metadataOrigin: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughMoves Reborn composition auth/OAuth/credential code under Changesproduct_auth restructure
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 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 |
There was a problem hiding this comment.
Code Review
This pull request reorganizes the product_auth modules in ironclaw_reborn_composition into a cleaner, nested directory structure (api, credentials, durable, oauth, and serve) and updates all internal imports and public exports accordingly. The review feedback correctly identifies a test coverage gap in reborn_dependency_boundaries.rs, where the path to the serve module's entry point was incorrectly updated to serve.rs instead of serve/mod.rs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| collect_forbidden_reborn_auth_path_uses( | ||
| &root.join("crates/ironclaw_reborn_composition/src/product_auth_serve"), | ||
| &root.join("crates/ironclaw_reborn_composition/src/product_auth_serve.rs"), | ||
| &root.join("crates/ironclaw_reborn_composition/src/product_auth/serve"), | ||
| &root.join("crates/ironclaw_reborn_composition/src/product_auth/serve.rs"), |
There was a problem hiding this comment.
The entry point file for the serve module is located at crates/ironclaw_reborn_composition/src/product_auth/serve/mod.rs rather than crates/ironclaw_reborn_composition/src/product_auth/serve.rs. Passing a non-existent file path to collect_forbidden_reborn_auth_path_uses will cause the test to silently skip scanning the module's entry point, creating a test coverage gap.
| collect_forbidden_reborn_auth_path_uses( | |
| &root.join("crates/ironclaw_reborn_composition/src/product_auth_serve"), | |
| &root.join("crates/ironclaw_reborn_composition/src/product_auth_serve.rs"), | |
| &root.join("crates/ironclaw_reborn_composition/src/product_auth/serve"), | |
| &root.join("crates/ironclaw_reborn_composition/src/product_auth/serve.rs"), | |
| collect_forbidden_reborn_auth_path_uses( | |
| &root.join("crates/ironclaw_reborn_composition/src/product_auth/serve"), | |
| &root.join("crates/ironclaw_reborn_composition/src/product_auth/serve/mod.rs"), |
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Verdict: ✅ Approved
Findings: 0 blocking / 0 notes
Next: No reviewer action needed.
Head: 2813b7a7e49c170d40a78895053c055ba134de2f
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete correctness, security, or maintainability issues were found in this module reorganization. The public product-auth re-exports appear preserved, internal references were updated to the new product_auth::* paths, and whitespace checks passed.
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.
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.45% — 276153 / 323179 lines Per-crate breakdown (65 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (4 entry/entries excluded from the accounting above)
|
|
🚅 Deployed to the ironclaw-pr-5686 environment in ironclaw-ci-preview
|
🗂️ Archived IronLoop Review: reviewerThis result is from an older PR head and is no longer the active review.
Archived summaryNo concrete correctness, security, or maintainability issues were found in this module reorganization. The public product-auth re-exports appear preserved, internal references were updated to the new |
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Verdict: ✅ Approved
Findings: 0 blocking / 0 notes
Next: No reviewer action needed.
Head: 18497d2bdb3afa5e037d7f5e4396c1d701e519d6
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete, actionable regressions found. The PR reorganizes Reborn product-auth modules under a product_auth cluster and updates internal references, architecture boundary paths, and the public-use snapshot accordingly.
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.
serrrfirat
left a comment
There was a problem hiding this comment.
Multi-agent review complete for PR #5686.
Reviewers: security clean, bugs clean, performance/concurrency clean, tests clean, conventions found 2 anchored issues.
Findings posted inline:
- Medium: current docs still reference old product-auth paths after the module move.
- Low: one moved comment still names the old durable module.
| //! WebUI route serving (`serve`), and runtime credential resolution/refresh | ||
| //! (`credentials`) — behind one internal module. The crate root re-exports the | ||
| //! same public items from here so the crate's public API is unchanged. | ||
|
|
There was a problem hiding this comment.
Medium - repo-rule conformance
This move introduces the new product_auth module layout, but current docs still point readers at the old paths: docs/extensions/building-a-tool.md still references src/notion_oauth.rs, src/oauth_provider_client.rs, and src/product_auth_serve/; docs/reborn/contracts/auth-product.md still references src/product_auth_serve/mod.rs. .claude/rules/review-discipline.md requires updating .md/CLAUDE.md references to moved paths in the same refactor PR.
Fix: update those docs to the new product_auth/oauth/... and product_auth/serve/... paths.
| @@ -103,12 +103,12 @@ pub(crate) trait CredentialRefreshCandidateSource: Send + Sync { | |||
| // Note: this requires the `product_auth_durable` module to be `pub(crate)`. | |||
There was a problem hiding this comment.
Low - stale comment
This comment still says the blanket impl requires the old product_auth_durable module to be pub(crate), but the impl now targets crate::product_auth::durable::FilesystemAuthProductServices. That leaves a stale path immediately after the move.
Fix: change the comment to name product_auth::durable, or remove the module-path detail.
What this is
Dissection n4 — step 4 of the composition internal-module plan (
docs/plans/2026-07-02-reborn-internal-module-refactor.md§3, on main). Pure behavior-preserving module move: the product-auth cluster (~23k lines, 57 files) groups under one internalproduct_auth/module, mirroring the pattern #5585 established forobservability//outbound//support/.Mapping
product_auth::apiauth.rs,auth_prompt.rs,auth_dcr_tests.rsproduct_auth::oauthoauth_dcr(.rs/_protocol),oauth_gate.rs,oauth_provider_client(+tests),google_oauth/,notion_oauth.rsproduct_auth::durableproduct_auth_durable+ its 8 submodulesproduct_auth::serveproduct_auth_serve/(cfg-gatedwebui-v2-beta)product_auth::credentialsproduct_auth_runtime_credentials(+tests),product_auth_providers.rs,product_auth_refresh_lock.rs,credential_refresh_worker.rs,manual_token_flow.rsDeliberately NOT absorbed (other domains per the plan):
profile_approval_authorization(runtime-profile policy — root),extension_activation_credentials+extension_credential_requirements(extension_host, n7),input.rs(build config),nearai_login_serve(llm_admin, n6).API stability
Crate-root
pub useblocks re-pointed 1:1 — exported item set byte-identical (verified againstcomposition-pubuse.snapshot). The snapshot file is updated in this PR for the path re-points and to pick upAttachmentTestSupport, which landed on main after the baseline was generated. Architecture boundary tests: path updates only, no assertion changes.Sequence
Includes a fresh-
mainmerge (post-#5593) with fallout sweep (zero stale-path references). Next in the series: n5projection→ n6llm_admin→ n7extension_host→ n8slack→ n9automation→ n10webui→ n11 root collapse.🤖 Generated with Claude Code