Skip to content

POST /api/v1/automation/:name/toggle answers 500 INTERNAL_ERROR instead of 404 NOT_FOUND for an unknown flow #7535

Description

@huangyiirene

Symptom

POST /api/v1/automation/:name/toggle against a flow name that does not exist answers 500 INTERNAL_ERROR where it should answer 404 NOT_FOUND.

An error-class mismatch, P3. Nothing is corrupted and nothing leaks — the toggle does not happen — but a 500 tells every caller the wrong thing: clients and retry layers treat 5xx as "the server broke, try again" and 4xx as "your request was wrong, don't". A typo in a flow name currently presents as a server fault, and any client with retry-on-5xx will hammer the endpoint over a request that can never succeed.

Root cause

Not root-caused to a line. The handler is POST /automation/:name/toggle, registered in packages/runtime/src/dispatcher-plugin.ts and dispatched to toggleFlow in packages/runtime/src/domains/automation.ts; the route is declared in packages/runtime/src/route-ledger.ts. The shape is the familiar one — an unknown-name lookup throws and the catch maps everything to INTERNAL_ERROR rather than mapping the not-found case to 404 first.

Note the same endpoint already gets its body validation right: { enabled?: boolean } is strict, and {"enable": false} (one letter off) is a located 400 rather than a silent enable (#3899). The missing-flow arm just never got the same treatment.

Reproduction

  1. Boot the showcase on an isolated file DB and port.
  2. POST /api/v1/automation/definitely_not_a_flow/toggle with body {"enabled": false}.
  3. Observe 500 INTERNAL_ERROR. Expected 404 NOT_FOUND naming the unknown flow.

Re-check the handler:

rg -n "toggle" packages/runtime/src/domains/automation.ts packages/runtime/src/dispatcher-plugin.ts

Source

Extracted from the QA run #7516 (framework a86db17). Listed there as item 4 under "Fixture / authoring issues to fix", flagged P3.

Activity

  1. self-assigned this
    on Aug 11, 2026
  2. os-help commented on Aug 11, 2026

    @os-help
    Collaborator

    Claim — PM loop domain:cli, session session_0158ZQo7LiHSxGWpYKuPq1wu (os-help seat, #6024), wave 5.


    Generated by Claude Code

  3. os-help commented on Aug 11, 2026

    @os-help
    Collaborator

    os-dev-report — issue #7535 · branch claude/issue-7535-toggle-unknown-flow-404 · draft PR #7558

    {
      "marker": "os-dev-report",
      "issue": 7535,
      "status": "complete",
      "branch": "claude/issue-7535-toggle-unknown-flow-404",
      "pr": 7558,
      "serial_constraint": "respected — http-dispatcher.ts NOT touched",
      "files": [
        "packages/runtime/src/domains/automation.ts",
        "packages/runtime/src/domains/automation-toggle-unknown-flow.test.ts",
        ".changeset/automation-toggle-unknown-flow-404.md"
      ],
      "gates": {
        "typecheck": "pass (@objectstack/runtime, tsc --noEmit)",
        "lint": "pass (eslint --no-inline-config, both changed files)",
        "tests": "pass — @objectstack/runtime full suite, 123 files / 1982 tests"
      }
    }

    Root cause — located

    packages/services/service-automation/src/engine.ts:2411 — toggleFlow throws a plain Error("Flow '<name>' not found") for an unregistered name. It carries no .status, and both dispatcher error exits (HttpDispatcher.errorFromThrown, dispatcher-plugin's errorResponseBase) read .status / .statusCode / a ValidationError duck-type and otherwise fall back to 500. Nothing was calling the not-found case a not-found.

    The serial constraint — checked, and it does not bind

    The card warned that if the mapping belonged in http-dispatcher.ts's dispatch() catch (where #7450 is in flight), I should report back rather than edit. It does not belong there, and I did not touch that file:

    • The catch is generic. The only thing it could match on is the engine's message prose (/not found/), which would make every domain's not-found status depend on one service's wording — precisely the coupling validation-failure.ts was written to avoid ("deciding it means 400 is the job of whichever boundary serves it").
    • Fixing it in the catch would also fix it for exactly one implementation's phrasing, not for the contract.

    So the fix is at the domain handler, as dispatched. No serialisation needed — #7450 and this card can land in either order.

    Fix

    packages/runtime/src/domains/automation.ts, toggle branch: probe existence before touching the service, return 404 in the house envelope naming the flow.

    if (typeof automationService.getFlow === 'function') {
        const existing = await automationService.getFlow(name);
        if (!existing) {
            return { handled: true, response: deps.error(`Flow '${name}' not found`, 404) };
        }
    }
    await automationService.toggleFlow(name, enabled);

    Three decisions worth the PM's eye:

    1. The probe is the one GET /:name already uses (same file, line ~528), not a second notion of existence — the two routes cannot disagree about which flows exist.
    2. After the body checks, not before. 请求体从不与声明它的 schema 对照(#3877 的请求侧对偶):7 个 schema 定义了从未启用,而 API 目录已宣称生效 #3899's guarantee is that nothing reaches the service until the body is legal; probing first would consult the registry on a request about to be refused anyway. Pinned by its own test.
    3. getFlow? is optional on IAutomationService. An implementation that omits it cannot be asked, so its toggle proceeds exactly as before rather than this inventing a 404. Also pinned.

    Quality bar (#3899 parity): the 404 names the flow (Flow 'definitely_not_a_flow' not found) in the house envelope — success:false, semantic error.code = RESOURCE_NOT_FOUND, error.httpStatus = 404 — the way the body rejection names the offending key. error.code is left to derive from the status, matching the sibling GET /:name; the 404 bucket in StandardErrorCode has no flow-specific member and inventing one is not this card's business.

    Mutation table — every new test proved able to fail

    6 new tests in automation-toggle-unknown-flow.test.ts. The fake models the real engine (getFlow → null for unknown, toggleFlow throws), so a handler that never checks cannot pass.

    # Mutation applied to source Result Tests that went red
    M1 Guard deleted entirely (pre-fix behaviour) 🔴 2 failed 404-envelope · no-500-by-any-route
    M2 deps.error(…, 404) → 500 🔴 2 failed 404-envelope · no-500-by-any-route
    M3 Message stops naming the flow ('Flow not found') 🔴 1 failed 404-envelope
    M4 if (!existing) → if (existing) 🔴 3 failed 404-envelope · no-500-by-any-route · real flow still toggles
    M5 typeof getFlow === 'function' guard dropped 🔴 1 failed contract-optional getFlow
    M6 Probe moved ahead of the body checks 🔴 1 failed body-before-registry ordering
    — baseline restored 🟢 6 passed —

    Coverage: all 6 tests are killed by at least one mutation; no test is inert.

    Card scope, honoured

    Note for the PM (no action taken)

    docs/qa/platform-checklist/areas/automation.json:982 already specifies the negative probe as "expect a not-found error, no state change" — the checklist was correct and the implementation was the thing out of step. No checklist edit needed; this change makes reality match it.

    Ending turn here — not waiting on CI, not arming auto-merge, per dispatch.


    Generated by Claude Code

  4. os-help commented on Aug 11, 2026

    @os-help
    Collaborator

    ACCEPT — PR #7558, head 524b44ada.

    • Cause located (the card had not root-caused it): toggleFlow on an unregistered name throws a plain Error("Flow '<name>' not found") from service-automation's engine. It carries no .status, so both dispatcher error exits — errorFromThrown and the plugin's errorResponseBase — fell through to their 500 default. Now 404 RESOURCE_NOT_FOUND, naming the flow.
    • The serial constraint was honoured, and for the right reason rather than because I said so. packages/runtime/src/http-dispatcher.ts is untouched — verified by diff, so The runtime dispatcher serialises a PermissionDeniedError's details to the client, so positions / permissionSets reach the browser on the /data transport #7450's landing was never at risk. The dev's own argument for fixing at the domain handler is the better one: which HTTP status a plain domain error means is the serving boundary's decision (the rule validation-failure.ts already states for ValidationError → 400), and teaching a shared catch to recognise one engine's message string would make every domain's not-found depend on that prose.
    • The existence check reuses the probe GET /automation/:name already uses, so the two routes cannot disagree about which flows exist. That is a stronger outcome than a local lookup: it makes the answer consistent by construction rather than by coincidence.
    • Composition order is stated and pinned: a malformed body is still rejected without the registry being consulted at all, so this sits behind 请求体从不与声明它的 schema 对照(#3877 的请求侧对偶):7 个 schema 定义了从未启用,而 API 目录已宣称生效 #3899's strict { enabled?: boolean } 400 rather than in front of it.
    • The optional-method case was thought through rather than papered over: an IAutomationService that omits the optional getFlow cannot be asked whether a flow exists, so its toggle proceeds exactly as before instead of this inventing a 404 it cannot justify.
    • Six mutations, each proved red then reverted — including the two that would silently degrade the fix (!existing inverted; the probe moved ahead of the body checks, which would break the composition order above) and the one that keeps the status but loses the point (message stops naming the flow).
    • CI, conclusions read personally: ESLint success, TypeScript Type Check success, Check Changeset success, all Test Core / Dogfood / Temporal shards success — 26 checks, zero failures. Full @objectstack/runtime suite green (123 files, 1982 tests).

    Ready flipped, auto-merge armed; queue membership verified by branch before any flip.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions