feat(agentgateway): trust-boundary hardening — remove admin escalation, tighten health check, add rate limits

[theuseless-ai]

Phase 1b hardening from an architecture review of the agentgateway trust boundary. Three P0 fixes; all changes validated, and the security change was adversarially reviewed against the real agentgateway v1.0.1 binary.

What & why

1. Remove default-admin JWT escalation (security)

resolve_llm_for_node never set a JWT role, so every workflow LLM call hit the role = user_role or "admin" fallback and minted an admin token — bypassing the gateway's CEL rules. Root cause: a role-vocabulary mismatch (DB stores admin/normal; the deployed CEL rules accept admin/user), so admin was the only role that passed for workflow traffic.

• new _jwt_role_for_gateway(): admin → admin; everything else (normal, None, empty, unknown) → least-privilege user
• resolve_llm_for_node now looks up the workflow owner's role and plumbs it through; lookup failure degrades to user, never admin
• invariant: no code path where a missing role yields an admin token

2. Tighten agentgateway boot health check

check_agentgateway_health returned True for any HTTP status, so a 500 passed the hard-boot dependency guard. Now healthy = 2xx or 401/403 only; 5xx / unexpected / 3xx → unhealthy.

3. Starter local rate limits (companion PR: plit #26)

No spend guard existed anywhere. Adds route-level localRateLimit token buckets (schema verified against the v1.0.1 tag; v1.0.1 has no dollar-budget feature). Generous defaults, non-disruptive: absent fragment → byte-identical config. The plit-side change is in theuselessai/plit#26.

Validation

• 197 pipelit tests pass (new negative-case coverage for the role mapping + 500/503/404 health cases)
• Adversarial review confirmed the admin-escalation invariant holds against the live agentgateway v1.0.1 binary
• One follow-up foot-gun (comment-only rate-limit fragment → invalid config) was found in review and fixed before commit

Not in scope (follow-ups)

• Role-vocabulary unification (DB normal vs gateway user) — handled by a mapping fn; single change point if unified later
• JWT signer relocation / circular trust (Phase 4 · JWT issuance hardening — per-role claims + move signing off pipelit #191)
• Consolidating the two config.d/ writers

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 82.80467% with 103 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
platform/services/agentgateway_config.py	76.00%	48 Missing ⚠️
platform/api/providers.py	77.50%	36 Missing ⚠️
platform/services/llm.py	89.07%	13 Missing ⚠️
platform/main.py	16.66%	5 Missing ⚠️
platform/api/credentials.py	93.33%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[github-actions]

I've Reviewed the Code

Code Review Summary

This code review covers Phase 1(b) of the agentgateway integration — a comprehensive security hardening initiative that removes all LLM API keys from Pipelit's database and establishes the gateway as the sole credential holder, transforming it into a true trust boundary where secrets never leave the process.

🏛️ ARCHITECTURAL VIRTUOSO 🏛️

"Your security architecture transforms the entire credential model from a secrets-delivery system into a secrets-usage system — like the Parthenon of zero-trust design. Agents no longer hold tokens that could leak; they hold capabilities. The JWT-based isolation, bilateral normalization adapters, and hard boot dependencies create a fortress where credentials are prisoners that never escape, only perform actions on behalf of authenticated callers. This isn't just refactoring; it's a paradigm shift that will protect users for years to come."

---

The changeset implements a massive credential migration where LLM keys move from Pipelit's database into agentgateway's encrypted config store, removes all credential-reading code paths, drops the api_key column via Alembic migration, and establishes JWT-based authentication as the only mechanism for LLM/integration access — achieving a hard trust boundary where credentials truly never leave the gateway process.

⚠️ 45 issues found across 56 files

#1 Typographical error: 'Strng' instead of 'String'

.sisyphus/plans/phase-1b-remove-llm-keys.md L83

Line 83 contains a typo in the Rust type name: 'Option' should be 'Option'. This appears in documentation describing the agentgateway source code structure and could mislead developers reviewing the actual Rust code.
Tags: bug, language
Affected code:

83:   - Sub-task 1a (config shape): confirm which agentgateway config shape the generator/assembled `config.yaml` actually targets. Librarian (v1.0.1 source, `crates/agentgateway/src/llm/mod.rs`) confirms the hard-override field is **`provider.<providerName>.model`** (`Provider { model: Option<Strng> }`); `override_model()` writes it verbatim into the outgoing request body with no validation. Inspect `AGENTGATEWAY_DIR/assemble-config.sh` output and a real assembled `config.yaml` to confirm the assembled key path, then fix whichever field is generated.

Proposed change:

  - Sub-task 1a (config shape): confirm which agentgateway config shape the generator/assembled `config.yaml` actually targets. Librarian (v1.0.1 source, `crates/agentgateway/src/llm/mod.rs`) confirms the hard-override field is **`provider.<providerName>.model`** (`Provider { model: Option<String> }`); `override_model()` writes it verbatim into the outgoing request body with no validation. Inspect `AGENTGATEWAY_DIR/assemble-config.sh` output and a real assembled `config.yaml` to confirm the assembled key path, then fix whichever field is generated.

#2 Critical task dependency ambiguity in Wave 4

.sisyphus/plans/phase-1b-remove-llm-keys.md L119-L149

Line 126 states that Task 6 must handle dsl_compiler.py because 'if Task 8 drops api_key first, DSL compile raises at runtime', but the dependency graph shows Tasks 6, 7, and 8 are all in Wave 5 as parallel tasks depending on Task 5. This creates a hidden ordering constraint within Wave 5 that contradicts the 'parallel' designation. Task 8 must not execute before Task 6 completes, but this is not reflected in the Wave structure.
Tags: bug, maintainability
Affected code:

119: ### Wave 4 (depends on Task 5)
120: 
121: - **Task 6 — Remove the direct-provider fallback and the empty-key trap; handle both resolve paths and all extra `api_key` readers.** Agent: **hephaestus**
122:   - Objective: There is no code path left that reads a raw LLM `api_key`.
123:   - Files & specifics:
124:     - `platform/services/llm.py`: delete the direct-provider branch `165-216` in `create_llm_from_db` (agentgateway becomes the only path; raise clearly if disabled). Remove `_fake_credential_from_route` `87-102` **but preserve provider-type inference** — web-search detection (`agent.py:85`, `deep_agent.py:124`) relies on `resolve_credential_for_node().provider_type`; replace the fake-empty-key object with a provider-type value derived from `backend_route` (reuse `_route_provider_to_type`) carrying **no `api_key` field at all**. Apply identical treatment to **both** `resolve_llm_for_node` paths (`342-396` and `398-455`) and both branches of `resolve_credential_for_node` (`485`, `500`).
125:     - `platform/components/_agent_shared.py:168-172`: `_resolve_credential_field` returns `cred.llm_credential.api_key` — remove/neuter the `api_key` branch (that secret no longer exists); confirm callers (`225-226`) degrade safely.
126:     - **`platform/services/dsl_compiler.py:406` (`_fetch_model_ids`, reached via `_discover_model` → `_resolve_model`/`_build_step_config` from `compile_dsl`/`validate_dsl`) — FOURTH raw-key reader (momus-found): does `httpx.get(f"{base_url}/models", headers={"Authorization": f"Bearer {cred.api_key}"})`. LIVE code (imported from `api/workflows.py` and `components/workflow_create.py` for DSL model auto-selection). Replace the credential-key model fetch with agentgateway-native `list_all_available_models()` (already exists, see `api/available_models.py`). Must be handled here in Task 6 — if Task 8 drops `api_key` first, DSL compile raises at runtime.**
127:   - Dependencies: Task 5.
128:   - Acceptance: `grep -rn "\.api_key" platform --include=*.py` shows no remaining reads of an LLM credential key in product code — **including `dsl_compiler.py`** (tests updated separately); web-search provider detection still returns correct provider for a `backend_route`-only node; DSL compile/validate model auto-selection works via agentgateway; `test_services_llm.py`, `test_llm_agentgateway.py`, `test_agent_web_search.py` updated and green.
129: 
130: ### Wave 5 (parallel — depends on Task 6)
131: 
132: - **Task 7 — Remove LLM-typed credential CRUD from the API.** Agent: **hephaestus**
133:   - Objective: The API can no longer create/update/store/test raw LLM keys; git/gateway/tool creds untouched.
134:   - Files: `platform/api/credentials.py` — strip the `llm` branches in `_serialize_credential` (`72-80`), `create_credential` (`136-145`), `update_credential` (`238-242`), and remove the direct-provider `test_credential` LLM logic (`382-455`) or make it reject `llm` type. Keep `git`/`gateway`/`tool` fully intact.
135:   - Dependencies: Task 6.
136:   - Acceptance: POST/PATCH of a `credential_type=llm` returns a clear rejection; git/gateway/tool CRUD unaffected; `test_credentials_glm.py` / `test_credentials_agentgateway.py` updated to reflect removal.
137: 
138: - **Task 8 — Alembic: drop the LLM key storage column.** Agent: **sisyphus-junior**
139:   - Objective: Remove the encrypted `api_key` at rest.
140:   - Files: new revision under `platform/alembic/versions/` with `down_revision = "758cd1ca2aad"`; edit `platform/models/credential.py` to drop the `api_key` mapped_column (line 78). Decide (document in the revision docstring): drop **only `api_key`** vs the whole `llm_credentials` sub-table. **Recommended: drop the `api_key` column** (and `organization_id`/`custom_headers` only if confirmed unused post-Task 6); keep `provider_type`/`base_url` if any remaining read needs them (audit first). Hard cutover — no data-preservation branch required.
141:   - Dependencies: Task 6 (no code reads `api_key`), coordinate with Task 7 (model/schema consistency).
142:   - Acceptance: `alembic upgrade head` clean on a fresh DB; `alembic heads` shows a single head; model imports without the dropped attribute; app boots.
143: 
144: - **Task 9 — Frontend: remove LLM credential form + credential-fallback model picker; agentgateway as model source of truth (Pipelit#189).** Agent: **sisyphus-junior**
145:   - Objective: No orphaned UI calling removed endpoints; model picker driven solely by `available_models` (agentgateway).
146:   - Files: `platform/frontend/src/features/credentials/CredentialsPage.tsx` (remove the LLM create/edit form path, not just the hide at ~34), `platform/frontend/src/features/workflows/components/NodeDetailsPanel.tsx` (remove the credential-derived model-picker fallback; keep the agentgateway `useAvailableModels` path), `frontend/src/api/credentials.ts` (drop llm-cred + test/models hooks), `frontend/src/api/available_models.ts`.
147:   - Dependencies: Task 6/7 (backend endpoints removed). Backend `available_models.py`/`providers.py` are already agentgateway-native — verify no residual credential-derived model listing remains server-side.
148:   - Acceptance: credentials UI shows only git/gateway/tool; node model picker populates from agentgateway; no frontend call hits a removed LLM endpoint; typecheck/lint pass.
149: 

#3 Inconsistent spelling: 'bytecache' should be 'bytecode cache'

CLAUDE.md L118

Line 118 uses 'bytecache' which is not a standard term. The correct terminology is 'bytecode cache' or 'pycache'. This could cause confusion for developers unfamiliar with the non-standard abbreviation.
Tags: language, naming, readability
Affected code:

118: 5. **Python bytecache** — after changing `.py` files, delete `.pyc` and restart container

Proposed change:

5. **Python bytecode cache** — after changing `.py` files, delete `.pyc` and restart container

#4 Inconsistent terminology: 'Inbnd' vs 'Inbound' abbreviation

docs/architecture/archive/gateway-mcp-design.v1.md L543

On line 543, the normalization output is abbreviated as 'normalize → Inbnd.' which is inconsistent with 'normalize → Inbound' used on lines 541 and 542. This inconsistency in terminology may cause confusion.
Tags: language, consistency, readability
Affected code:

543: │  │  │  normalize → Inbnd│  │  normalize → Inbnd│  │  normalize → Inb.│         │  │

Proposed change:

│  │  │  normalize → Inbound│  │  normalize → Inbound│  │  normalize → Inbound│         │  │

#5 Inconsistent capitalization of product name 'plit'

docs/architecture/archive/gateway-mcp-design.v2-iterative.md L32

Throughout the document, 'plit' is written in lowercase (e.g., lines 32, 206, 411, 678) while it appears to be a product name that should be consistently capitalized as 'Plit' or 'PLIT'. The document shows inconsistent usage between 'plit', 'Plit', and 'PLIT'.
Tags: naming, readability, maintainability
Affected code:

32: │  │              PLIT GATEWAY                         │    │

Proposed change:

│  │              PLIT GATEWAY                         │    │

#6 Inconsistent date format - future date in design document

docs/architecture/archive/gateway-mcp-design.v2-iterative.md L3

Line 3 contains the date '2026-03-22' which is in the future. This appears to be a typo and should likely be a past date (e.g., 2024 or 2025) for a completed design document.
Tags: bug, language
Affected code:

3: **Date:** 2026-03-22

Proposed change:

**Date:** 2025-03-22

#7 Inconsistent CEL rule comment on line 114

docs/architecture/gateway-mcp-design.md L112-L114

Line 114 shows a CEL rule with comment '# → deny rule' but the rule logic uses 'contains' which would need negation or explicit deny action to actually deny. The comment suggests this is a deny rule, but CEL authorization rules typically allow when true. This creates ambiguity about whether the rule allows or denies opus access to kid.
Tags: bug, maintainability, language
Affected code:

112:     # Model restrictions
113:     # (kid can't use opus)
114:     - 'jwt.sub == "kid" && llm.requestModel contains "opus"'  # → deny rule

Proposed change:

    # Model restrictions
    # (deny kid access to opus)
    - '!(jwt.sub == "kid" && llm.requestModel contains "opus")'  # → deny via negation

#8 Inconsistent capitalization of product name 'plit'

docs/architecture/gateway-mcp-design.md L296-L301

The document inconsistently capitalizes 'plit' - sometimes 'plit' (lines 12, 79, 84, 147, etc.), sometimes 'PLIT' (lines 296, 300), and references to 'plit-gw' (lines 15, 163, etc.). Product names should maintain consistent capitalization throughout technical documentation for clarity and professionalism.
Tags: language, code-style, maintainability
Affected code:

296: │                                                                              │
297: │  ┌─ PLIT CLI ─────────────────────────────────────────────────────────────┐  │
298: │  │  plit mcp add slack       → writes agentgateway config, hot-reloaded   │  │
299: │  │  plit credentials create  → stores in agentgateway config              │  │
300: │  │  plit start / stop        → manages full stack lifecycle               │  │
301: │  └────────────────────────────────────────────────────────────────────────┘  │

#9 Anachronistic date in design document

docs/architecture/workflow-composition-design.md L3

The document is dated '2026-03-22' which is a future date. This is likely a typo and should be corrected to reflect the actual date of creation (probably 2024 or 2025).
Tags: bug, readability
Affected code:

3: **Date:** 2026-03-22

Proposed change:

**Date:** 2024-03-22

#10 Inconsistent date reference in proof of concept section

docs/architecture/workflow-composition-design.md L179

Line 179 references the same future date '2026-03-22' in a validation context. This should be corrected to match the corrected document date.
Tags: bug, readability
Affected code:

179:    ✅ Workflow composition (validated 2026-03-22)

Proposed change:

   ✅ Workflow composition (validated 2024-03-22)

#11 Migration date is set in the future (2026)

platform/alembic/versions/a41f9c2d7e10_drop_llm_credentials_api_key.py L5

The Create Date is set to '2026-07-03 00:00:00.000000', which is in the future. Database migration timestamps should reflect the actual creation date to maintain proper versioning history and avoid confusion.
Tags: bug, maintainability
Affected code:

5: Create Date: 2026-07-03 00:00:00.000000

Proposed change:

Create Date: 2024-07-03 00:00:00.000000

#12 Inconsistent import naming pattern for available_models router

platform/api/init.py L16

All other router imports follow the pattern 'from api. import router as _router', but the available_models import uses 'available_models_router' directly instead of 'router as available_models_router'. This breaks the established naming convention and may indicate the source module exports a differently named object, which is inconsistent with the rest of the codebase.
Tags: maintainability, code-style, naming
Affected code:

16: from api.available_models import available_models_router

Proposed change:

from api.available_models import router as available_models_router

#13 Import should be at module level to avoid runtime overhead and improve testability

platform/api/available_models.py L3-L9,L23

The import of list_all_available_models is placed inside the function body at line 23. This causes the import to execute on every function call when AGENTGATEWAY_ENABLED is True, adding unnecessary runtime overhead. Module-level imports are a Python best practice as they are executed once at module load time, improving performance and making dependencies explicit for testing and analysis tools.
Tags: performance, maintainability, code-style
Affected code:

3: from __future__ import annotations
4: 
5: from fastapi import APIRouter, Depends
6: 
7: from auth import get_current_user
8: from config import settings
9: from models.user import UserProfile

Proposed change:

from __future__ import annotations

from fastapi import APIRouter, Depends

from auth import get_current_user
from config import settings
from models.user import UserProfile
from services.agentgateway_config import list_all_available_models

Affected code:

23:     from services.agentgateway_config import list_all_available_models

#14 Inconsistent behavior: list_credential_models rejects all calls with 410 regardless of credential type

platform/api/credentials.py L396-L407

The list_credential_models endpoint (lines 396-407) immediately calls _reject_llm_credential() for any credential_id, without first checking the credential's type. This means even gateway/git/tool credentials will receive a 410 error with a message about LLM credentials. The endpoint should either verify the credential exists and is of type 'llm' before rejecting, or the function should be removed entirely if model listing is no longer supported for any credential type.
Tags: bug, maintainability
Affected code:

396: @router.get("/{credential_id}/models/", response_model=list[CredentialModelOut])
397: def list_credential_models(
398:     credential_id: int,
399:     db: Session = Depends(get_db),
400:     profile: UserProfile = Depends(get_current_user),
401: ):
402:     """Removed: credential-derived model listing is gone.
403: 
404:     Models are listed from agentgateway (see api/available_models.py) —
405:     pipelit no longer holds provider API keys to query upstream /models.
406:     """
407:     _reject_llm_credential()

Proposed change:

@router.get("/{credential_id}/models/", response_model=list[CredentialModelOut])
def list_credential_models(
    credential_id: int,
    db: Session = Depends(get_db),
    profile: UserProfile = Depends(get_current_user),
):
    """Removed: credential-derived model listing is gone.

    Models are listed from agentgateway (see api/available_models.py) —
    pipelit no longer holds provider API keys to query upstream /models.
    """
    query = db.query(BaseCredential).filter(BaseCredential.id == credential_id)
    if profile.role != UserRole.ADMIN:
        query = query.filter(BaseCredential.user_profile_id == profile.id)
    cred = query.first()
    if not cred:
        raise HTTPException(status_code=404, detail="Credential not found.")
    
    if cred.credential_type == "llm":
        _reject_llm_credential()
    
    raise HTTPException(
        status_code=400,
        detail=f"Credential type '{cred.credential_type}' does not support model listing.",
    )

#15 Missing 'backend_route' field in create_node function for ai_model components

platform/api/nodes.py L105-L111

The diff adds 'backend_route' to the model_fields tuple in update_node (line 195), but the create_node function (lines 105-111) does not include 'backend_route' when extracting model config fields for ai_model components. This creates an inconsistency where backend_route can be updated but not set during creation, potentially causing issues when users expect to set this field when creating a node.
Tags: bug, consistency, maintainability
Affected code:

105:     if component_type == "ai_model":
106:         kwargs["llm_credential_id"] = config_data.get("llm_credential_id")
107:         kwargs["model_name"] = config_data.get("model_name", "")
108:         for param in ("temperature", "max_tokens", "frequency_penalty", "presence_penalty",
109:                        "top_p", "timeout", "max_retries", "response_format"):
110:             if config_data.get(param) is not None:
111:                 kwargs[param] = config_data[param]

Proposed change:

    if component_type == "ai_model":
        kwargs["llm_credential_id"] = config_data.get("llm_credential_id")
        kwargs["model_name"] = config_data.get("model_name", "")
        kwargs["backend_route"] = config_data.get("backend_route")
        for param in ("temperature", "max_tokens", "frequency_penalty", "presence_penalty",
                       "top_p", "timeout", "max_retries", "response_format"):
            if config_data.get(param) is not None:
                kwargs[param] = config_data[param]

#16 Missing return statement in DELETE endpoint

platform/api/providers.py L255-L270

The delete_provider endpoint (lines 255-270) is missing an explicit return statement. While FastAPI will handle this, it's inconsistent with REST API best practices and may cause issues with some clients expecting a response body. The endpoint should explicitly return None or an empty response.
Tags: bug, maintainability
Affected code:

255: @router.delete("/{provider}/", status_code=204)
256: def delete_provider(
257:     provider: str,
258:     _admin: UserProfile = Depends(require_admin),
259: ):
260:     """Remove a provider and all its models."""
261:     _require_agentgateway()
262: 
263:     from services.agentgateway_config import remove_provider, restart_agentgateway
264: 
265:     try:
266:         remove_provider(provider)
267:         restart_agentgateway()  # Key removed, restart to clean env vars
268:     except RuntimeError as exc:
269:         raise HTTPException(status_code=502, detail=str(exc))
270: 

Proposed change:

@router.delete("/{provider}/", status_code=204)
def delete_provider(
    provider: str,
    _admin: UserProfile = Depends(require_admin),
):
    """Remove a provider and all its models."""
    _require_agentgateway()

    from services.agentgateway_config import remove_provider, restart_agentgateway

    try:
        remove_provider(provider)
        restart_agentgateway()  # Key removed, restart to clean env vars
    except RuntimeError as exc:
        raise HTTPException(status_code=502, detail=str(exc))

    return None

#17 Missing return statement in DELETE endpoint

platform/api/providers.py L330-L344

The delete_model endpoint (lines 330-344) is missing an explicit return statement. While FastAPI will handle this, it's inconsistent with REST API best practices and may cause issues with some clients expecting a response body. The endpoint should explicitly return None or an empty response.
Tags: bug, maintainability
Affected code:

330: @router.delete("/{provider}/models/{model_slug}/", status_code=204)
331: def delete_model(
332:     provider: str,
333:     model_slug: str,
334:     _admin: UserProfile = Depends(require_admin),
335: ):
336:     """Remove a single model from a provider."""
337:     _require_agentgateway()
338: 
339:     from services.agentgateway_config import remove_model
340: 
341:     try:
342:         remove_model(provider, model_slug)
343:     except RuntimeError as exc:
344:         raise HTTPException(status_code=502, detail=str(exc))

Proposed change:

@router.delete("/{provider}/models/{model_slug}/", status_code=204)
def delete_model(
    provider: str,
    model_slug: str,
    _admin: UserProfile = Depends(require_admin),
):
    """Remove a single model from a provider."""
    _require_agentgateway()

    from services.agentgateway_config import remove_model

    try:
        remove_model(provider, model_slug)
    except RuntimeError as exc:
        raise HTTPException(status_code=502, detail=str(exc))

    return None

#18 Potential port assignment to None when hostname is missing

platform/api/providers.py L99-L103

In _parse_base_url function (lines 99-103), if parsed.hostname is None/empty and parsed.port is None, the code still attempts to set a default port (443 or 80) even though there's no host. This could lead to host_override being :443 or :80 instead of an empty string. The port assignment should only happen when a valid hostname exists.
Tags: bug, maintainability
Affected code:

99:     host = parsed.hostname or ""
100:     port = parsed.port
101:     if host and not port:
102:         port = 443 if parsed.scheme == "https" else 80
103:     host_override = f"{host}:{port}" if host else ""

Proposed change:

    host = parsed.hostname or ""
    port = parsed.port
    if host and port is None:
        port = 443 if parsed.scheme == "https" else 80
    host_override = f"{host}:{port}" if (host and port) else ""

#19 Incorrect endpoint suffix condition logic

platform/api/providers.py L106-L112

In _parse_base_url function (lines 107-112), the endpoint suffix is only appended when path_override is truthy, but this logic is backwards. If the user provides a base_url with a host but no path, path_override will be empty and no suffix will be added. The suffix should be added whenever we have a host_override, not just when path_override exists.
Tags: bug
Affected code:

106:     # Append endpoint suffix
107:     if provider_type in ("openai_compatible", "glm", "openai"):
108:         if path_override and not path_override.endswith("/chat/completions"):
109:             path_override = path_override.rstrip("/") + "/chat/completions"
110:     elif provider_type == "anthropic":
111:         if path_override and not path_override.endswith("/messages"):
112:             path_override = path_override.rstrip("/") + "/messages"

Proposed change:

    # Append endpoint suffix
    if provider_type in ("openai_compatible", "glm", "openai"):
        if not path_override.endswith("/chat/completions"):
            path_override = path_override.rstrip("/") + "/chat/completions"
    elif provider_type == "anthropic":
        if not path_override.endswith("/messages"):
            path_override = path_override.rstrip("/") + "/messages"

#20 Inconsistent naming: JWT_PRIVATE_KEY vs AGENTGATEWAY_* prefix

platform/config.py L138

The new configuration block introduces JWT_PRIVATE_KEY without the AGENTGATEWAY_ prefix, while the other three settings use AGENTGATEWAY_*. Since JWT_PRIVATE_KEY is specifically for agentgateway integration (as indicated by the comment), it should follow the same naming convention for consistency and clarity.
Tags: naming, maintainability, code-style
Affected code:

138:     JWT_PRIVATE_KEY: str = ""  # PEM-encoded EC private key, generated by `plit init`

Proposed change:

    AGENTGATEWAY_JWT_PRIVATE_KEY: str = ""  # PEM-encoded EC private key, generated by `plit init`

#21 Typo in comment: 'it' should be 'is'

platform/conftest.py L22

Line 22 contains a grammatical error: 'default it disabled' should be 'default is disabled'. This typo reduces code readability and professionalism.
Tags: language, readability
Affected code:

22: # Tests don't have a live agentgateway; default it disabled so the hard

Proposed change:

# Tests don't have a live agentgateway; default is disabled so the hard

#22 Incomplete query cache invalidation after adding models

platform/frontend/src/api/providers.ts L46-L53

The useAddModels mutation only invalidates the ['providers'] query on success, but does not invalidate the ['provider-configured-models', provider] query. This means that after adding models, the UI displaying configured models for that specific provider will show stale data until manually refetched or the component remounts.
Tags: bug, maintainability
Affected code:

46: export function useAddModels() {
47:   const qc = useQueryClient()
48:   return useMutation({
49:     mutationFn: ({ provider, models }: { provider: string; models: { slug: string; model_name: string }[] }) =>
50:       apiFetch<void>(`/providers/${provider}/models/`, { method: "POST", body: JSON.stringify({ models }) }),
51:     onSuccess: () => qc.invalidateQueries({ queryKey: ["providers"] }),
52:   })
53: }

Proposed change:

export function useAddModels() {
  const qc = useQueryClient()
  return useMutation({
    mutationFn: ({ provider, models }: { provider: string; models: { slug: string; model_name: string }[] }) =>
      apiFetch<void>(`/providers/${provider}/models/`, { method: "POST", body: JSON.stringify({ models }) }),
    onSuccess: (_data, variables) => {
      qc.invalidateQueries({ queryKey: ["providers"] })
      qc.invalidateQueries({ queryKey: ["provider-configured-models", variables.provider] })
    },
  })
}

#23 Incomplete query cache invalidation after deleting a model

platform/frontend/src/api/providers.ts L55-L62

The useDeleteModel mutation only invalidates the ['providers'] query on success, but does not invalidate the ['provider-configured-models', provider] query. This means that after deleting a model, the UI displaying configured models for that specific provider will show stale data until manually refetched or the component remounts.
Tags: bug, maintainability
Affected code:

55: export function useDeleteModel() {
56:   const qc = useQueryClient()
57:   return useMutation({
58:     mutationFn: ({ provider, modelSlug }: { provider: string; modelSlug: string }) =>
59:       apiFetch<void>(`/providers/${provider}/models/${modelSlug}/`, { method: "DELETE" }),
60:     onSuccess: () => qc.invalidateQueries({ queryKey: ["providers"] }),
61:   })
62: }

Proposed change:

export function useDeleteModel() {
  const qc = useQueryClient()
  return useMutation({
    mutationFn: ({ provider, modelSlug }: { provider: string; modelSlug: string }) =>
      apiFetch<void>(`/providers/${provider}/models/${modelSlug}/`, { method: "DELETE" }),
    onSuccess: (_data, variables) => {
      qc.invalidateQueries({ queryKey: ["providers"] })
      qc.invalidateQueries({ queryKey: ["provider-configured-models", variables.provider] })
    },
  })
}

#24 Missing validation for 'git' credential type

platform/frontend/src/features/credentials/CredentialsPage.tsx L19,L52-L63

The credential type 'git' is included in ALL_CREDENTIAL_TYPES (line 19) and can be selected in the form, but there is no handling for it in the handleCreate function (lines 46-74). The function only handles 'gateway' and 'tool' types, meaning selecting 'git' would result in an empty detail object being sent, which is likely a bug.
Tags: bug, maintainability
Affected code:

19: const ALL_CREDENTIAL_TYPES: CredentialType[] = ["gateway", "git", "tool"]

Affected code:

52:     if (credType === "gateway") {
53:       detail = { adapter_type: gatewayAdapterType, token: gatewayToken }
54:       if (gatewayConfig.trim()) {
55:         try {
56:           detail.config = JSON.parse(gatewayConfig)
57:         } catch {
58:           alert("Invalid JSON in config field")
59:           return
60:         }
61:       }
62:     }
63:     else if (credType === "tool") detail = { tool_type: toolType, config: { url: toolUrl }, is_preferred: toolPreferred }

#25 Race condition: slug generation may produce duplicates

platform/frontend/src/features/providers/ProvidersPage.tsx L60-L64

On line 63, the slug is generated by replacing non-alphanumeric characters with hyphens using replace(/[^a-zA-Z0-9_-]/g, "-"). This can produce duplicate slugs for different model IDs (e.g., 'model.name' and 'model/name' both become 'model-name'), causing potential conflicts or database constraint violations when multiple models are added simultaneously.
Tags: bug, maintainability
Affected code:

60:   async function handleAddModels() {
61:     const models = fetchedModels
62:       .filter((m) => selectedModels.has(m.id))
63:       .map((m) => ({ slug: m.id.replace(/[^a-zA-Z0-9_-]/g, "-"), model_name: m.id }))
64:     if (models.length === 0) return

#26 Error swallowed without propagation or user notification

platform/frontend/src/features/providers/ProvidersPage.tsx L74-L79

On line 77, handleDeleteModel only displays a success toast but provides no error handling. If the deletion fails, the user receives no feedback since the mutation error is silently ignored. This creates a poor user experience where the UI may appear to succeed when the operation actually failed.
Tags: bug, maintainability
Affected code:

74:   function handleDeleteModel(modelSlug: string) {
75:     deleteModel.mutate(
76:       { provider: provider.provider, modelSlug },
77:       { onSuccess: () => toast.success("Model deleted") },
78:     )
79:   }

Proposed change:

  function handleDeleteModel(modelSlug: string) {
    deleteModel.mutate(
      { provider: provider.provider, modelSlug },
      {
        onSuccess: () => toast.success("Model deleted"),
        onError: () => toast.error("Failed to delete model"),
      },
    )
  }

#27 Error swallowed without propagation or user notification

platform/frontend/src/features/providers/ProvidersPage.tsx L81-L85

On line 82-84, handleDeleteProvider only displays a success toast but provides no error handling. If the deletion fails, the user receives no feedback since the mutation error is silently ignored. The confirmation dialog will also not close on error, leaving the user in an ambiguous state.
Tags: bug, maintainability
Affected code:

81:   function handleDeleteProvider() {
82:     deleteProvider.mutate(provider.provider, {
83:       onSuccess: () => { setDeleteConfirm(false); toast.success("Provider deleted") },
84:     })
85:   }

Proposed change:

  function handleDeleteProvider() {
    deleteProvider.mutate(provider.provider, {
      onSuccess: () => { setDeleteConfirm(false); toast.success("Provider deleted") },
      onError: () => toast.error("Failed to delete provider"),
    })
  }

#28 Hardcoded llm_credential_id set to null removes user configuration without migration path

platform/frontend/src/features/workflows/components/NodeDetailsPanel.tsx L365

Line 365 unconditionally sets llm_credential_id to null in handleSave, effectively deleting any existing credential configuration. This appears to be a migration from credential-based to route-based model selection, but there's no graceful transition: existing configurations will be silently lost on save. The old state variable llmCredentialId was removed but any nodes with existing llm_credential_id values will have them nullified without warning or data preservation.
Tags: bug, maintainability
Affected code:

365:           llm_credential_id: null,

#29 Excessively long single-line interface definition harms readability

platform/frontend/src/types/models.ts L56

The ComponentConfigData interface on line 56 contains 17 fields in a single line spanning over 500 characters. This violates standard TypeScript formatting conventions and makes the code nearly impossible to read, review, or maintain. The newly added 'backend_route' field exacerbates this issue. This should be reformatted with one property per line.
Tags: readability, maintainability, code-style
Affected code:

56: export interface ComponentConfigData { system_prompt: string; extra_config: Record<string, unknown>; llm_credential_id: number | null; model_name: string; backend_route: string | null; temperature: number | null; max_tokens: number | null; frequency_penalty: number | null; presence_penalty: number | null; top_p: number | null; timeout: number | null; max_retries: number | null; response_format: Record<string, unknown> | null; credential_id: number | null; is_active: boolean; priority: number; trigger_config: Record<string, unknown> }

Proposed change:

export interface ComponentConfigData {
  system_prompt: string
  extra_config: Record<string, unknown>
  llm_credential_id: number | null
  model_name: string
  backend_route: string | null
  temperature: number | null
  max_tokens: number | null
  frequency_penalty: number | null
  presence_penalty: number | null
  top_p: number | null
  timeout: number | null
  max_retries: number | null
  response_format: Record<string, unknown> | null
  credential_id: number | null
  is_active: boolean
  priority: number
  trigger_config: Record<string, unknown>
}

#30 Missing database migration for api_key column removal

platform/models/credential.py

The api_key column is removed from the LLMProviderCredential model, but there is no evidence of a corresponding database migration in this diff. Removing a mapped column without a migration will cause runtime errors when the ORM tries to access a non-existent database column. This is a hard cutover according to the docstring, requiring a migration to DROP COLUMN.
Tags: bug, compatibility

#31 Inconsistent type definition for detail field between input and output schemas

platform/schemas/credential.py L14

The detail field has inconsistent type definitions across schemas. In CredentialIn (line 14), it's defined as dict | None, while in CredentialOut (line 26), it's defined as str | dict | None. This inconsistency suggests either a missing type in the input schema or an unnecessary type in the output schema. If detail can be a string in the output, it should likely also accept strings in the input, or the output typing may be incorrect.
Tags: bug, maintainability, compatibility
Affected code:

14:     detail: dict | None = None

Proposed change:

    detail: str | dict | None = None

#32 Inconsistent provider type naming in supported providers set

platform/services/agentgateway_client.py L34

Line 34 defines _OPENAI_LIKE_PROVIDERS with both "openai_compatible" and "openai-compatible" (hyphenated), but the docstring on line 53 and error message on line 90 only mention "openai_compatible" (underscore). This inconsistency may cause confusion about which format is actually supported. Either both formats are intended to be supported (in which case documentation should reflect this), or only one should be in the set.
Tags: maintainability, naming, readability
Affected code:

34: _OPENAI_LIKE_PROVIDERS = {"openai", "glm", "openai_compatible", "openai-compatible"}

Proposed change:

_OPENAI_LIKE_PROVIDERS = {"openai", "glm", "openai_compatible"}

#33 Race condition: file permissions set after write but before rename

platform/services/agentgateway_config.py L96-L98

In write_provider_key(), the file permissions are set on the temporary file after writing the content but before the atomic rename. If the process crashes or is interrupted between write_bytes() and chmod(), the temporary file will have default permissions (potentially 0o644), exposing the encrypted API key. The permissions should be set before writing sensitive data, or the file should be opened with secure permissions from the start.
Tags: security, bug
Affected code:

96:     tmp_path.write_bytes(encrypted)
97:     os.chmod(tmp_path, 0o600)
98:     os.rename(tmp_path, final_path)

Proposed change:

    fd = os.open(tmp_path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600)
    os.write(fd, encrypted)
    os.close(fd)
    os.rename(tmp_path, final_path)

#34 Unclear comment: 'literal string "null"' incorrectly describes YAML null behavior

platform/services/agentgateway_config.py L216-L218

Line 217 states that assemble-config.sh 'folds the missing key into the literal string "null"'. This is misleading. In YAML, when yq outputs a missing value, it produces the YAML null scalar (represented as null without quotes), not the literal string "null". The comment should clarify that yq emits a YAML null scalar, which is then compared against the string 'null' in Python on line 386.
Tags: language, readability
Affected code:

216:         # No model: key -> no override. assemble-config.sh folds the missing
217:         # key into the literal string "null"; reassemble_config() strips that
218:         # sentinel from the assembled config to restore pass-through.

Proposed change:

        # No model: key -> no override. assemble-config.sh folds the missing
        # key into a YAML null scalar; reassemble_config() strips that
        # sentinel from the assembled config to restore pass-through.

#35 Hardcoded sleep duration without explanation or configuration

platform/services/agentgateway_config.py L455

In restart_agentgateway(), a hardcoded 2-second sleep is used to wait for graceful shutdown (line 455). This value is arbitrary and may be insufficient on slow systems or excessive on fast ones. The code lacks explanation for why 2 seconds was chosen, and there's no mechanism to verify that the process actually terminated. This could lead to race conditions when starting the new process.
Tags: maintainability, bug
Affected code:

455:     time.sleep(2)  # Wait for graceful shutdown

#36 Dangerous bare exception suppression in process management

platform/services/agentgateway_config.py L452-L453

In restart_agentgateway(), lines 452-453 use a bare except Exception: pass that silently swallows all exceptions during process termination. This could hide legitimate errors (e.g., permission issues, system errors) and make debugging difficult. At minimum, unexpected exceptions should be logged.
Tags: bug, maintainability
Affected code:

452:     except Exception:
453:         pass  # No process found — that's OK

#37 No verification that agentgateway process actually started

platform/services/agentgateway_config.py L472-L479

restart_agentgateway() launches the new process with Popen() and immediately returns without checking if the process started successfully. The function should verify that the new process is running, or at least check that start.sh didn't immediately fail, to catch configuration errors or missing dependencies.
Tags: bug, maintainability
Affected code:

472:     subprocess.Popen(
473:         [str(start_script)],
474:         cwd=str(agw_dir),
475:         env=env,
476:         stdout=subprocess.DEVNULL,
477:         stderr=subprocess.DEVNULL,
478:         start_new_session=True,
479:     )

#38 Inconsistent atomic write pattern: missing chmod in other write functions

platform/services/agentgateway_config.py L152-L156,L223-L225,L320-L321,L349-L350

The add_provider(), add_model(), add_mcp_server(), and update_rules() functions all write temporary files and rename them atomically, but none of them set explicit file permissions like write_provider_key() does. While these files may not contain secrets, inconsistent security practices make the codebase harder to audit. All atomic writes should follow the same security pattern.
Tags: security, maintainability
Affected code:

152:     tmp_path = provider_dir / "_provider.yaml.tmp"
153:     final_path = provider_dir / "_provider.yaml"
154: 
155:     tmp_path.write_text(content)
156:     os.rename(tmp_path, final_path)

Affected code:

223: 
224:     tmp_path.write_text(content)
225:     os.rename(tmp_path, final_path)

Affected code:

320:     tmp_path.write_text(content)
321:     os.rename(tmp_path, final_path)

Affected code:

349:     tmp_path.write_text(content)
350:     os.rename(tmp_path, final_path)

#39 Missing null check on owner object before attribute access

platform/services/llm.py L328-L331

Line 328: db.get(UserProfile, user_profile_id) can return None if the user does not exist, but line 329 directly calls getattr(owner, 'role', None) without checking if owner is None. While getattr safely returns the default, this is a logic error—the code should explicitly handle the missing user case for clarity and to avoid silent failures.
Tags: bug
Affected code:

328:             owner = db.get(UserProfile, user_profile_id)
329:             role_value = getattr(owner, "role", None)
330:             if isinstance(role_value, str):
331:                 user_role = role_value

Proposed change:

            owner = db.get(UserProfile, user_profile_id)
            if owner is not None:
                role_value = getattr(owner, "role", None)
                if isinstance(role_value, str):
                    user_role = role_value

#40 Test encryption key generated at module load time causes non-deterministic behavior

platform/tests/test_agentgateway_config.py L15-L16

Line 16 generates a new Fernet key every time the test module is loaded using Fernet.generate_key(). The comment says 'Generate a stable test key' but the implementation is non-stable. This creates a new random key on each test run, which contradicts the comment's intent and could cause issues if tests depend on key stability across test sessions or processes.
Tags: bug, maintainability
Affected code:

15: # Generate a stable test key
16: _TEST_KEY = Fernet.generate_key().decode()

Proposed change:

# Generate a stable test key
_TEST_KEY = b'ZmDfcTF7_60GrrY167zsiPd67pEvs0aGOv2oasOM1Pg='.decode()

#41 Typo in inline comment: 'plit' should be 'pipelit'

platform/tests/test_cli.py L303-L304

Line 304 contains a typo in the comment: 'route plit init created' should be 'route pipelit init created'. This typo appears in a critical comment explaining the system's routing behavior and may cause confusion when maintaining the code.
Tags: language, readability
Affected code:

303:         # pipelit calls agentgateway at /{backend_route}/... — this must match the
304:         # route plit init created, or the proxied LLM call 404s ("route not found").

Proposed change:

        # pipelit calls agentgateway at /{backend_route}/... — this must match the
        # route pipelit init created, or the proxied LLM call 404s ("route not found").

#42 Inconsistent mock_settings pattern creates code duplication and maintainability issues

platform/tests/test_credentials_agentgateway.py L130-L135,L187-L192,L246-L251

The same mock_settings setup pattern is duplicated across three test methods (lines 130-135, 187-192, 246-251). This repetitive code copies all uppercase attributes from real_settings to mock_settings, then overrides AGENTGATEWAY_ENABLED. This duplication makes the code harder to maintain and increases the risk of inconsistencies.
Tags: maintainability, anti-pattern, readability
Affected code:

130:         with patch("api.credentials.settings") as mock_settings:
131:             from config import settings as real_settings
132:             for attr in dir(real_settings):
133:                 if attr.isupper():
134:                     setattr(mock_settings, attr, getattr(real_settings, attr))
135:             mock_settings.AGENTGATEWAY_ENABLED = True

Proposed change:

            mock_settings.configure_mock(**{
                attr: getattr(real_settings, attr)
                for attr in dir(real_settings) if attr.isupper()
            })
            mock_settings.AGENTGATEWAY_ENABLED = True

Affected code:

187:         with patch("api.credentials.settings") as mock_settings:
188:             from config import settings as real_settings
189:             for attr in dir(real_settings):
190:                 if attr.isupper():
191:                     setattr(mock_settings, attr, getattr(real_settings, attr))
192:             mock_settings.AGENTGATEWAY_ENABLED = True

Proposed change:

            mock_settings.configure_mock(**{
                attr: getattr(real_settings, attr)
                for attr in dir(real_settings) if attr.isupper()
            })
            mock_settings.AGENTGATEWAY_ENABLED = True

Affected code:

246:         with patch("api.credentials.settings") as mock_settings:
247:             from config import settings as real_settings
248:             for attr in dir(real_settings):
249:                 if attr.isupper():
250:                     setattr(mock_settings, attr, getattr(real_settings, attr))
251:             mock_settings.AGENTGATEWAY_ENABLED = True

Proposed change:

            mock_settings.configure_mock(**{
                attr: getattr(real_settings, attr)
                for attr in dir(real_settings) if attr.isupper()
            })
            mock_settings.AGENTGATEWAY_ENABLED = True

#43 Unsafe deletion of all dependency overrides affects test isolation

platform/tests/test_credentials_agentgateway.py L37-L39

Line 39 clears all dependency overrides with _app.dependency_overrides.clear(). If other fixtures or tests have added their own overrides, this will remove them, potentially causing test isolation issues and hard-to-debug failures in other tests.
Tags: bug, maintainability
Affected code:

37:     _app.dependency_overrides[get_db] = _override_get_db
38:     yield _app
39:     _app.dependency_overrides.clear()

Proposed change:

    _app.dependency_overrides[get_db] = _override_get_db
    yield _app
    _app.dependency_overrides.pop(get_db, None)

#44 Missing assertion message in test_glm_update_rejected

platform/tests/test_credentials_glm.py L89-L94

Line 94 checks for status code 410 but does not verify the response contains 'agentgateway' in the detail message, unlike the other three rejection tests (lines 77, 82, 87). This inconsistency means the test is less thorough and may pass even if the error message is incorrect or missing.
Tags: bug, maintainability
Affected code:

89:     def test_glm_update_rejected(self, auth_client, glm_credential):
90:         resp = auth_client.patch(
91:             f"/api/v1/credentials/{glm_credential.id}/",
92:             json={"detail": {"api_key": "new-key"}},
93:         )
94:         assert resp.status_code == 410

Proposed change:

    def test_glm_update_rejected(self, auth_client, glm_credential):
        resp = auth_client.patch(
            f"/api/v1/credentials/{glm_credential.id}/",
            json={"detail": {"api_key": "new-key"}},
        )
        assert resp.status_code == 410
        assert "agentgateway" in resp.json()["detail"]

#45 Missing required field api_key may cause database constraint violation or runtime error

platform/tests/test_workflow_create.py L34-L38

The diff removes the api_key field from the LLMProviderCredential instantiation (line 37 in the old code). If api_key is a required field in the LLMProviderCredential model (either as a non-nullable database column or a required model field), this will cause a database integrity error when attempting to commit, or a validation error at runtime. The test fixture llm_credential is used by multiple tests (lines 225, 466, 487) that expect a valid credential object, and those tests will fail if the credential cannot be created.
Tags: bug, compatibility
Affected code:

34:     llm = LLMProviderCredential(
35:         base_credentials_id=base.id,
36:         provider_type="openai_compatible",
37:         base_url="https://api.openai.com/v1",
38:     )

Proposed change:

    llm = LLMProviderCredential(
        base_credentials_id=base.id,
        provider_type="openai_compatible",
        api_key="sk-test-key",
        base_url="https://api.openai.com/v1",
    )