Skip to content

🐛 fix(server): preserve approval decision payloads across HTTP routes - #1428

Open
roninjin10 wants to merge 10 commits into
mainfrom
dpf/issue-1421
Open

🐛 fix(server): preserve approval decision payloads across HTTP routes#1428
roninjin10 wants to merge 10 commits into
mainfrom
dpf/issue-1421

Conversation

@roninjin10

Copy link
Copy Markdown
Contributor

Closes #1421

Threaded approval decisions through legacy routes, added shared validation, regression tests, and API docs.


Filed from the aomi labs deep-dive (2026-07-27).
Pipeline: Fable investigated, Codex Luna implemented, Fable + Codex Sol both approved in a review loop, a human reviewed at the landing gate.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown

❌ smithers review failed: 6 file reviews failed — run

@roninjin10

Copy link
Copy Markdown
Contributor Author

Automated code review

Reviewed 4 file(s) in the diff against main. 3 minor.

packages/server/src/approvalDecision.js:101 — minor/correctness (confirmed)

Fail closed when a select or rank request has no valid parsed options. parseApprovalRequest filters malformed option entries, so a JavaScript workflow such as options: [{ key: "safe" }] produces an empty request.options; both membership checks are then skipped and an arbitrary selection or ranking is accepted and persisted as workflow output. Return INVALID_REQUEST for option-bearing modes with no valid options, or preserve an option-parse error.

function validateApprovalDecision(request, decision) {
  if ((request.mode === "select" || request.mode === "rank") && request.options.length === 0) {
    return { ok: false, code: "INVALID_REQUEST", message: `${request.mode} approval has no valid options` };
  }
  if (request.mode === "select") {

packages/server/src/approvalDecision.js:113 — minor/correctness (confirmed)

Reject mixed-type ranking arrays instead of sanitizing only the validation copy. parseStringArray drops invalid members, so { ranked: ["canary", 7] } passes validation when canary is allowed, but approveNode serializes the original decision unchanged. The stored decisionJson then violates the declared string[] contract and different consumers can observe different rankings.

const payload = asObject(decision);
    const ranked = payload?.ranked;
    if (
      !Array.isArray(ranked) ||
      ranked.length === 0 ||
      ranked.some((value) => typeof value !== "string")
    ) {
      return { ok: false, code: "INVALID_REQUEST", message: "rank approvals require decision.ranked to be a non-empty string array" };
    }

packages/server/src/index.js:27 — minor/correctness (confirmed)

Re-export this binding. The package wildcard exposes approvalDecision.js at runtime while mapping its types to index.d.ts; because this file only imports the binding, TypeScript consumers of that subpath cannot import approvalDecision.

import { approvalDecision } from "./approvalDecision.js";
export { approvalDecision };

Generated by smithers review (Codex sol), findings adversarially verified.

roninjin10 and others added 2 commits July 30, 2026 14:30
…pe rankings

- select/rank validation rejects requests whose option entries were all
  dropped as malformed, instead of accepting arbitrary decisions
- rank validation rejects mixed-type ranked arrays outright so the
  persisted decisionJson honors the string[] contract
- re-export approvalDecision from the package index so TypeScript
  consumers can import it (regenerated index.d.ts)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
roninjin10 and others added 8 commits August 1, 2026 13:07
Co-Authored-By: Codex Sol <noreply@openai.com>
Co-Authored-By: Codex Sol <noreply@openai.com>
Co-Authored-By: Codex Sol <noreply@openai.com>
Co-Authored-By: Codex Sol <noreply@openai.com>
Co-Authored-By: Codex Sol <noreply@openai.com>
Co-Authored-By: Codex Sol <noreply@openai.com>
Co-Authored-By: Codex Sol <noreply@openai.com>
Co-Authored-By: Codex Sol <noreply@openai.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gateway approve route drops select-mode decision payloads ({selected, notes} lost)

1 participant