OSPP_PR1:Add request-scoped context support for the Agent Gateway plugin - #7133

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Review (PMC Aias00) — OSPP_PR1: Add request-scoped context support for the Agent Gateway plugin

I am leaving this as a comment, not a formal approve/request-changes, because the PR is still a draft and the description says the mentor version has not been merged yet. But there are two hard blockers below that will break a fresh install, plus a few design points worth settling before this leaves draft.

🔴 Blocking — rebase required, the SQL IDs now collide with master

The branch was cut around 2026-09-19; master has since landed the sensitiveWord plugin (2026-09-21), which consumed exactly the ID range this PR allocates. Verified against current master:

1. plugin.id = '67' is already taken. All six init scripts insert
   ('67', 'agentGateway', ... 'Ai', 198 ...), but master already has

   INSERT INTO `plugin` VALUES ('67', 'sensitiveWord', null, 'Ai', 197, ...);

   → duplicate primary key, schema init fails on a fresh install. Please move to the next free id (68) in all six files (db/init/mysql|ob|og|oracle|pg + shenyu-admin/.../h2/schema.sql).

2. resource ids 1942847622591684620 / …621 / …622 are already used. Master has

   INSERT INTO `plugin_handle` VALUES ('1942847622591684620', '67', 'failClosed', ...);
   INSERT INTO `plugin_handle` VALUES ('1942847622591684621', '67', 'words', ...);
   INSERT INTO `plugin_handle` VALUES ('1942847622591684622', '67', 'maxBodySize', ...);

   This PR reuses the same three values for its resource rows → duplicate primary key. Re-allocate, and please re-verify the permission ids 1942847622591684630–639 against master as well while you are there.

Good news: plugin code 198 itself is free and slots correctly immediately before AI_PROXY(199, ...), so the enum addition is fine as-is.

🟠 Should fix before leaving draft

3. trafficType is hard-restricted to "LLM". AgentGatewayRuleHandleParser returns invalid("trafficType must be LLM") for anything else, and an invalid handle becomes a 503 on live traffic. The PR claims MCP coexistence and only lists "full MCP JSON-RPC tool workflow" as out of scope — but as written the plugin cannot even be configured for MCP later without another code change. Either open the enum now (LLM, MCP, …) or state plainly in the README that PR1 supports LLM only.
4. Unknown fields reject live traffic. Any key outside {trafficType, responseRequestId} → invalid handle → 503. Strict validation is right at config-save time, but failing requests because a stored handle carries an extra key is fragile (admin schema evolution, hand-edited JSON, copy-paste from docs). Suggest logging a warning and ignoring unknown fields on the request path, keeping the strict check in admin.
5. 503 is the wrong status for a configuration error. reject() sets SERVICE_UNAVAILABLE, which tells clients and upstream proxies to retry — but a bad rule handle will never start working. Use 500 (server-side misconfiguration) or 400, and reserve 503 for real unavailability. The AGENT_GATEWAY_CONFIG_INVALID code prefix is a nice touch.
6. selector.getContinued() == FALSE → 503. continued=false conventionally means "stop the chain after this plugin", not "misconfigured". Failing the request here is surprising. Consider just proceeding, or document the constraint clearly.
7. The AtomicBoolean subscribed guard is risky.

   if (!subscribed.compareAndSet(false, true)) {
       return Mono.error(new IllegalStateException("...cannot be subscribed twice"));
   }

   WebFlux subscribes once, but this makes the returned Mono non-idempotent, so any retry, repeat, cache() or re-subscribing composition turns into a hard error. If double subscription is not a real scenario, drop the guard; if it is, handle it without failing the request.

🟡 Non-blocking

8. resolveHandle re-parses JSON on every request when the handle is invalid (invalid results are never cached). Consider caching the invalid handle too, keyed by the raw string.
9. restorePreviousContext relies on Map#remove(k, v) / Map#replace(k, old, new). DefaultServerWebExchange backs attributes with a ConcurrentHashMap so it works today, but those default methods throw UnsupportedOperationException on other Map implementations — worth a comment stating the assumption.
10. AgentTrafficContext has no toString(); adding one (requestId + trafficType + selector/rule) makes correlation in logs much easier.
11. The Verification section says GitHub CI has not been run. Apache requires a green CI run before merge — please make sure the full matrix passes (and rerun after the rebase).

✅ What looks good

• AgentTrafficContext is genuinely immutable, created per subscription, and never stored in a global map or thread local — real request isolation, and the test asserting 12 distinct request ids proves it.
• contextWrite is applied to chain.execute(exchange), so downstream plugins see the context — correct Reactor usage, and the test asserts exchange context == reactor context.
• doFinally restores/removes the previous attribute, including on cancel — good cleanup hygiene.
• AgentGatewayPluginDataHandler cleans the handle cache on removeRule, removeSelector (prefix scan) and removePlugin — no leak on config removal.
• Response correlation via beforeCommit + X-Shenyu-Agent-Request-Id, and deliberately not trusting a client-supplied id.
• 26 tests across plugin / parser / data handler / starter config, covering isolation, restore, cancellation, rejection and the response-header opt-in.

Solid foundation — fix the ID collisions and the status-code semantics and this is in good shape.

The runtime context/plugin implementation and targeted tests look sound, but fresh-install schemas are the only place that seed agentGateway. None of the mysql/ob/og/oracle/pg 2.7.1-to-2.7.2 upgrade scripts add the plugin, handles, resources, permissions, or namespace relation. Existing upgraded installations therefore cannot expose/configure the plugin. Please add dialect-appropriate upgrade migrations and a migration/schema consistency check.

Re-reviewed after the new commits, and my previous concern is fully addressed: the upgrade path now seeds the Agent Gateway data too. All five dialects' 2.7.1-upgrade-2.7.2-*.sql insert the plugin row, plugin handles, resources and permissions, and on top of that you added check-agent-gateway-migrations.py (plus its test) to assert seed parity between the fresh-install schemas and the upgrade scripts across dialects — that is a better guarantee than what I asked for.

The plugin itself (context, rule handle + parser + validator, configuration, plugin data handler) has substantial test coverage, and the migration check is wired into CI. Approving.