Skip to content

refactor(gateway): extract start_server + route composition into platform/router.rs — ironclaw#2599 stage 2 - #2643

Merged
ilblackdragon merged 2 commits into
stagingfrom
refactor/gateway-platform-stage2
Apr 18, 2026
Merged

ilblackdragon merged 2 commits into
stagingfrom
refactor/gateway-platform-stage2

Conversation

@ilblackdragon

Copy link
Copy Markdown
Member

Summary

Second increment of the ironclaw#2599 platform/feature split (follow-up to #2628). Moves start_server() and the Axum route composition out of src/channels/web/server.rs into a dedicated src/channels/web/platform/router.rs, so the platform-vs-features dependency direction becomes visible: the router depends on handler modules, never the reverse.

Changes

  • New platform/router.rs — owns start_server(), the four routers (`public`, `protected`, `statics`, `projects`), and the cross-cutting layer stack (CORS, 10 MB body limit, panic catch, `X-Content-Type-Options`, `X-Frame-Options`, CSP).
  • Feature handlers still inline in `server.rs` are now `pub(crate)` so the router can register them without leaking them outside the crate. Two private structs (`HistoryQuery`, `GatewayStatusResponse`) directly referenced by pub(crate) handlers were also raised to `pub(crate)` so the handler signatures type-check from the router module.
  • `server.rs` keeps the feature handlers that haven't migrated yet (OAuth callbacks, chat, extensions, pairing, logs, gateway status) and adds `pub use platform::router::start_server` so external callers (`src/channels/web/mod.rs`, `tests/multi_tenant_integration.rs`) keep working. Unused imports (`Router`, `DefaultBodyLimit`, `CorsLayer`, `tokio::sync::mpsc`, etc.) dropped.
  • `mod.rs` now calls `platform::router::start_server` directly.
  • `CLAUDE.md` file map updated.

Behavior change

None. Route table, middleware stack, CORS policy, body limits, panic handling, security headers, and CSP are byte-identical to origin/staging — verified by visual diff against the pre-move code.

Stats

  • `server.rs`: 7463 → 6973 lines (−490)
  • New `platform/router.rs`: 537 lines

Test plan

  • `cargo fmt --all`
  • `cargo clippy --all --benches --tests --examples --all-features` — clean
  • `cargo check --lib --tests --all-features` — clean
  • `cargo test --lib` — 5068 passed, same 2 pre-existing failures as stage 1 (`pairing::approval::tests::propagate_approval_restores_runtime_state_when_on_start_fails`, `extensions::manager::tests::test_telegram_token_colon_preserved_in_validation_url`) — neither touches any symbol this PR moves
  • Manual smoke: reviewer runs `cargo run` and exercises a few routes

Follow-ups

Stage 3: relocate `auth.rs` / `sse.rs` / `ws.rs` into `platform/`.
Stage 4: open `features//` directories and migrate handlers.
Stage 5: CI boundary check.
Stage 6: delete re-export shims.

Unrelated follow-ups remain tracked in issue #2633.

🤖 Generated with Claude Code

…atform/router.rs — ironclaw#2599 stage 2

Second increment of the ironclaw#2599 platform/feature split. Moves
`start_server()` and the Axum route composition out of `server.rs` into
a dedicated `platform/router.rs`, so the platform-vs-features
dependency direction is visible: the router depends on handler modules
(both `handlers/*` and the still-inline handlers in `server.rs`),
never the reverse.

Changes:

- New `src/channels/web/platform/router.rs` owns `start_server()`, the
  four routers (`public`, `protected`, `statics`, `projects`), and the
  cross-cutting layer stack (CORS, 10 MB body limit, panic catch,
  `X-Content-Type-Options`, `X-Frame-Options`, CSP).
- Feature handlers still inline in `server.rs` are now `pub(crate)` so
  the router can register them without leaking them outside the crate.
  Two private structs that are directly referenced by pub(crate)
  handlers (`HistoryQuery`, `GatewayStatusResponse`) were also raised
  to `pub(crate)` so the handler signatures type-check from the router
  module.
- `server.rs` keeps the feature handlers that haven't migrated yet
  (OAuth callbacks, chat, extensions, pairing, logs, gateway status)
  and adds `pub use platform::router::start_server` so external call
  sites — `src/channels/web/mod.rs`, `tests/multi_tenant_integration.rs`
  — keep working. The trimmed imports drop `Router`, `DefaultBodyLimit`,
  `CorsLayer`, `tokio::sync::mpsc`, etc., since they're no longer used
  in the remaining body.
- `mod.rs` now calls `platform::router::start_server` directly; the
  `server::start_server` shim exists only for external code paths that
  still reach for it.
- `CLAUDE.md` file map now lists `platform/router.rs` and clarifies
  that `server.rs` is feature-handler-only pending migration.

No behavior change. Route table, middleware stack, CORS policy, body
limits, panic handling, security headers, and CSP are byte-identical
to origin/staging.

Stats: server.rs 7463 → 6973 lines (−490); new `platform/router.rs` is
537 lines. `cargo clippy --all --benches --tests --examples
--all-features` is clean. Unit test run: 5068 passed; the same 2
pre-existing failures carried over from stage 1
(`pairing::approval::tests::propagate_approval_restores_runtime_state_when_on_start_fails`
needs a telegram WASM fixture;
`extensions::manager::tests::test_telegram_token_colon_preserved_in_validation_url`
has a test-infra URL override — neither references any symbol this
PR touches).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 18, 2026 09:56
@github-actions github-actions Bot added scope: channel/web Web gateway channel scope: docs Documentation size: XL 500+ changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Apr 18, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the web gateway by moving the start_server function and Axum route composition from server.rs to a new platform/router.rs module. This change is part of a migration to decouple the platform layer from feature handlers. One issue was identified regarding the use of .expect() in production code, which violates project rules; a suggestion was provided to replace it with proper error handling during server bootstrap.

Comment thread src/channels/web/platform/router.rs Outdated
Comment on lines +452 to +459
.allow_origin([
format!("http://{}:{}", addr.ip(), addr.port())
.parse()
.expect("valid origin"),
format!("http://localhost:{}", addr.port())
.parse()
.expect("valid origin"),
])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Avoid using .expect() in production code, as it is banned by project rules. In this context, constructing the CORS origins can fail if the resulting string is not a valid HeaderValue. It is better to handle the error explicitly by mapping it to a semantically specific error variant like ChannelError::StartupFailed to ensure the server fails gracefully during bootstrap. Additionally, using format!("http://{}", addr) is more robust for IPv6 addresses than manually concatenating the IP and port.

Suggested change
.allow_origin([
format!("http://{}:{}", addr.ip(), addr.port())
.parse()
.expect("valid origin"),
format!("http://localhost:{}", addr.port())
.parse()
.expect("valid origin"),
])
.allow_origin([
format!("http://{}", addr)
.parse()
.map_err(|e| crate::error::ChannelError::StartupFailed {
name: "gateway".to_string(),
reason: format!("Invalid IP origin: {}", e),
})?,
format!("http://localhost:{}", addr.port())
.parse()
.map_err(|e| crate::error::ChannelError::StartupFailed {
name: "gateway".to_string(),
reason: format!("Invalid localhost origin: {}", e),
})?,
])
References
  1. Avoid using .expect() in production code, as it is banned by project rules. Prefer alternatives like compile-time macros (e.g., serde_json::json!) or proper error handling.
  2. Create specific error variants for different failure modes (e.g., DownloadFailed with a URL string vs. ManifestRead with a file path) to provide semantically correct and clear error messages.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 31735cc. Replaced the two .expect() calls with .map_err(|e| ChannelError::StartupFailed { .. }) so a malformed bound address fails the gateway bootstrap instead of panicking. Also switched to format!("http://{addr}") so IPv6 binds produce the correctly bracketed origin. This also resolves the No panics in production code CI failure.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the web gateway to make the platform/feature split (ironclaw#2599 stage 2) explicit by moving Axum route wiring and server bootstrap out of the server.rs feature-handler module and into platform/router.rs.

Changes:

  • Added src/channels/web/platform/router.rs to own start_server() and all route composition + shared middleware layers.
  • Converted remaining inline handlers (and a couple referenced DTO structs) in server.rs to pub(crate) so the new router can register them.
  • Updated platform/mod.rs exports and refreshed the web gateway file map docs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/channels/web/server.rs Removes start_server() implementation; keeps unmigrated feature handlers and re-exports start_server from platform::router; adjusts handler visibility for router registration.
src/channels/web/platform/router.rs New module containing TCP bind + route composition (public/protected/statics/projects) and shared middleware stack.
src/channels/web/platform/mod.rs Exposes the new router submodule.
src/channels/web/CLAUDE.md Updates the file map to document the new router module.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/channels/web/platform/mod.rs Outdated
Comment on lines +7 to +11
@@ -8,5 +8,6 @@
//!
//! See `src/channels/web/CLAUDE.md` for the staged migration plan.

pub mod router;

Copilot AI Apr 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

platform/mod.rs docstring says feature handlers depend on platform "not the other way around", but platform::router now imports feature handlers from handlers/* and server.rs. Please update this doc comment to clarify the router is the intentional coupling point (or relocate the router out of platform/ if the no-back-edges rule is still intended).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 31735cc. Rewrote the platform/mod.rs docstring to name router as the single, intentional exception to the no-back-edges rule, and explicitly list the platform submodules the rule still applies to (state, static_files, and the future auth/sse/ws once they move here).

Comment on lines 9 to 12
| `mod.rs` | Gateway builder, startup, `WebChannel` implementation, `with_*` builder methods |
| `server.rs` | `start_server()`, Axum route registrations, and feature handlers that have not yet moved (OAuth callbacks, chat, extensions, pairing, logs, gateway status). Re-exports `GatewayState` and friends from `platform::state` for backward compatibility during the ironclaw#2599 migration. |
| `server.rs` | Feature handlers that have not yet moved (OAuth callbacks, chat, extensions, pairing, logs, gateway status). Re-exports `GatewayState` / `start_server` / related types from `platform::*` for backward compatibility during the ironclaw#2599 migration. |
| `platform/router.rs` | `start_server()` + Axum route composition (public / protected / statics / projects) and the cross-cutting layer stack (CORS, body limit, panic catch, static security headers, CSP). Single coupling point between platform and features. |
| `platform/state.rs` | `GatewayState`, `RateLimiter`, `PerUserRateLimiter`, `WorkspacePool`, `FrontendHtmlCache`, `FrontendCacheKey`, `ActiveConfigSnapshot`, `PromptQueue`, `RoutineEngineSlot`. Canonical home for shared gateway state. |

Copilot AI Apr 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file map now describes platform/router.rs as the coupling point between platform and features, but the later "Platform vs. feature layering" section still states the platform/ subtree has "no back-edges". Please reconcile the layering docs so readers understand whether the router is an exception or the rule has changed.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 31735cc. Rewrote the "Platform vs. feature layering" section in CLAUDE.md to reconcile with the File Map: router is the coupling point; every other platform submodule must stay handler-agnostic; the CI check planned for ironclaw#2599 stage 5 will forbid cross-imports between platform/{state,static_files,auth,sse,ws}.rs and handlers/* / features/* while explicitly allowing platform/router.rs to reference both sides.

…er CORS + doc reconciliation

Three fixes for review comments on #2643:

1. **router.rs CORS origin parsing (comment #3104968733, CI `no-panics`):**
   Replaced `.expect("valid origin")` with `.map_err(|e| ChannelError::StartupFailed { .. })`
   so a malformed bound address fails the bootstrap with a semantically
   specific error instead of panicking. Also switched from
   `format!("http://{}:{}", addr.ip(), addr.port())` to
   `format!("http://{addr}")` — `SocketAddr`'s `Display` brackets IPv6
   addresses correctly (`[::1]:8080` rather than the ambiguous
   `::1:8080`), so the CORS origin stays valid on v6 binds. Fixes the
   `No panics in production code` CI check failure and the
   `Code Style` composite check that depends on it.

2. **platform/mod.rs docstring (comment #3104970223):** The previous
   "feature handlers depend on platform, not the other way around"
   wording conflicted with `router` importing feature handlers. Rewrote
   to name `router` as the single, intentional exception to the
   no-back-edges rule, and explicitly list the platform submodules the
   rule still applies to (`state`, `static_files`, future `auth`/`sse`/`ws`).

3. **CLAUDE.md layering section (comment #3104970232):** Same
   contradiction — rewrote to reconcile: router is the coupling point;
   every other platform submodule must stay handler-agnostic; the
   forthcoming CI check (ironclaw#2599 stage 5) enforces forbidden
   imports between `platform/{state,static_files,auth,sse,ws}.rs` and
   `handlers/*` / `features/*` while explicitly allowing
   `platform/router.rs` to reference both sides.

Verified: `python3 scripts/check_no_panics.py` clean;
`cargo clippy --all --benches --tests --examples --all-features`
clean; `cargo test --lib channels::web::server::tests::test_` passes
30 server-module tests including CORS / CSP / start_server smoke
coverage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ilblackdragon
ilblackdragon merged commit 962aaed into staging Apr 18, 2026
16 checks passed
@ilblackdragon
ilblackdragon deleted the refactor/gateway-platform-stage2 branch April 18, 2026 13:37
ilblackdragon added a commit that referenced this pull request Apr 18, 2026
…#2599 stage 3

Third increment of the ironclaw#2599 platform/feature split (follow-up
to #2628 and #2643). Moves the three transport / framing modules into
the `platform/` subtree where they logically belong, so the platform
layer now contains the full set of cross-cutting infrastructure
(state, router, static_files, auth, sse, ws).

Changes:

- `src/channels/web/auth.rs`  → `src/channels/web/platform/auth.rs`
- `src/channels/web/sse.rs`   → `src/channels/web/platform/sse.rs`
- `src/channels/web/ws.rs`    → `src/channels/web/platform/ws.rs`
- `platform/mod.rs` declares the three new submodules.
- `channels/web/mod.rs` adds backward-compat re-exports
  (`pub use platform::{auth, sse, ws};`) so every existing
  `crate::channels::web::{auth,sse,ws}::...` call site — roughly 40
  files across handlers, tests, integration tests, and sibling
  modules — continues to resolve without edits. Follow-up PRs will
  migrate call sites to the canonical `platform::` path incrementally.
- `platform/mod.rs` doc comment now describes the platform layer as
  having auth / SSE / WS (no longer "in later stages of #2599").
- `CLAUDE.md` file map points at the new paths and notes the
  re-exports.

Pure move + re-export. No behavior change. Module contents are
byte-identical to pre-move.

Verified: `cargo fmt --all`;
`cargo clippy --all --benches --tests --examples --all-features` clean;
`cargo check --all-features --all-targets` clean;
`python3 scripts/check_no_panics.py` clean;
`cargo test --lib` 5068 passed, same 2 pre-existing failures as
stages 1/2 (`pairing::approval::tests::propagate_approval_restores_runtime_state_when_on_start_fails`
needs a telegram WASM fixture;
`extensions::manager::tests::test_telegram_token_colon_preserved_in_validation_url`
has a test-infra URL override — neither touches any moved module).

Stacks on #2643. Rebases cleanly on staging once #2643 lands.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ilblackdragon added a commit that referenced this pull request Apr 18, 2026
…#2599 stage 3

Third increment of the ironclaw#2599 platform/feature split (follow-up
to #2628 and #2643). Moves the three transport / framing modules into
the platform/ subtree so the platform layer now contains the full set
of cross-cutting infrastructure (state, router, static_files, auth,
sse, ws).

Changes:

- src/channels/web/auth.rs  -> src/channels/web/platform/auth.rs
- src/channels/web/sse.rs   -> src/channels/web/platform/sse.rs
- src/channels/web/ws.rs    -> src/channels/web/platform/ws.rs
- platform/mod.rs declares the three new submodules.
- channels/web/mod.rs adds backward-compat re-exports
  (`pub use platform::{auth, sse, ws};`) so every existing
  `crate::channels::web::{auth,sse,ws}::...` call site - roughly 40
  files across handlers, tests, integration tests, and sibling
  modules - continues to resolve without edits. Follow-up PRs will
  migrate call sites to the canonical `platform::` path incrementally.
- platform/mod.rs doc comment now describes the platform layer as
  having auth / SSE / WS (no longer "in later stages of #2599").
- CLAUDE.md file map points at the new paths and notes the re-exports.

Pure move + re-export. No behavior change. Module contents are
byte-identical to pre-move.

Verified: cargo fmt --all; cargo clippy --all --benches --tests
--examples --all-features clean; python3 scripts/check_no_panics.py
clean; cargo check --all-features --all-targets clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ilblackdragon added a commit that referenced this pull request Apr 18, 2026
…#2599 stage 3 (#2644)

Third increment of the ironclaw#2599 platform/feature split (follow-up
to #2628 and #2643). Moves the three transport / framing modules into
the platform/ subtree so the platform layer now contains the full set
of cross-cutting infrastructure (state, router, static_files, auth,
sse, ws).

Changes:

- src/channels/web/auth.rs  -> src/channels/web/platform/auth.rs
- src/channels/web/sse.rs   -> src/channels/web/platform/sse.rs
- src/channels/web/ws.rs    -> src/channels/web/platform/ws.rs
- platform/mod.rs declares the three new submodules.
- channels/web/mod.rs adds backward-compat re-exports
  (`pub use platform::{auth, sse, ws};`) so every existing
  `crate::channels::web::{auth,sse,ws}::...` call site - roughly 40
  files across handlers, tests, integration tests, and sibling
  modules - continues to resolve without edits. Follow-up PRs will
  migrate call sites to the canonical `platform::` path incrementally.
- platform/mod.rs doc comment now describes the platform layer as
  having auth / SSE / WS (no longer "in later stages of #2599").
- CLAUDE.md file map points at the new paths and notes the re-exports.

Pure move + re-export. No behavior change. Module contents are
byte-identical to pre-move.

Verified: cargo fmt --all; cargo clippy --all --benches --tests
--examples --all-features clean; python3 scripts/check_no_panics.py
clean; cargo check --all-features --all-targets clean.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ilblackdragon added a commit that referenced this pull request Apr 19, 2026
…e 5 (#2647)

* refactor(gateway): relocate auth / sse / ws into platform/ — ironclaw#2599 stage 3

Third increment of the ironclaw#2599 platform/feature split (follow-up
to #2628 and #2643). Moves the three transport / framing modules into
the platform/ subtree so the platform layer now contains the full set
of cross-cutting infrastructure (state, router, static_files, auth,
sse, ws).

Changes:

- src/channels/web/auth.rs  -> src/channels/web/platform/auth.rs
- src/channels/web/sse.rs   -> src/channels/web/platform/sse.rs
- src/channels/web/ws.rs    -> src/channels/web/platform/ws.rs
- platform/mod.rs declares the three new submodules.
- channels/web/mod.rs adds backward-compat re-exports
  (`pub use platform::{auth, sse, ws};`) so every existing
  `crate::channels::web::{auth,sse,ws}::...` call site - roughly 40
  files across handlers, tests, integration tests, and sibling
  modules - continues to resolve without edits. Follow-up PRs will
  migrate call sites to the canonical `platform::` path incrementally.
- platform/mod.rs doc comment now describes the platform layer as
  having auth / SSE / WS (no longer "in later stages of #2599").
- CLAUDE.md file map points at the new paths and notes the re-exports.

Pure move + re-export. No behavior change. Module contents are
byte-identical to pre-move.

Verified: cargo fmt --all; cargo clippy --all --benches --tests
--examples --all-features clean; python3 scripts/check_no_panics.py
clean; cargo check --all-features --all-targets clean.

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

* refactor(gateway): extract OAuth / relay callbacks into features/oauth/ — ironclaw#2599 stage 4a

Fourth increment of the ironclaw#2599 platform/feature split. Opens
the `features/` subtree with the OAuth feature slice — the first
vertical slice to move out of server.rs into its own module under
the ironclaw#2599 target layout.

Slice contents:

- `features/oauth/mod.rs` owns the three public gateway routes
  that receive OAuth-style callbacks:
  * `oauth_callback_handler` — generic OAuth callback for
    installable extensions (CSRF lookup, token exchange, storage,
    optional auto-activation).
  * `relay_events_handler` — HMAC-signed webhook from channel-relay.
  * `slack_relay_oauth_callback_handler` — Slack-specific relay
    completion flow.
- Slice-private helpers `oauth_error_page` and
  `redact_oauth_state_for_logs` move with the slice (they have no
  other callers).

Wiring:

- `platform/router.rs` imports the three handlers from
  `features::oauth` instead of `server`; no route-table change.
- `channels/web/mod.rs` registers `pub(crate) mod features;`.
- `server.rs` loses the three handlers and their helpers, plus the
  imports they owned (`Sha256`, `Digest`, `HeaderMap`,
  `DEFAULT_RELAY_NAME`, `extension_name_candidates`,
  `SecretConsumeResult`). The test module re-imports the ones it
  still uses for the integration-level OAuth callback tests.

Pure move. No behavior change. Each handler body is byte-identical
to its pre-move counterpart. Every test in `server.rs` that exercises
the OAuth callbacks (`test_oauth_callback_missing_params`, etc.)
continues to pass against the re-imported handlers.

Stats: server.rs 6973 → 6248 lines (−725); new `features/oauth/mod.rs`
is 775 lines; new `features/mod.rs` 14 lines. The +30 delta is
comment headers documenting the slice boundary.

Verified: `cargo fmt --all`;
`cargo clippy --all --benches --tests --examples --all-features`
clean; `python3 scripts/check_no_panics.py` clean;
`cargo test --lib` 5069 passed (one more than stage 3 — the new
`css_handler_returns_base_in_multi_tenant_mode` test from staging
lands green), same 2 pre-existing failures carried over (fixture
and test-infra issues unrelated to gateway layout).

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

* ci(gateway): enforce platform/feature boundaries — ironclaw#2599 stage 5

Adds `scripts/check_gateway_boundaries.py` and wires it into the
`code_style` CI workflow as a required check. The script enforces the
ironclaw#2599 layering rule: every file under `src/channels/web/platform/`
except `router.rs` must not import from `handlers/` or `features/`.

How it works:

- Walks `src/channels/web/platform/*.rs`, skipping `router.rs` (the
  intentional composition point) and test modules.
- Strips line comments, block comments, and string / raw-string / char
  literals so references inside docstrings and explanatory text don't
  trigger false positives.
- Matches six forbidden import shapes:
  `crate::channels::web::{handlers,features}::`,
  `super::{handlers,features}::`,
  `super::super::{handlers,features}::`.
- Prints diagnostics with file:line and the matched pattern for every
  violation; exits non-zero on any.
- Carries unit tests behind a `test` subcommand
  (`python3 scripts/check_gateway_boundaries.py test`) that the CI
  job runs alongside the check itself.

Simultaneous fix: one pre-existing back-edge that the check surfaced
was the OIDC `check_email_domain()` helper living in
`handlers/auth.rs` but called from `platform/auth.rs`. The helper is
platform-level (it gates JWT validation before any handler runs), so
it moves into `platform::auth` along with its five unit tests; the
handler call site in `handlers::auth::handle_callback` now imports
from the new home. No behavior change.

The second pre-existing back-edge is the frontend bundle assembly
path: `platform/static_files::build_frontend_html` calls
`read_layout_config` and `load_resolved_widgets`, both still in
`handlers/frontend.rs`. Migrating them requires also moving
`read_widget_manifest` and the widget-size constants, which touches
`load_widget_manifests` (used by `/api/frontend/widgets` and the
engine-v2 widget endpoint). That's a separate focused PR — tracked
via a narrow allowlist entry in the script with a follow-up comment.
The allowlist is explicitly documented as "must not grow without
reviewer sign-off".

CLAUDE.md's "Platform vs. feature layering" section now names the
script as the enforcement point.

Verified: `python3 scripts/check_gateway_boundaries.py test` — 9
tests pass; `python3 scripts/check_gateway_boundaries.py` — clean;
`cargo fmt --all`; `cargo clippy --all --benches --tests --examples
--all-features` clean; `python3 scripts/check_no_panics.py` clean;
`cargo test --lib channels::web` — 425 passed.

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

* ci(gateway): close boundary-checker bypasses — PR #2647 review

Four issues raised on PR #2647's review are addressed:

- Grouped `use crate::channels::web::{ handlers::... }` imports escape
  the per-line scan because the forbidden segment lands on a
  continuation line. Adds a multiline GROUPED_FORBIDDEN_PATTERN that
  matches across newlines and reports the line where `handlers::`,
  `features::`, or `server::` actually appears.
- `use crate::channels::web::server::...` routes through the
  `server.rs` compatibility shim and still creates a platform →
  feature back-edge. Adds `server::` (and its `super::` variants) to
  FORBIDDEN_PATTERNS. Existing pre-existing shim usage in
  `platform/ws.rs` is captured as a tracked allowlist entry — the
  allowlist shrinks as individual types migrate out of `server.rs`.
- `#[cfg(test)] mod ...` and `mod tests { ... }` bodies are now
  actually blanked before pattern matching, matching the docstring's
  stated exemption. Caller-level regression tests in platform files
  can import handler/feature modules without tripping the check.
- `gateway-boundaries` is no longer gated solely on `has_code`. A new
  `has_boundary_check` output on the `changes` job fires when the
  checker script or this workflow itself changes, so PRs that only
  edit `scripts/check_gateway_boundaries.py` or
  `.github/workflows/code_style.yml` still run the guardrail.

Also picks up a small perf nit: `text.splitlines()` is now computed
once outside the loop instead of per-violation.

Regression tests cover each case (grouped crate-web import, grouped
super import, server-shim back-edge, cfg(test)/mod tests skip, and a
sanity check that the test-module skip doesn't blanket-ignore the
rest of the file).

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

* ci(gateway): brace-aware grouped scan + narrower ws.rs allowlist — PR #2647 Copilot review

Two issues raised by Copilot on the round-1 fixes:

- `GROUPED_FORBIDDEN_PATTERN` used `[^{}]*?` and so could not match
  grouped imports that contain *nested* braces — e.g.
  `use crate::channels::web::{ platform::{state::GatewayState},
  handlers::auth::login_handler };` produced zero violations even
  though the forbidden segment is plainly inside the web::{...}
  group. Replaced the regex with a depth-tracking walk: find each
  `crate::channels::web::{` / `super::{` / `super::super::{` header,
  find the matching `}` by counting braces (`{` / `}` only; string
  and comment contents are already blanked), then scan the body for
  `(handlers|features|server)::`. Report line numbers off absolute
  offsets so the reported line is where the forbidden segment lives,
  not where the header's `{` is.

- `ws.rs`'s allowlist entry whitelisted the whole
  `crate::channels::web::server::` prefix, which would let any *new*
  accidental server-shim import in ws.rs silently pass. Narrowed to
  seven per-symbol entries covering the current pre-existing uses
  (GatewayState, PerUserRateLimiter, RateLimiter,
  ActiveConfigSnapshot, images_to_attachments, and the two
  handle_legacy_auth_* helpers). Future accidental shim imports fail
  the check and require explicit reviewer sign-off to add.

Added `test_detects_nested_brace_grouped_import` as the regression
test for the brace-aware scanner.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@henrypark133 henrypark133 mentioned this pull request Apr 21, 2026
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
…form/router.rs — ironclaw#2599 stage 2 (nearai#2643)

* refactor(gateway): extract start_server and route composition into platform/router.rs — ironclaw#2599 stage 2

Second increment of the ironclaw#2599 platform/feature split. Moves
`start_server()` and the Axum route composition out of `server.rs` into
a dedicated `platform/router.rs`, so the platform-vs-features
dependency direction is visible: the router depends on handler modules
(both `handlers/*` and the still-inline handlers in `server.rs`),
never the reverse.

Changes:

- New `src/channels/web/platform/router.rs` owns `start_server()`, the
  four routers (`public`, `protected`, `statics`, `projects`), and the
  cross-cutting layer stack (CORS, 10 MB body limit, panic catch,
  `X-Content-Type-Options`, `X-Frame-Options`, CSP).
- Feature handlers still inline in `server.rs` are now `pub(crate)` so
  the router can register them without leaking them outside the crate.
  Two private structs that are directly referenced by pub(crate)
  handlers (`HistoryQuery`, `GatewayStatusResponse`) were also raised
  to `pub(crate)` so the handler signatures type-check from the router
  module.
- `server.rs` keeps the feature handlers that haven't migrated yet
  (OAuth callbacks, chat, extensions, pairing, logs, gateway status)
  and adds `pub use platform::router::start_server` so external call
  sites — `src/channels/web/mod.rs`, `tests/multi_tenant_integration.rs`
  — keep working. The trimmed imports drop `Router`, `DefaultBodyLimit`,
  `CorsLayer`, `tokio::sync::mpsc`, etc., since they're no longer used
  in the remaining body.
- `mod.rs` now calls `platform::router::start_server` directly; the
  `server::start_server` shim exists only for external code paths that
  still reach for it.
- `CLAUDE.md` file map now lists `platform/router.rs` and clarifies
  that `server.rs` is feature-handler-only pending migration.

No behavior change. Route table, middleware stack, CORS policy, body
limits, panic handling, security headers, and CSP are byte-identical
to origin/staging.

Stats: server.rs 7463 → 6973 lines (−490); new `platform/router.rs` is
537 lines. `cargo clippy --all --benches --tests --examples
--all-features` is clean. Unit test run: 5068 passed; the same 2
pre-existing failures carried over from stage 1
(`pairing::approval::tests::propagate_approval_restores_runtime_state_when_on_start_fails`
needs a telegram WASM fixture;
`extensions::manager::tests::test_telegram_token_colon_preserved_in_validation_url`
has a test-infra URL override — neither references any symbol this
PR touches).

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

* fix(gateway): address PR nearai#2643 review — proper error handling in router CORS + doc reconciliation

Three fixes for review comments on nearai#2643:

1. **router.rs CORS origin parsing (comment #3104968733, CI `no-panics`):**
   Replaced `.expect("valid origin")` with `.map_err(|e| ChannelError::StartupFailed { .. })`
   so a malformed bound address fails the bootstrap with a semantically
   specific error instead of panicking. Also switched from
   `format!("http://{}:{}", addr.ip(), addr.port())` to
   `format!("http://{addr}")` — `SocketAddr`'s `Display` brackets IPv6
   addresses correctly (`[::1]:8080` rather than the ambiguous
   `::1:8080`), so the CORS origin stays valid on v6 binds. Fixes the
   `No panics in production code` CI check failure and the
   `Code Style` composite check that depends on it.

2. **platform/mod.rs docstring (comment #3104970223):** The previous
   "feature handlers depend on platform, not the other way around"
   wording conflicted with `router` importing feature handlers. Rewrote
   to name `router` as the single, intentional exception to the
   no-back-edges rule, and explicitly list the platform submodules the
   rule still applies to (`state`, `static_files`, future `auth`/`sse`/`ws`).

3. **CLAUDE.md layering section (comment #3104970232):** Same
   contradiction — rewrote to reconcile: router is the coupling point;
   every other platform submodule must stay handler-agnostic; the
   forthcoming CI check (ironclaw#2599 stage 5) enforces forbidden
   imports between `platform/{state,static_files,auth,sse,ws}.rs` and
   `handlers/*` / `features/*` while explicitly allowing
   `platform/router.rs` to reference both sides.

Verified: `python3 scripts/check_no_panics.py` clean;
`cargo clippy --all --benches --tests --examples --all-features`
clean; `cargo test --lib channels::web::server::tests::test_` passes
30 server-module tests including CORS / CSP / start_server smoke
coverage.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
…#2599 stage 3 (nearai#2644)

Third increment of the ironclaw#2599 platform/feature split (follow-up
to nearai#2628 and nearai#2643). Moves the three transport / framing modules into
the platform/ subtree so the platform layer now contains the full set
of cross-cutting infrastructure (state, router, static_files, auth,
sse, ws).

Changes:

- src/channels/web/auth.rs  -> src/channels/web/platform/auth.rs
- src/channels/web/sse.rs   -> src/channels/web/platform/sse.rs
- src/channels/web/ws.rs    -> src/channels/web/platform/ws.rs
- platform/mod.rs declares the three new submodules.
- channels/web/mod.rs adds backward-compat re-exports
  (`pub use platform::{auth, sse, ws};`) so every existing
  `crate::channels::web::{auth,sse,ws}::...` call site - roughly 40
  files across handlers, tests, integration tests, and sibling
  modules - continues to resolve without edits. Follow-up PRs will
  migrate call sites to the canonical `platform::` path incrementally.
- platform/mod.rs doc comment now describes the platform layer as
  having auth / SSE / WS (no longer "in later stages of nearai#2599").
- CLAUDE.md file map points at the new paths and notes the re-exports.

Pure move + re-export. No behavior change. Module contents are
byte-identical to pre-move.

Verified: cargo fmt --all; cargo clippy --all --benches --tests
--examples --all-features clean; python3 scripts/check_no_panics.py
clean; cargo check --all-features --all-targets clean.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
…e 5 (nearai#2647)

* refactor(gateway): relocate auth / sse / ws into platform/ — ironclaw#2599 stage 3

Third increment of the ironclaw#2599 platform/feature split (follow-up
to nearai#2628 and nearai#2643). Moves the three transport / framing modules into
the platform/ subtree so the platform layer now contains the full set
of cross-cutting infrastructure (state, router, static_files, auth,
sse, ws).

Changes:

- src/channels/web/auth.rs  -> src/channels/web/platform/auth.rs
- src/channels/web/sse.rs   -> src/channels/web/platform/sse.rs
- src/channels/web/ws.rs    -> src/channels/web/platform/ws.rs
- platform/mod.rs declares the three new submodules.
- channels/web/mod.rs adds backward-compat re-exports
  (`pub use platform::{auth, sse, ws};`) so every existing
  `crate::channels::web::{auth,sse,ws}::...` call site - roughly 40
  files across handlers, tests, integration tests, and sibling
  modules - continues to resolve without edits. Follow-up PRs will
  migrate call sites to the canonical `platform::` path incrementally.
- platform/mod.rs doc comment now describes the platform layer as
  having auth / SSE / WS (no longer "in later stages of nearai#2599").
- CLAUDE.md file map points at the new paths and notes the re-exports.

Pure move + re-export. No behavior change. Module contents are
byte-identical to pre-move.

Verified: cargo fmt --all; cargo clippy --all --benches --tests
--examples --all-features clean; python3 scripts/check_no_panics.py
clean; cargo check --all-features --all-targets clean.

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

* refactor(gateway): extract OAuth / relay callbacks into features/oauth/ — ironclaw#2599 stage 4a

Fourth increment of the ironclaw#2599 platform/feature split. Opens
the `features/` subtree with the OAuth feature slice — the first
vertical slice to move out of server.rs into its own module under
the ironclaw#2599 target layout.

Slice contents:

- `features/oauth/mod.rs` owns the three public gateway routes
  that receive OAuth-style callbacks:
  * `oauth_callback_handler` — generic OAuth callback for
    installable extensions (CSRF lookup, token exchange, storage,
    optional auto-activation).
  * `relay_events_handler` — HMAC-signed webhook from channel-relay.
  * `slack_relay_oauth_callback_handler` — Slack-specific relay
    completion flow.
- Slice-private helpers `oauth_error_page` and
  `redact_oauth_state_for_logs` move with the slice (they have no
  other callers).

Wiring:

- `platform/router.rs` imports the three handlers from
  `features::oauth` instead of `server`; no route-table change.
- `channels/web/mod.rs` registers `pub(crate) mod features;`.
- `server.rs` loses the three handlers and their helpers, plus the
  imports they owned (`Sha256`, `Digest`, `HeaderMap`,
  `DEFAULT_RELAY_NAME`, `extension_name_candidates`,
  `SecretConsumeResult`). The test module re-imports the ones it
  still uses for the integration-level OAuth callback tests.

Pure move. No behavior change. Each handler body is byte-identical
to its pre-move counterpart. Every test in `server.rs` that exercises
the OAuth callbacks (`test_oauth_callback_missing_params`, etc.)
continues to pass against the re-imported handlers.

Stats: server.rs 6973 → 6248 lines (−725); new `features/oauth/mod.rs`
is 775 lines; new `features/mod.rs` 14 lines. The +30 delta is
comment headers documenting the slice boundary.

Verified: `cargo fmt --all`;
`cargo clippy --all --benches --tests --examples --all-features`
clean; `python3 scripts/check_no_panics.py` clean;
`cargo test --lib` 5069 passed (one more than stage 3 — the new
`css_handler_returns_base_in_multi_tenant_mode` test from staging
lands green), same 2 pre-existing failures carried over (fixture
and test-infra issues unrelated to gateway layout).

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

* ci(gateway): enforce platform/feature boundaries — ironclaw#2599 stage 5

Adds `scripts/check_gateway_boundaries.py` and wires it into the
`code_style` CI workflow as a required check. The script enforces the
ironclaw#2599 layering rule: every file under `src/channels/web/platform/`
except `router.rs` must not import from `handlers/` or `features/`.

How it works:

- Walks `src/channels/web/platform/*.rs`, skipping `router.rs` (the
  intentional composition point) and test modules.
- Strips line comments, block comments, and string / raw-string / char
  literals so references inside docstrings and explanatory text don't
  trigger false positives.
- Matches six forbidden import shapes:
  `crate::channels::web::{handlers,features}::`,
  `super::{handlers,features}::`,
  `super::super::{handlers,features}::`.
- Prints diagnostics with file:line and the matched pattern for every
  violation; exits non-zero on any.
- Carries unit tests behind a `test` subcommand
  (`python3 scripts/check_gateway_boundaries.py test`) that the CI
  job runs alongside the check itself.

Simultaneous fix: one pre-existing back-edge that the check surfaced
was the OIDC `check_email_domain()` helper living in
`handlers/auth.rs` but called from `platform/auth.rs`. The helper is
platform-level (it gates JWT validation before any handler runs), so
it moves into `platform::auth` along with its five unit tests; the
handler call site in `handlers::auth::handle_callback` now imports
from the new home. No behavior change.

The second pre-existing back-edge is the frontend bundle assembly
path: `platform/static_files::build_frontend_html` calls
`read_layout_config` and `load_resolved_widgets`, both still in
`handlers/frontend.rs`. Migrating them requires also moving
`read_widget_manifest` and the widget-size constants, which touches
`load_widget_manifests` (used by `/api/frontend/widgets` and the
engine-v2 widget endpoint). That's a separate focused PR — tracked
via a narrow allowlist entry in the script with a follow-up comment.
The allowlist is explicitly documented as "must not grow without
reviewer sign-off".

CLAUDE.md's "Platform vs. feature layering" section now names the
script as the enforcement point.

Verified: `python3 scripts/check_gateway_boundaries.py test` — 9
tests pass; `python3 scripts/check_gateway_boundaries.py` — clean;
`cargo fmt --all`; `cargo clippy --all --benches --tests --examples
--all-features` clean; `python3 scripts/check_no_panics.py` clean;
`cargo test --lib channels::web` — 425 passed.

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

* ci(gateway): close boundary-checker bypasses — PR nearai#2647 review

Four issues raised on PR nearai#2647's review are addressed:

- Grouped `use crate::channels::web::{ handlers::... }` imports escape
  the per-line scan because the forbidden segment lands on a
  continuation line. Adds a multiline GROUPED_FORBIDDEN_PATTERN that
  matches across newlines and reports the line where `handlers::`,
  `features::`, or `server::` actually appears.
- `use crate::channels::web::server::...` routes through the
  `server.rs` compatibility shim and still creates a platform →
  feature back-edge. Adds `server::` (and its `super::` variants) to
  FORBIDDEN_PATTERNS. Existing pre-existing shim usage in
  `platform/ws.rs` is captured as a tracked allowlist entry — the
  allowlist shrinks as individual types migrate out of `server.rs`.
- `#[cfg(test)] mod ...` and `mod tests { ... }` bodies are now
  actually blanked before pattern matching, matching the docstring's
  stated exemption. Caller-level regression tests in platform files
  can import handler/feature modules without tripping the check.
- `gateway-boundaries` is no longer gated solely on `has_code`. A new
  `has_boundary_check` output on the `changes` job fires when the
  checker script or this workflow itself changes, so PRs that only
  edit `scripts/check_gateway_boundaries.py` or
  `.github/workflows/code_style.yml` still run the guardrail.

Also picks up a small perf nit: `text.splitlines()` is now computed
once outside the loop instead of per-violation.

Regression tests cover each case (grouped crate-web import, grouped
super import, server-shim back-edge, cfg(test)/mod tests skip, and a
sanity check that the test-module skip doesn't blanket-ignore the
rest of the file).

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

* ci(gateway): brace-aware grouped scan + narrower ws.rs allowlist — PR nearai#2647 Copilot review

Two issues raised by Copilot on the round-1 fixes:

- `GROUPED_FORBIDDEN_PATTERN` used `[^{}]*?` and so could not match
  grouped imports that contain *nested* braces — e.g.
  `use crate::channels::web::{ platform::{state::GatewayState},
  handlers::auth::login_handler };` produced zero violations even
  though the forbidden segment is plainly inside the web::{...}
  group. Replaced the regex with a depth-tracking walk: find each
  `crate::channels::web::{` / `super::{` / `super::super::{` header,
  find the matching `}` by counting braces (`{` / `}` only; string
  and comment contents are already blanked), then scan the body for
  `(handlers|features|server)::`. Report line numbers off absolute
  offsets so the reported line is where the forbidden segment lives,
  not where the header's `{` is.

- `ws.rs`'s allowlist entry whitelisted the whole
  `crate::channels::web::server::` prefix, which would let any *new*
  accidental server-shim import in ws.rs silently pass. Narrowed to
  seven per-symbol entries covering the current pre-existing uses
  (GatewayState, PerUserRateLimiter, RateLimiter,
  ActiveConfigSnapshot, images_to_attachments, and the two
  handle_legacy_auth_* helpers). Future accidental shim imports fail
  the check and require explicit reviewer sign-off to add.

Added `test_detects_nested_brace_grouped_import` as the regression
test for the brace-aware scanner.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: channel/web Web gateway channel scope: docs Documentation size: XL 500+ changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants