Skip to content

feat: generate and serve OpenAPI/Swagger spec (#55)#121

Merged
michaelmcnees merged 24 commits intomainfrom
feat/openapi-spec
Apr 5, 2026
Merged

feat: generate and serve OpenAPI/Swagger spec (#55)#121
michaelmcnees merged 24 commits intomainfrom
feat/openapi-spec

Conversation

@michaelmcnees
Copy link
Copy Markdown
Collaborator

@michaelmcnees michaelmcnees commented Apr 5, 2026

Summary

  • Generates a Swagger 2.0 spec from // @-prefixed annotations on all API handlers using swaggo/swag
  • Serves the spec at GET /api/v1/openapi.json (embedded into the binary via //go:embed)
  • Adds make spec target for local regeneration
  • Adds a CI Spec Freshness job that fails the build if committed spec drifts from annotations

Changes

  • internal/server/docs.go — global API metadata, security schemes, health endpoint stub annotations
  • internal/server/api.go — named request/response types, @Summary/@Description/@Param/@Success/@Failure/@Security/@Router on all handlers
  • internal/server/server.go — annotations on handleRun and handleCancel; route registration for /api/v1/openapi.json
  • internal/server/openapi.go//go:embed docs/swagger.json + handler
  • internal/server/docs/swagger.json + swagger.yaml — generated spec (13 paths, 21 types)
  • internal/budget/store.go — field-level comments on TeamBudget for spec clarity
  • packages/engine/Makefilespec target
  • .github/workflows/engine-ci.ymlSpec Freshness job

Test Plan

  • go test ./... passes
  • TestHandleOpenAPISpec validates the embedded spec is valid JSON with required OpenAPI fields
  • TestResponseTypesExported validates named response types serialize correctly
  • make spec regenerates without drift
  • CI spec freshness job re-runs swag and checks git diff --exit-code

Closes #55

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added GET /api/v1/openapi.json to serve the API contract.
    • Introduced automated multi-package release/tagging workflows and per-package release workflows for engine, Helm chart, and site.
    • Site now auto-deploys to Cloudflare Pages on main branch updates.
    • Helm chart publishing to OCI registry with automatic appVersion synchronization.
  • Documentation

    • Added release pipeline design and OpenAPI spec/design docs (including generated API spec).

michaelmcnees and others added 21 commits March 27, 2026 21:32
Change gitignore to only exclude superpowers plans (ephemeral) while
tracking specs (lasting decision records). Add release pipeline design
spec covering changesets, goreleaser, Helm OCI publish, and site deploy.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix race condition: sync appVersion and commit before pushing tags
- Add Dockerfile.goreleaser to avoid double-compilation in Docker builds
- Add concurrency control to release-tags.yml
- Add permissions block to site-deploy.yml
- Pin Trivy action to v0.28.0 instead of @master
- Update spec to reflect appVersion sync approach

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds `make spec` to regenerate the OpenAPI spec locally and a GitHub
Actions job that re-runs swag on CI and fails if the committed spec
has drifted from the annotations.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add @description to all 12 API handler annotations
- Add StatusResponse named type; replace map[string]string in budget
  mutation responses
- Add w.WriteHeader(http.StatusOK) to handleOpenAPISpec for consistency
- Add field-level comments to budget.TeamBudget (enforcement values,
  token limit units)
- Regenerate swagger.json/yaml

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 5, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dca2d3e1-4709-4e27-8e38-b7f22ba592ad

📥 Commits

Reviewing files that changed from the base of the PR and between df573f7 and ca4f2d3.

📒 Files selected for processing (1)
  • packages/engine/Makefile

📝 Walkthrough

Walkthrough

Adds a multi-package release pipeline (Changesets + release-please), tag-triggered engine and Helm release workflows, site deploy, OpenAPI generation and embedded spec serving, GoReleaser/Docker release config, workspace/package manifests, a Chart appVersion sync script, and multiple CI/workflow adjustments; deletes the legacy release workflow.

Changes

Cohort / File(s) Summary
Release Pipeline & Scripts
/.github/workflows/release-please.yml, /.github/workflows/release-tags.yml, /.github/workflows/release-engine.yml, /.github/workflows/release-helm.yml, /.github/scripts/sync-chart-appversion.sh
Add Changesets-driven version PR generation, per-package tag detection and creation, tag-triggered engine/helm release workflows, and a script to sync packages/helm-chart/Chart.yaml appVersion.
Site Deployment
/.github/workflows/site-deploy.yml, packages/site/*
New workflow to build site with Bun and deploy packages/site/dist to Cloudflare Pages on changes under packages/site/**.
CI Workflow Updates
/.github/workflows/claude.yml, /.github/workflows/engine-ci.yml
Adjust Claude workflow permissions and add a PR-comment step; modify govulncheck to filter specific findings and add a spec-freshness job that regenerates OpenAPI docs and fails on diffs.
Release Removal
/.github/workflows/release.yml
Remove legacy monolithic release workflow and its jobs.
Engine Release Config
packages/engine/.goreleaser.yaml, packages/engine/Dockerfile.goreleaser, packages/engine/Makefile
Add GoReleaser config, Dockerfile for runtime images, and spec Makefile target to run swag init.
OpenAPI / Server API
packages/engine/internal/server/api.go, .../docs.go, .../openapi.go, .../openapi_test.go, .../server.go, .../server_test.go, packages/engine/internal/server/docs/*
Export DTOs and typed request/response models, embed and serve generated Swagger JSON at /api/v1/openapi.json, add Swagger annotations and tests, and include generated YAML/JSON spec artifacts.
Module / Dependencies
packages/engine/go.mod
Add indirect dependencies required by swag/openapi generation.
Workspace & Package Manifests
package.json, packages/engine/package.json, packages/helm-chart/package.json
Enable root workspaces and add per-package package.json metadata.
Docs & Specs
docs/superpowers/specs/2026-03-31-release-pipeline-design.md, docs/superpowers/specs/2026-04-04-openapi-spec-design.md
Add design docs describing multi-package release pipeline and OpenAPI generation/spec decisions.
Misc
/.gitignore, packages/engine/internal/budget/store.go
Refine ignored paths and clarify TeamBudget field comments.

Sequence Diagram(s)

sequenceDiagram
    participant Dev as Developer
    participant Repo as Git Repo
    participant RP as release-please<br/>Workflow
    participant RT as release-tags<br/>Workflow
    participant Tag as Git Tag
    participant RE as release-engine<br/>Workflow
    participant RH as release-helm<br/>Workflow
    participant SD as site-deploy<br/>Workflow
    participant GHCR as GHCR / OCI Registry
    participant CF as Cloudflare Pages

    Dev->>Repo: Merge version PR (ci: version packages)
    activate RP
    RP->>Repo: create/update version PR (Changesets)
    deactivate RP

    Dev->>Repo: Commit "ci: version packages"
    activate RT
    RT->>Repo: detect package bumps & set outputs
    RT->>Repo: run sync-chart-appversion.sh if engine bumped
    RT->>Tag: create tags (engine/v*, helm-chart/v*, site/v*)
    deactivate RT

    rect rgba(100,150,200,0.5)
      Tag->>RE: push engine/v* tag
      activate RE
      RE->>GHCR: build & push multi-arch image
      RE->>RE: run Trivy scan
      deactivate RE

      Tag->>RH: push helm-chart/v* tag
      activate RH
      RH->>GHCR: package & push Helm OCI chart
      deactivate RH
    end

    Repo->>SD: push touching packages/site/**
    activate SD
    SD->>CF: build & deploy site to Cloudflare Pages
    deactivate SD
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐇 Hop, hop — tags take flight tonight,

Charts and engines set to right,
Swagger tucked inside the tree,
Pipelines hum — the rabbit grins with glee,
Binaries and charts sail free.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes comprehensive changes for OpenAPI support, but also introduces an extensive release pipeline refactor (7 new workflow files, 1 script, new configuration) that extends significantly beyond the stated objective of generating and serving an OpenAPI spec. Consider separating the release pipeline changes into a distinct PR. The OpenAPI feature itself is well-scoped, but the release automation represents substantial out-of-scope additions that should be reviewed independently.
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat: generate and serve OpenAPI/Swagger spec' accurately summarizes the main change: implementing OpenAPI spec generation and serving it at an API endpoint.
Linked Issues check ✅ Passed The PR addresses issue #55 by generating and serving an OpenAPI/Swagger specification for the REST API with named request/response types, handler annotations, and a Spec Freshness CI job to maintain accuracy. However, it generates Swagger 2.0 instead of the requested OpenAPI 3.1.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/openapi-spec

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 13

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/engine/internal/server/api.go (2)

501-518: ⚠️ Potential issue | 🟠 Major

Budget errors no longer match the published ErrorResponse contract.

These handlers now advertise ErrorResponse, but Lines 515, 540, 547, 552, 593, and 642 still use http.Error, which returns text/plain and exposes raw backend errors on 500s. That makes the spec false and leaks internal details to API consumers.

🔧 Example replacement pattern
-		http.Error(w, "invalid request body", http.StatusBadRequest)
+		writeJSONError(w, "invalid request body", http.StatusBadRequest)
@@
-		http.Error(w, err.Error(), http.StatusInternalServerError)
+		writeJSONError(w, "internal server error", http.StatusInternalServerError)

Apply the same pattern to the other http.Error branches in these handlers.

Also applies to: 521-553, 577-595, 616-644

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/api.go` around lines 501 - 518, The handlers
(e.g., handleListBudgets) currently call http.Error(w, err.Error(),
http.StatusInternalServerError) which returns text/plain and leaks internal
errors, violating the published ErrorResponse contract; replace each http.Error
branch with a JSON ErrorResponse payload (use the ErrorResponse type) and a
generic message, log the detailed err via the server logger (e.g., s.logger or
s.Logger.Errorf) for debugging, and send writeJSON(w,
http.StatusInternalServerError, ErrorResponse{Message: "internal server error"})
(or the project-standard message/fields) in functions like handleListBudgets and
the other handlers mentioned so the API returns the documented JSON error format
and does not expose backend details.

18-46: ⚠️ Potential issue | 🟠 Major

Make unset timestamps optional in the JSON contract.

Lines 24, 34, and 44 are pointer fields without omitempty, so pending executions/steps will serialize started_at: null when the value is unset, while the generated schema advertises a string. That mismatch will bleed into generated SDKs.

🧩 One safe way to align the wire format with the schema
 type ExecutionSummary struct {
 	ID          string  `json:"id"`
 	Workflow    string  `json:"workflow"`
 	Version     int     `json:"version"`
 	Status      string  `json:"status"`
-	StartedAt   *string `json:"started_at"`
+	StartedAt   *string `json:"started_at,omitempty"`
 	CompletedAt *string `json:"completed_at,omitempty"`
 }
@@
 type ExecutionDetail struct {
 	ID          string        `json:"id"`
 	Workflow    string        `json:"workflow"`
 	Version     int           `json:"version"`
 	Status      string        `json:"status"`
-	StartedAt   *string       `json:"started_at"`
+	StartedAt   *string       `json:"started_at,omitempty"`
 	CompletedAt *string       `json:"completed_at,omitempty"`
 	Steps       []StepSummary `json:"steps"`
 }
@@
 type StepSummary struct {
 	Name        string  `json:"name"`
 	Status      string  `json:"status"`
 	Error       string  `json:"error,omitempty"`
-	StartedAt   *string `json:"started_at"`
+	StartedAt   *string `json:"started_at,omitempty"`
 	CompletedAt *string `json:"completed_at,omitempty"`
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/api.go` around lines 18 - 46, The JSON
contract currently serializes nil start timestamps as null, mismatching the
generated schema; update the struct tags for the StartedAt fields on
ExecutionSummary (StartedAt), ExecutionDetail (StartedAt) and StepSummary
(StartedAt) to include `omitempty` so unset/pending timestamps are omitted from
JSON (CompletedAt already has omitempty), ensuring the wire format matches the
schema and SDKs.
🧹 Nitpick comments (9)
.github/workflows/engine-ci.yml (1)

86-87: Consider pinning swag version for CI reproducibility.

Using @latest may cause unexpected CI failures if swag releases a breaking change that alters the generated output format. Pinning to a specific version (e.g., @v1.16.6 matching go.mod) ensures consistent spec generation across CI runs.

📝 Suggested fix
       - name: Install swag
-        run: go install github.com/swaggo/swag/cmd/swag@latest
+        run: go install github.com/swaggo/swag/cmd/swag@v1.16.6
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/engine-ci.yml around lines 86 - 87, The CI step currently
installs swag with the unstable tag "@latest"; update the install command in the
engine-ci.yml workflow (the "Install swag" step) to pin a specific release
matching go.mod (for example use github.com/swaggo/swag/cmd/swag@v1.16.6) so
that the swag binary version is reproducible across CI runs.
.github/scripts/sync-chart-appversion.sh (2)

7-7: Consider adding input validation for clarity.

While the script will fail with unbound variable due to set -u if $1 is missing, an explicit check with a helpful error message would improve developer experience.

📝 Optional: Add explicit argument validation
+if [[ -z "${1:-}" ]]; then
+  echo "Usage: $0 <engine-version>"
+  echo "Example: $0 0.4.1"
+  exit 1
+fi
+
 ENGINE_VERSION="$1"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/scripts/sync-chart-appversion.sh at line 7, Add explicit argument
validation at the top of the sync-chart-appversion.sh script to check that
ENGINE_VERSION (derived from "$1") is provided; if empty or missing, print a
clear usage/error message and exit with a non-zero status. Locate the assignment
to ENGINE_VERSION and add a guard that tests the variable (or "$1") and emits a
helpful message like "Usage: sync-chart-appversion.sh <engine_version>" before
exiting, so callers get a descriptive error instead of relying on set -u.

15-16: Verify sed -i portability if local execution is expected.

GNU sed -i (Linux) and BSD sed -i (macOS) have incompatible syntax. Since this script runs in CI on ubuntu-latest, it will work correctly there. However, if developers might run this locally on macOS, the sed -i call will fail without an empty string argument (sed -i '').

If local execution is expected, consider using a portable pattern or documenting the Linux-only requirement.

📝 Optional: Portable sed in-place edit
-sed -i "s/^appVersion:.*/appVersion: \"${ENGINE_VERSION}\"/" "$CHART_FILE"
+sed -i.bak "s/^appVersion:.*/appVersion: \"${ENGINE_VERSION}\"/" "$CHART_FILE" && rm -f "$CHART_FILE.bak"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/scripts/sync-chart-appversion.sh around lines 15 - 16, The sed -i
call is not portable to macOS; update the sed invocation around the sed line
that edits CHART_FILE (which uses ENGINE_VERSION and CHART_FILE) to support both
GNU and BSD sed: detect the OS via uname (e.g., if uname = "Darwin" set
SED_INPLACE=" -i ''" else SED_INPLACE="-i") or set a portable fallback by
writing to a temp file and moving it back (use mktemp, run sed without -i and
redirect, then mv). Replace the current sed -i "s/^appVersion:.*/appVersion:
\"${ENGINE_VERSION}\"/" "$CHART_FILE" with the chosen portable approach and keep
the echo line intact.
.github/workflows/site-deploy.yml (1)

6-8: Include shared dependency/config files in the deploy trigger.

Right now deploy runs only for packages/site/**. If shared lockfiles or root workspace metadata change, site output can drift without triggering deployment.

Proposed trigger expansion
 on:
   push:
     branches: [main]
     paths:
       - 'packages/site/**'
+      - 'package.json'
+      - 'bun.lock'
+      - 'bun.lockb'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/site-deploy.yml around lines 6 - 8, The deploy workflow's
paths currently only include 'packages/site/**', so changes to shared lockfiles
or workspace config won't trigger deploy; update the paths array in the workflow
(the paths key containing 'packages/site/**') to also include root/shared files
such as package.json, package-lock.json, yarn.lock, pnpm-lock.yaml,
pnpm-workspace.yaml, turbo.json (or other monorepo config), and workspace
package manifests like 'packages/**/package.json' so that changes to dependency
or workspace metadata trigger the site-deploy job.
packages/engine/internal/server/docs/swagger.yaml (5)

334-342: Define enum constraints on status response fields.

The status query parameter on GET /api/v1/executions defines an enum with values pending, running, completed, failed, cancelled (lines 334-339), but the status fields in ExecutionSummary, ExecutionDetail, and StepSummary are plain strings without enum constraints. This inconsistency makes the spec ambiguous—are those the only valid values?

🏷️ Recommended fix
 server.ExecutionSummary:
   properties:
     # ... other fields
     status:
+      enum:
+        - pending
+        - running
+        - completed
+        - failed
+        - cancelled
       type: string

Apply to server.ExecutionDetail.status and server.StepSummary.status (though step status values might differ).

Also applies to: 39-40, 66-66, 118-119

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/docs/swagger.yaml` around lines 334 - 342,
The OpenAPI spec is inconsistent: the query param enum lists allowed statuses
but the response fields are plain strings; update the response schemas to
constrain status to the same enum. Modify server.ExecutionSummary.status,
server.ExecutionDetail.status, and server.StepSummary.status to use an enum
(values: pending, running, completed, failed, cancelled) or refactor to a shared
Status schema and reference it from those fields so the response types match the
GET /api/v1/executions query enum.

3-178: Consider adding descriptions to all properties.

Only 4 properties across all definitions have descriptions (enforcement, monthly_token_limit, provider, and enforcement in SetBudgetRequest). Descriptions improve generated SDK documentation and help API consumers understand the contract without reading source code.

Example fields that would benefit:

  • id, team_id, execution_id: Document format (UUID? arbitrary string?)
  • status: Document possible values
  • workflow: Workflow name or identifier?
  • steps, executions, workflows: Brief description of what's included
  • error: Structure of error messages
  • details in ReadyzResponse: What keys are present?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/docs/swagger.yaml` around lines 3 - 178, Add
human-readable description fields for all properties in the OpenAPI definitions
to improve SDK/docs: update definitions like budget.TeamBudget (id, team_id,
monthly_token_limit already has one but add format/units), server.CancelResponse
(execution_id, status), server.ErrorResponse (error), server.ExecutionDetail and
server.ExecutionSummary (id, started_at, completed_at, status, version,
workflow), server.ExecutionListResponse (executions), server.HealthResponse
(status), server.ReadyzResponse (status, details — describe expected keys),
server.RunResponse (execution_id, workflow, version), server.SetBudgetRequest
(enforcement already has text but clarify allowed values; monthly_token_limit
units), server.StepSummary (name, status, started_at, completed_at, error),
server.UsageResponse (provider, prompt_tokens, completion_tokens, total_tokens,
period_start), server.WorkflowDetailResponse (name, version, definition),
server.WorkflowListResponse (workflows), server.WorkflowVersionListResponse
(name, versions) and workflow.* summaries; for fields like
id/execution_id/team_id indicate format (e.g., UUID vs string), for status
enumerate possible values (e.g., running, completed, failed), for workflow
explain it’s a workflow name/identifier, and for arrays (steps, executions,
workflows, versions) briefly describe what each item contains. Update the YAML
property blocks to include description strings accordingly so generated SDK docs
and consumers get clear expectations.

10-12: Add format and constraints to integer fields.

Integer fields like monthly_token_limit, version, token counts, and limit lack format specifications (int32 vs int64) and validation constraints (minimum, maximum). This can lead to SDK generation issues and doesn't document valid ranges for consumers.

🔢 Example improvements
 monthly_token_limit:
   description: Maximum tokens allowed per billing period
+  format: int64
+  minimum: 0
   type: integer
 version:
+  description: Immutable version number
+  format: int32
+  minimum: 1
   type: integer
 limit:
   description: Max results (default 20)
+  format: int32
+  minimum: 1
+  maximum: 1000
   type: integer

Also applies to: 45-46, 67-68, 90-91, 100-101, 123-133, 165-166, 172-173

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/docs/swagger.yaml` around lines 10 - 12, The
integer schema properties (e.g., monthly_token_limit, version, token counts,
limit) lack format and validation constraints; update each integer property in
swagger.yaml to include an appropriate "format" (use int64 for token-related
counts like monthly_token_limit and other token_* fields, int32 for small
counters like version/limit where applicable) and add "minimum" (e.g., 0) and
sensible "maximum" bounds (e.g., a large upper bound for token counts) so SDK
generators and consumers have explicit types and valid ranges; ensure you apply
these changes to the listed properties (monthly_token_limit, version, token
counts, limit and the other referenced blocks at the indicated locations).

42-46: Consider adding maxItems constraints to array fields.

Arrays in response schemas (steps, executions, workflows, versions) lack maxItems constraints. While not strictly required, adding these constraints documents expected cardinality and can help SDK generators allocate appropriately sized collections.

📊 Example
 steps:
+  maxItems: 1000
   items:
     $ref: '#/definitions/server.StepSummary'
   type: array
 executions:
+  maxItems: 1000
   items:
     $ref: '#/definitions/server.ExecutionSummary'
   type: array

Also applies to: 52-55, 145-148, 154-157

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/docs/swagger.yaml` around lines 42 - 46, The
array properties 'steps', 'executions', 'workflows', and 'versions' in the
Swagger schema lack maxItems constraints; update each array schema (e.g., the
'steps' array that currently references '#/definitions/server.StepSummary' and
the other array definitions at the referenced blocks) to include a sensible
"maxItems" value (or a documented constant like 100/1000) directly under the
array object, so the schema reads: type: array, items: ..., maxItems: <number>;
apply the same change to the other occurrences mentioned (lines corresponding to
the 'executions', 'workflows', and 'versions' array definitions).

568-578: Add a global security field for consistency.

The specification defines two security schemes (ApiKeyAuth and OIDCAuth), both using the Authorization header, but no global security field is defined at the root level—though each operation does declare security: [ApiKeyAuth: [], OIDCAuth: []] as an OR relationship.

Token distinction appears to be prefix-based: the rate limiting middleware uses the first 10 characters of the Authorization header to identify the key ("Bearer mk_" for API keys vs. "Bearer <jwt>" for JWTs).

Consider adding a global security field to serve as the default for all operations:

security:
  - ApiKeyAuth: []
  - OIDCAuth: []

This improves consistency and makes the authentication requirements explicit at the API level, reducing duplication across operations.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/docs/swagger.yaml` around lines 568 - 578,
Add a root-level default security declaration so the OpenAPI file uses the
defined security schemes consistently: add a top-level "security" field that
lists ApiKeyAuth and OIDCAuth as alternatives (e.g., security: - ApiKeyAuth: []
- OIDCAuth: []). Update the swagger.yaml near the existing securityDefinitions
block so the file-level default applies to all operations (while per-operation
security can still override), referencing the existing ApiKeyAuth and OIDCAuth
scheme names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/claude.yml:
- Around line 26-31: The claude job currently grants pull-requests: write for
the whole job; remove or change pull-requests: write in the claude job's
permissions block to read-only (keep contents: read, issues: read, id-token:
write, actions: read as needed) and create a second job (e.g., a follow_up or
code_rabbit_comment job) that runs after claude and contains only the CodeRabbit
comment step; give that new job the minimal permissions including pull-requests:
write and required checkout/token access and set needs: claude so the write
token is scoped only to the job that posts the PR comment.

In @.github/workflows/release-tags.yml:
- Around line 18-19: Replace the brittle
startsWith(github.event.head_commit.message, 'ci: version packages') + HEAD~1
comparisons with checks based on the push base using github.event.before: use
github.event.before as the base ref/commit to diff against (instead of
head_commit.message or HEAD~1) in the version-detection step and in any
subsequent comparisons (the logic currently referencing
startsWith(github.event.head_commit.message, ...) and the HEAD~1 parent-diff).
Update the detection step to no-op when github.event.before equals github.sha
(no changes), and use git diff --name-only ${{ github.event.before }}..${{
github.sha }} (or the equivalent action inputs) to reliably detect
package/version changes across merges, rebases, and multi-commit pushes.

In @.gitignore:
- Around line 49-52: The new .gitignore entries are too broad and seem unrelated
to the OpenAPI work; narrow or document them by either anchoring the patterns or
explaining they are intentionally global: replace the unanchored "plans/" and
"marketing/variants/" with path-anchored patterns (e.g.,
"/docs/superpowers/plans/" or "/marketing/variants/") if they should only ignore
those specific locations, or add a short comment above the patterns clarifying
that "plans/" and "marketing/variants/" are meant to be global ignores; update
the entries referencing "docs/superpowers/plans/" or the pattern names so
reviewers can see the intended scope.

In `@packages/engine/.goreleaser.yaml`:
- Around line 62-85: The alias docker manifest entries under docker_manifests
(the name_template entries "ghcr.io/dvflw/mantle:{{ .Major }}.{{ .Minor }}",
"ghcr.io/dvflw/mantle:{{ .Major }}" and "ghcr.io/dvflw/mantle:latest") are being
pushed for prerelease tags; add skip_push: auto to each of those manifest blocks
so they are skipped for prereleases when prerelease: auto is set. Edit the three
manifest objects in the docker_manifests section to include skip_push: auto
directly under their image_templates blocks (leave the primary {{ .Version }}
manifest unchanged).

In `@packages/engine/internal/server/docs.go`:
- Line 11: Remove the hard-coded `@host` directive from the embedded OpenAPI doc
so the served spec does not lock clients to "localhost:8080"; specifically
delete the commented annotation line containing "@host localhost:8080" (the
`@host` annotation in the docs embedding) so the server will serve a host-agnostic
spec and consumers will use the origin that served /api/v1/openapi.json.

In `@packages/engine/internal/server/docs/swagger.json`:
- Around line 1-2: The file still uses the Swagger 2.0 artifact ("swagger":
"2.0") so the API contract must be migrated to OpenAPI 3.x: replace the
top-level "swagger" key with "openapi": "3.0.3" and migrate Swagger 2 constructs
to their OpenAPI 3 equivalents (move "definitions" → "components.schemas",
convert "parameters"/"responses" to components if present). Also add a
components.securitySchemes.BearerAuth entry using type: http, scheme: bearer,
bearerFormat: JWT and reference it in security at the global or operation level,
and update schema nullable handling to OpenAPI 3 style (use "nullable": true or
type arrays like ["string","null"] where appropriate) so /api/v1/openapi.json
serves a true OpenAPI 3.x contract.

In `@packages/engine/internal/server/docs/swagger.yaml`:
- Around line 33-34: The timestamp properties (completed_at, started_at,
created_at, updated_at, period_start) are currently declared as type: string
without a format; update each of these field definitions in swagger.yaml to
include format: date-time so SDK generators and consumers know these are ISO
8601 timestamps (apply to all occurrences of completed_at, started_at,
created_at, updated_at, period_start referenced in the diff and elsewhere in the
file).
- Around line 237-270: The PUT operation for "Set provider budget" (summary: Set
provider budget, request schema: '#/definitions/server.SetBudgetRequest') is
missing Swagger 2.0's required operation-level consumes and produces; update
this PUT to include consumes: ["application/json"] and produces:
["application/json"] and apply the same change to the POST operations for cancel
(POST /api/v1/cancel/{execution}, summary or path reference) and run (POST
/api/v1/run/{workflow}), and consider adding produces: ["application/json"] to
other endpoints for consistency.
- Around line 324-369: The response schema for the GET /api/v1/executions
endpoint is missing pagination metadata: update the server.ExecutionListResponse
definition to include pagination fields (e.g., next_cursor (string|null),
has_more (boolean), and total_count (integer) or page_size) so clients can page
results, and add an optional query parameter (e.g., cursor or offset) to the
endpoint parameters alongside limit; ensure the new fields are referenced in the
responses section for /api/v1/executions and that the names match the
ExecutionListResponse schema (use ExecutionListResponse, next_cursor, has_more,
total_count, and the new query param name such as cursor).
- Around line 3-18: Add explicit OpenAPI `required` arrays to each schema (e.g.,
budget.TeamBudget) listing the non-nullable property names so clients know which
fields must be present; inspect the corresponding Go structs (fields like
TeamBudget.provider, TeamBudget.id, server.ExecutionDetail.id, etc.) to
determine actual nullability and populate `required: [ ... ]` accordingly, then
repeat this pattern for all other schema definitions referenced in the review
(the blocks around lines 31-49, 57-71, 86-94, 103-107, 134-142, 150-158) to
ensure every object schema includes its mandatory properties.
- Around line 136-137: The OpenAPI schema currently defines
WorkflowDetailResponse.definition as a bare object; update it to explicitly
document the structure by replacing the anonymous object with either a free-form
map (e.g., type: object, additionalProperties: true) if the workflow definition
is arbitrary, or by creating a named schema (e.g., workflow.WorkflowDefinition)
that lists known properties (steps, triggers, etc.) and then reference it from
WorkflowDetailResponse.definition; ensure the YAML uses $ref:
'#/components/schemas/workflow.WorkflowDefinition' when using the named schema
and add clear property definitions for any known fields or additionalProperties:
true for free-form content.
- Line 1: The file is written as Swagger 2.0; switch it to OpenAPI 3.1 by
replacing the root Swagger fields (remove "swagger: '2.0'", "host", "basePath",
"schemes", "consumes", "produces") and add "openapi: 3.1.0" plus a "servers"
array with environment variables (use server variables if needed) to replace the
hardcoded host; update endpoint request/response media-type declarations to use
"requestBody" with "content" and response "content" instead of Swagger
"consumes"/"produces"; update timestamp properties like "completed_at" and
"started_at" in the schemas to "type: string" and "format: date-time"; replace
the vague "WorkflowDetailResponse.definition" object with a full schema object
(explicit properties and types) or a $ref to a defined schema so the contract is
explicit; ensure securitySchemes are migrated to components.securitySchemes per
OpenAPI 3.1.

In `@packages/engine/internal/server/openapi_test.go`:
- Around line 25-27: The test currently accepts either Swagger 2.0 or OpenAPI by
checking doc["swagger"] or doc["openapi"]; tighten it to enforce OpenAPI 3.1 by
requiring doc["openapi"] to be present and to be a string beginning with "3.1"
(convert doc["openapi"] to a string and check HasPrefix "3.1"). Update the
failure message to state that an OpenAPI 3.1 document is required. Refer to the
local doc variable and the test in openapi_test.go when making this change.

---

Outside diff comments:
In `@packages/engine/internal/server/api.go`:
- Around line 501-518: The handlers (e.g., handleListBudgets) currently call
http.Error(w, err.Error(), http.StatusInternalServerError) which returns
text/plain and leaks internal errors, violating the published ErrorResponse
contract; replace each http.Error branch with a JSON ErrorResponse payload (use
the ErrorResponse type) and a generic message, log the detailed err via the
server logger (e.g., s.logger or s.Logger.Errorf) for debugging, and send
writeJSON(w, http.StatusInternalServerError, ErrorResponse{Message: "internal
server error"}) (or the project-standard message/fields) in functions like
handleListBudgets and the other handlers mentioned so the API returns the
documented JSON error format and does not expose backend details.
- Around line 18-46: The JSON contract currently serializes nil start timestamps
as null, mismatching the generated schema; update the struct tags for the
StartedAt fields on ExecutionSummary (StartedAt), ExecutionDetail (StartedAt)
and StepSummary (StartedAt) to include `omitempty` so unset/pending timestamps
are omitted from JSON (CompletedAt already has omitempty), ensuring the wire
format matches the schema and SDKs.

---

Nitpick comments:
In @.github/scripts/sync-chart-appversion.sh:
- Line 7: Add explicit argument validation at the top of the
sync-chart-appversion.sh script to check that ENGINE_VERSION (derived from "$1")
is provided; if empty or missing, print a clear usage/error message and exit
with a non-zero status. Locate the assignment to ENGINE_VERSION and add a guard
that tests the variable (or "$1") and emits a helpful message like "Usage:
sync-chart-appversion.sh <engine_version>" before exiting, so callers get a
descriptive error instead of relying on set -u.
- Around line 15-16: The sed -i call is not portable to macOS; update the sed
invocation around the sed line that edits CHART_FILE (which uses ENGINE_VERSION
and CHART_FILE) to support both GNU and BSD sed: detect the OS via uname (e.g.,
if uname = "Darwin" set SED_INPLACE=" -i ''" else SED_INPLACE="-i") or set a
portable fallback by writing to a temp file and moving it back (use mktemp, run
sed without -i and redirect, then mv). Replace the current sed -i
"s/^appVersion:.*/appVersion: \"${ENGINE_VERSION}\"/" "$CHART_FILE" with the
chosen portable approach and keep the echo line intact.

In @.github/workflows/engine-ci.yml:
- Around line 86-87: The CI step currently installs swag with the unstable tag
"@latest"; update the install command in the engine-ci.yml workflow (the
"Install swag" step) to pin a specific release matching go.mod (for example use
github.com/swaggo/swag/cmd/swag@v1.16.6) so that the swag binary version is
reproducible across CI runs.

In @.github/workflows/site-deploy.yml:
- Around line 6-8: The deploy workflow's paths currently only include
'packages/site/**', so changes to shared lockfiles or workspace config won't
trigger deploy; update the paths array in the workflow (the paths key containing
'packages/site/**') to also include root/shared files such as package.json,
package-lock.json, yarn.lock, pnpm-lock.yaml, pnpm-workspace.yaml, turbo.json
(or other monorepo config), and workspace package manifests like
'packages/**/package.json' so that changes to dependency or workspace metadata
trigger the site-deploy job.

In `@packages/engine/internal/server/docs/swagger.yaml`:
- Around line 334-342: The OpenAPI spec is inconsistent: the query param enum
lists allowed statuses but the response fields are plain strings; update the
response schemas to constrain status to the same enum. Modify
server.ExecutionSummary.status, server.ExecutionDetail.status, and
server.StepSummary.status to use an enum (values: pending, running, completed,
failed, cancelled) or refactor to a shared Status schema and reference it from
those fields so the response types match the GET /api/v1/executions query enum.
- Around line 3-178: Add human-readable description fields for all properties in
the OpenAPI definitions to improve SDK/docs: update definitions like
budget.TeamBudget (id, team_id, monthly_token_limit already has one but add
format/units), server.CancelResponse (execution_id, status),
server.ErrorResponse (error), server.ExecutionDetail and server.ExecutionSummary
(id, started_at, completed_at, status, version, workflow),
server.ExecutionListResponse (executions), server.HealthResponse (status),
server.ReadyzResponse (status, details — describe expected keys),
server.RunResponse (execution_id, workflow, version), server.SetBudgetRequest
(enforcement already has text but clarify allowed values; monthly_token_limit
units), server.StepSummary (name, status, started_at, completed_at, error),
server.UsageResponse (provider, prompt_tokens, completion_tokens, total_tokens,
period_start), server.WorkflowDetailResponse (name, version, definition),
server.WorkflowListResponse (workflows), server.WorkflowVersionListResponse
(name, versions) and workflow.* summaries; for fields like
id/execution_id/team_id indicate format (e.g., UUID vs string), for status
enumerate possible values (e.g., running, completed, failed), for workflow
explain it’s a workflow name/identifier, and for arrays (steps, executions,
workflows, versions) briefly describe what each item contains. Update the YAML
property blocks to include description strings accordingly so generated SDK docs
and consumers get clear expectations.
- Around line 10-12: The integer schema properties (e.g., monthly_token_limit,
version, token counts, limit) lack format and validation constraints; update
each integer property in swagger.yaml to include an appropriate "format" (use
int64 for token-related counts like monthly_token_limit and other token_*
fields, int32 for small counters like version/limit where applicable) and add
"minimum" (e.g., 0) and sensible "maximum" bounds (e.g., a large upper bound for
token counts) so SDK generators and consumers have explicit types and valid
ranges; ensure you apply these changes to the listed properties
(monthly_token_limit, version, token counts, limit and the other referenced
blocks at the indicated locations).
- Around line 42-46: The array properties 'steps', 'executions', 'workflows',
and 'versions' in the Swagger schema lack maxItems constraints; update each
array schema (e.g., the 'steps' array that currently references
'#/definitions/server.StepSummary' and the other array definitions at the
referenced blocks) to include a sensible "maxItems" value (or a documented
constant like 100/1000) directly under the array object, so the schema reads:
type: array, items: ..., maxItems: <number>; apply the same change to the other
occurrences mentioned (lines corresponding to the 'executions', 'workflows', and
'versions' array definitions).
- Around line 568-578: Add a root-level default security declaration so the
OpenAPI file uses the defined security schemes consistently: add a top-level
"security" field that lists ApiKeyAuth and OIDCAuth as alternatives (e.g.,
security: - ApiKeyAuth: [] - OIDCAuth: []). Update the swagger.yaml near the
existing securityDefinitions block so the file-level default applies to all
operations (while per-operation security can still override), referencing the
existing ApiKeyAuth and OIDCAuth scheme names.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5a25819e-391d-4376-a771-97865728b468

📥 Commits

Reviewing files that changed from the base of the PR and between 13540d9 and 593309f.

⛔ Files ignored due to path filters (1)
  • packages/engine/go.sum is excluded by !**/*.sum
📒 Files selected for processing (28)
  • .github/scripts/sync-chart-appversion.sh
  • .github/workflows/claude.yml
  • .github/workflows/engine-ci.yml
  • .github/workflows/release-engine.yml
  • .github/workflows/release-helm.yml
  • .github/workflows/release-please.yml
  • .github/workflows/release-tags.yml
  • .github/workflows/release.yml
  • .github/workflows/site-deploy.yml
  • .gitignore
  • docs/superpowers/specs/2026-03-31-release-pipeline-design.md
  • docs/superpowers/specs/2026-04-04-openapi-spec-design.md
  • package.json
  • packages/engine/.goreleaser.yaml
  • packages/engine/Dockerfile.goreleaser
  • packages/engine/Makefile
  • packages/engine/go.mod
  • packages/engine/internal/budget/store.go
  • packages/engine/internal/server/api.go
  • packages/engine/internal/server/docs.go
  • packages/engine/internal/server/docs/swagger.json
  • packages/engine/internal/server/docs/swagger.yaml
  • packages/engine/internal/server/openapi.go
  • packages/engine/internal/server/openapi_test.go
  • packages/engine/internal/server/server.go
  • packages/engine/internal/server/server_test.go
  • packages/engine/package.json
  • packages/helm-chart/package.json
💤 Files with no reviewable changes (1)
  • .github/workflows/release.yml

Comment on lines 26 to 31
permissions:
contents: read
pull-requests: read
pull-requests: write
issues: read
id-token: write
actions: read
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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In GitHub Actions, do job-level permissions apply to all steps (including third-party actions), and is splitting write actions into a separate job the recommended least-privilege pattern?

💡 Result:

Yes, job-level permissions in GitHub Actions apply to all steps within that job, including third-party actions, as they all inherit the job's GITHUB_TOKEN permissions. Yes, splitting jobs so that those requiring write permissions are separate from read-only jobs (especially those using third-party actions) is a recommended least-privilege security pattern to minimize blast radius if an action is compromised.

Citations:


🏁 Script executed:

cd /
find . -name "claude.yml" -path "*/.github/workflows/*" 2>/dev/null | head -20

Repository: dvflw/mantle

Length of output: 104


🏁 Script executed:

cat -n .github/workflows/claude.yml

Repository: dvflw/mantle

Length of output: 3316


🌐 Web query:

GitHub Actions needs context syntax needs.job_id.outcome vs result

💡 Result:

In GitHub Actions, the needs context provides access to the status of prerequisite jobs specified in the needs key of a job. The syntax is needs.<job_id>.result, where <job_id> is the ID of the dependent job (e.g., needs.build.result). - needs.<job_id>.result: The final result of the dependent job after applying continue-on-error (possible values: success, failure, cancelled, skipped). This corresponds to the job's "conclusion". There is no needs.<job_id>.outcome property documented in the official GitHub Actions contexts reference. The term "outcome" is used for steps (e.g., steps.<step_id>.outcome is the raw result before continue-on-error), but not for jobs in the needs context. The job context (for the current job) contains job.status (e.g., in_progress, completed) but no outcome or result properties matching the query. For dependent jobs, use needs.<job_id>.result. Example usage in a workflow: jobs: build: runs-on: ubuntu-latest # ... steps ... test: runs-on: ubuntu-latest needs: build if: ${{ needs.build.result == 'success' }} # ... steps ... Note: An open issue notes that needs.<job_id>.result uses the conclusion (post-continue-on-error), not raw outcome, which may be relevant for continue-on-error: true jobs.

Citations:


Limit write token scope to a dedicated follow-up job

Line 28 grants pull-requests: write to the entire claude job, which also runs a third-party action (Line 40). This unnecessarily expands write-token exposure. Keep this job read-only and move the CodeRabbit comment step into a second job with only pull-requests: write.

Suggested refactor
 jobs:
   claude:
     if: |
       (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude') &&
         (github.event.comment.user.login == github.repository_owner || github.event.comment.author_association == 'OWNER' || github.event.comment.author_association == 'MEMBER' || github.event.comment.author_association == 'COLLABORATOR')) ||
       (github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude') &&
         (github.event.comment.user.login == github.repository_owner || github.event.comment.author_association == 'OWNER' || github.event.comment.author_association == 'MEMBER' || github.event.comment.author_association == 'COLLABORATOR')) ||
       (github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude') &&
         (github.event.review.user.login == github.repository_owner || github.event.review.author_association == 'OWNER' || github.event.review.author_association == 'MEMBER' || github.event.review.author_association == 'COLLABORATOR')) ||
       (github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude')) &&
         (github.event.issue.user.login == github.repository_owner || github.event.issue.author_association == 'OWNER' || github.event.issue.author_association == 'MEMBER' || github.event.issue.author_association == 'COLLABORATOR'))
     runs-on: ubuntu-latest
     timeout-minutes: 30
     permissions:
       contents: read
-      pull-requests: write
+      pull-requests: read
       issues: read
       id-token: write
       actions: read
     steps:
       - name: Checkout repository
         uses: actions/checkout@v4
         with:
           fetch-depth: 1

       - name: Run Claude Code
         id: claude
         uses: anthropics/claude-code-action@094bd24d575e7b30ac1576024817bf1a97c81262 # v1
         with:
           claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
           additional_permissions: |
             actions: read

-      - name: Request CodeRabbit re-review
-        if: >-
-          steps.claude.outcome == 'success' && (
-            github.event.issue.pull_request ||
-            github.event_name == 'pull_request_review_comment' ||
-            github.event_name == 'pull_request_review'
-          )
-        env:
-          GH_TOKEN: ${{ github.token }}
-          PR_NUMBER: >-
-            ${{
-              github.event.issue.number ||
-              github.event.pull_request.number
-            }}
-        run: |
-          gh pr comment "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" \
-            --body "@coderabbitai review"
+
+  request-coderabbit-review:
+    needs: claude
+    if: >-
+      needs.claude.result == 'success' && (
+        github.event.issue.pull_request ||
+        github.event_name == 'pull_request_review_comment' ||
+        github.event_name == 'pull_request_review'
+      )
+    runs-on: ubuntu-latest
+    permissions:
+      pull-requests: write
+    steps:
+      - name: Request CodeRabbit re-review
+        env:
+          GH_TOKEN: ${{ github.token }}
+          PR_NUMBER: >-
+            ${{
+              github.event.issue.number ||
+              github.event.pull_request.number
+            }}
+        run: |
+          gh pr comment "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" \
+            --body "@coderabbitai review"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/claude.yml around lines 26 - 31, The claude job currently
grants pull-requests: write for the whole job; remove or change pull-requests:
write in the claude job's permissions block to read-only (keep contents: read,
issues: read, id-token: write, actions: read as needed) and create a second job
(e.g., a follow_up or code_rabbit_comment job) that runs after claude and
contains only the CodeRabbit comment step; give that new job the minimal
permissions including pull-requests: write and required checkout/token access
and set needs: claude so the write token is scoped only to the job that posts
the PR comment.

Comment on lines +18 to +19
# Only run on Version PR merges
if: "startsWith(github.event.head_commit.message, 'ci: version packages')"
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.

⚠️ Potential issue | 🟠 Major

Use the push base instead of head_commit.message + HEAD~1.

Line 19 only works for one commit-message shape, and Lines 33-50 only compare the last parent commit. Merge commits, rebase merges, or multi-commit pushes from the version PR can therefore miss real version bumps and skip tagging entirely. Compare against ${{ github.event.before }} and let the detection step no-op when nothing changed.

🛠️ Safer version-base detection
-    # Only run on Version PR merges
-    if: "startsWith(github.event.head_commit.message, 'ci: version packages')"
     steps:
       - uses: actions/checkout@v4
         with:
-          fetch-depth: 2
+          fetch-depth: 0
@@
-          ENGINE_OLD=$(git show HEAD~1:packages/engine/package.json 2>/dev/null | jq -r '.version' 2>/dev/null || echo "")
+          ENGINE_OLD=$(git show "${{ github.event.before }}:packages/engine/package.json" 2>/dev/null | jq -r '.version' 2>/dev/null || echo "")
@@
-          HELM_OLD=$(git show HEAD~1:packages/helm-chart/package.json 2>/dev/null | jq -r '.version' 2>/dev/null || echo "")
+          HELM_OLD=$(git show "${{ github.event.before }}:packages/helm-chart/package.json" 2>/dev/null | jq -r '.version' 2>/dev/null || echo "")
@@
-          SITE_OLD=$(git show HEAD~1:packages/site/package.json 2>/dev/null | jq -r '.version' 2>/dev/null || echo "")
+          SITE_OLD=$(git show "${{ github.event.before }}:packages/site/package.json" 2>/dev/null | jq -r '.version' 2>/dev/null || echo "")

Also applies to: 21-61

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/release-tags.yml around lines 18 - 19, Replace the brittle
startsWith(github.event.head_commit.message, 'ci: version packages') + HEAD~1
comparisons with checks based on the push base using github.event.before: use
github.event.before as the base ref/commit to diff against (instead of
head_commit.message or HEAD~1) in the version-detection step and in any
subsequent comparisons (the logic currently referencing
startsWith(github.event.head_commit.message, ...) and the HEAD~1 parent-diff).
Update the detection step to no-op when github.event.before equals github.sha
(no changes), and use git diff --name-only ${{ github.event.before }}..${{
github.sha }} (or the equivalent action inputs) to reliably detect
package/version changes across merges, rebases, and multi-commit pushes.

Comment on lines +49 to 52
# Internal development process (ephemeral implementation plans)
docs/superpowers/plans/
plans/
marketing/variants/
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.

⚠️ Potential issue | 🟡 Minor

Unrelated change: Consider scoping ignore patterns more narrowly.

This .gitignore update appears unrelated to the PR's stated objectives (OpenAPI spec generation). Additionally, the patterns on lines 51-52 lack path anchors and will match directories anywhere in the repository:

  • plans/ → ignores any directory named plans/ at any depth
  • marketing/variants/ → ignores this pattern anywhere in the repo

If these are only intended to ignore specific paths (e.g., under docs/superpowers/), consider anchoring them:

 # Internal development process (ephemeral implementation plans)
 docs/superpowers/plans/
-plans/
-marketing/variants/
+docs/superpowers/marketing/variants/

Or if plans/ and marketing/variants/ are intentionally global, clarify this with comments.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Internal development process (ephemeral implementation plans)
docs/superpowers/plans/
plans/
marketing/variants/
# Internal development process (ephemeral implementation plans)
docs/superpowers/plans/
docs/superpowers/marketing/variants/
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.gitignore around lines 49 - 52, The new .gitignore entries are too broad
and seem unrelated to the OpenAPI work; narrow or document them by either
anchoring the patterns or explaining they are intentionally global: replace the
unanchored "plans/" and "marketing/variants/" with path-anchored patterns (e.g.,
"/docs/superpowers/plans/" or "/marketing/variants/") if they should only ignore
those specific locations, or add a short comment above the patterns clarifying
that "plans/" and "marketing/variants/" are meant to be global ignores; update
the entries referencing "docs/superpowers/plans/" or the pattern names so
reviewers can see the intended scope.

Comment on lines +62 to +85
docker_manifests:
- name_template: "ghcr.io/dvflw/mantle:{{ .Version }}"
image_templates:
- "ghcr.io/dvflw/mantle:{{ .Version }}-amd64"
- "ghcr.io/dvflw/mantle:{{ .Version }}-arm64"
- name_template: "ghcr.io/dvflw/mantle:{{ .Major }}.{{ .Minor }}"
image_templates:
- "ghcr.io/dvflw/mantle:{{ .Version }}-amd64"
- "ghcr.io/dvflw/mantle:{{ .Version }}-arm64"
- name_template: "ghcr.io/dvflw/mantle:{{ .Major }}"
image_templates:
- "ghcr.io/dvflw/mantle:{{ .Version }}-amd64"
- "ghcr.io/dvflw/mantle:{{ .Version }}-arm64"
- name_template: "ghcr.io/dvflw/mantle:latest"
image_templates:
- "ghcr.io/dvflw/mantle:{{ .Version }}-amd64"
- "ghcr.io/dvflw/mantle:{{ .Version }}-arm64"

release:
github:
owner: dvflw
name: mantle
draft: false
prerelease: auto
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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In GoReleaser v2, when release.prerelease: autois set, aredocker_manifestswithname_templatevalues likelatest, {{ .Major }}, and {{ .Major }}.{{ .Minor }}still published for prerelease tags such asv1.2.0-rc.1? What is the recommended way to skip those aliases for prereleases?

💡 Result:

In GoReleaser v2, with release.prerelease: auto set (which detects prereleases like v1.2.0-rc.1 based on the tag), docker_manifests with name_template values such as latest, {{ .Major }}, and {{ .Major }}.{{ .Minor }} are still published for prerelease tags. The name_template is always evaluated and used to create the manifest if not skipped, and static values like latest or templates that don't include prerelease info (e.g. {{ .Major }}) resolve to the same for both release and prerelease tags (e.g. 1 or 1.2 for v1.2.0-rc.1). The recommended way to skip those aliases for prereleases is to use separate docker_manifests entries for each alias with skip_push: auto (which skips on prerelease detection, same logic as release.prerelease: auto) or templated skip_push: "{{ .IsPrerelease }}" (available since earlier v2 versions). For example: docker_manifests: - name_template: "your/image:{{ .Version }}" image_templates: [...] # version-specific always publishes - name_template: "your/image:latest" image_templates: [...] skip_push: auto # or "{{ .IsPrerelease }}" - name_template: "your/image:{{ .Major }}" image_templates: [...] skip_push: auto - name_template: "your/image:{{ .Major }}.{{ .Minor }}" image_templates: [...] skip_push: auto Note: docker_manifests is deprecated (phasing out in v2.12+); migrate to dockers_v2 where tags support similar conditional templating, e.g. tags: ["{{ .Version }}", "{{ if not .IsPrerelease }}latest{{ end }}"].

Citations:


Add skip_push: auto to alias docker manifests to prevent RC tags from overwriting stable image aliases.

With prerelease: auto, the latest, {{ .Major }}, and {{ .Major }}.{{ .Minor }} manifests (lines 67–78) are still published for prerelease tags like v1.2.0-rc.1. Add skip_push: auto to each alias manifest entry:

Suggested fix
docker_manifests:
  - name_template: "ghcr.io/dvflw/mantle:{{ .Version }}"
    image_templates:
      - "ghcr.io/dvflw/mantle:{{ .Version }}-amd64"
      - "ghcr.io/dvflw/mantle:{{ .Version }}-arm64"
  - name_template: "ghcr.io/dvflw/mantle:{{ .Major }}.{{ .Minor }}"
    image_templates:
      - "ghcr.io/dvflw/mantle:{{ .Version }}-amd64"
      - "ghcr.io/dvflw/mantle:{{ .Version }}-arm64"
    skip_push: auto
  - name_template: "ghcr.io/dvflw/mantle:{{ .Major }}"
    image_templates:
      - "ghcr.io/dvflw/mantle:{{ .Version }}-amd64"
      - "ghcr.io/dvflw/mantle:{{ .Version }}-arm64"
    skip_push: auto
  - name_template: "ghcr.io/dvflw/mantle:latest"
    image_templates:
      - "ghcr.io/dvflw/mantle:{{ .Version }}-amd64"
      - "ghcr.io/dvflw/mantle:{{ .Version }}-arm64"
    skip_push: auto

Note: docker_manifests is deprecated in GoReleaser v2.12+; migrate to dockers_v2 with conditional tag templating (e.g., tags: ["{{ .Version }}", "{{ if not .IsPrerelease }}latest{{ end }}"]).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/.goreleaser.yaml` around lines 62 - 85, The alias docker
manifest entries under docker_manifests (the name_template entries
"ghcr.io/dvflw/mantle:{{ .Major }}.{{ .Minor }}", "ghcr.io/dvflw/mantle:{{
.Major }}" and "ghcr.io/dvflw/mantle:latest") are being pushed for prerelease
tags; add skip_push: auto to each of those manifest blocks so they are skipped
for prereleases when prerelease: auto is set. Edit the three manifest objects in
the docker_manifests section to include skip_push: auto directly under their
image_templates blocks (leave the primary {{ .Version }} manifest unchanged).

Comment on lines +33 to +34
completed_at:
type: string
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.

⚠️ Potential issue | 🟠 Major

Specify format: date-time for timestamp fields.

All timestamp fields (completed_at, started_at, created_at, updated_at, period_start) are defined as type: string without a format specifier. This prevents SDK generators from creating properly typed date/time objects in client languages and doesn't convey the expected ISO 8601 format to API consumers.

⏰ Recommended fix
 completed_at:
+  format: date-time
   type: string

Apply to all timestamp fields: completed_at, started_at, created_at, updated_at, period_start across all definitions.

Also applies to: 37-38, 59-60, 63-64, 110-111, 116-117, 125-126, 163-164, 170-171, 176-177

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/docs/swagger.yaml` around lines 33 - 34, The
timestamp properties (completed_at, started_at, created_at, updated_at,
period_start) are currently declared as type: string without a format; update
each of these field definitions in swagger.yaml to include format: date-time so
SDK generators and consumers know these are ISO 8601 timestamps (apply to all
occurrences of completed_at, started_at, created_at, updated_at, period_start
referenced in the diff and elsewhere in the file).

Comment on lines +136 to +137
definition:
type: object
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.

⚠️ Potential issue | 🟠 Major

Define the structure of WorkflowDetailResponse.definition.

The definition field is typed as a bare object with no properties, which provides no contract for SDK consumers. The description mentions it's a "workflow definition" but the schema is opaque. If the structure varies by workflow, document it as a map or free-form object explicitly. If it has a known schema, define it.

🔍 Recommended approach

If the workflow definition is a free-form JSON structure:

 definition:
+  additionalProperties: true
+  description: Workflow definition as free-form JSON
   type: object

If it has a known schema (e.g., steps, triggers), define it properly:

 definition:
-  type: object
+  $ref: '#/definitions/workflow.WorkflowDefinition'

And add the definition:

workflow.WorkflowDefinition:
  type: object
  properties:
    steps:
      type: array
      items:
        type: object
    # ... other known fields
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
definition:
type: object
definition:
type: object
additionalProperties: true
description: Workflow definition as free-form JSON
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/docs/swagger.yaml` around lines 136 - 137,
The OpenAPI schema currently defines WorkflowDetailResponse.definition as a bare
object; update it to explicitly document the structure by replacing the
anonymous object with either a free-form map (e.g., type: object,
additionalProperties: true) if the workflow definition is arbitrary, or by
creating a named schema (e.g., workflow.WorkflowDefinition) that lists known
properties (steps, triggers, etc.) and then reference it from
WorkflowDetailResponse.definition; ensure the YAML uses $ref:
'#/components/schemas/workflow.WorkflowDefinition' when using the named schema
and add clear property definitions for any known fields or additionalProperties:
true for free-form content.

Comment on lines +237 to +270
put:
description: Creates or replaces the monthly token budget for a provider. Enforcement
"hard" blocks execution when the limit is reached; "warn" logs a warning only.
parameters:
- description: Provider name (e.g. openai, bedrock)
in: path
name: provider
required: true
type: string
- description: Budget configuration
in: body
name: body
required: true
schema:
$ref: '#/definitions/server.SetBudgetRequest'
responses:
"200":
description: OK
schema:
$ref: '#/definitions/server.StatusResponse'
"400":
description: Bad Request
schema:
$ref: '#/definitions/server.ErrorResponse'
"500":
description: Internal Server Error
schema:
$ref: '#/definitions/server.ErrorResponse'
security:
- ApiKeyAuth: []
- OIDCAuth: []
summary: Set provider budget
tags:
- budgets
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.

⚠️ Potential issue | 🟠 Major

Add consumes and produces fields to PUT and POST operations.

Swagger 2.0 requires consumes and produces fields on operations that accept or return bodies. The PUT and POST operations are missing these fields, which can cause issues with code generators and violates the Swagger 2.0 specification.

📦 Recommended fix

For PUT /api/v1/budgets/{provider}:

   put:
     description: Creates or replaces the monthly token budget...
+    consumes:
+      - application/json
+    produces:
+      - application/json
     parameters:

Apply the same pattern to:

  • POST /api/v1/cancel/{execution}
  • POST /api/v1/run/{workflow}

And consider adding produces: [application/json] to all other endpoints for consistency.

Also applies to: 295-323, 402-434

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/docs/swagger.yaml` around lines 237 - 270,
The PUT operation for "Set provider budget" (summary: Set provider budget,
request schema: '#/definitions/server.SetBudgetRequest') is missing Swagger
2.0's required operation-level consumes and produces; update this PUT to include
consumes: ["application/json"] and produces: ["application/json"] and apply the
same change to the POST operations for cancel (POST /api/v1/cancel/{execution},
summary or path reference) and run (POST /api/v1/run/{workflow}), and consider
adding produces: ["application/json"] to other endpoints for consistency.

Comment on lines +324 to +369
/api/v1/executions:
get:
description: Returns a paginated list of workflow executions for the authenticated
team. Supports filtering by workflow name, status, and age.
parameters:
- description: Filter by workflow name
in: query
name: workflow
type: string
- description: Filter by status
enum:
- pending
- running
- completed
- failed
- cancelled
in: query
name: status
type: string
- description: Filter by age (e.g. 1h, 7d)
in: query
name: since
type: string
- description: Max results (default 20)
in: query
name: limit
type: integer
responses:
"200":
description: OK
schema:
$ref: '#/definitions/server.ExecutionListResponse'
"400":
description: Bad Request
schema:
$ref: '#/definitions/server.ErrorResponse'
"500":
description: Internal Server Error
schema:
$ref: '#/definitions/server.ErrorResponse'
security:
- ApiKeyAuth: []
- OIDCAuth: []
summary: List executions
tags:
- executions
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.

⚠️ Potential issue | 🟠 Major

Add pagination metadata to ExecutionListResponse.

The endpoint description mentions "paginated list" and accepts a limit parameter (line 347-350), but the response schema only contains an executions array with no pagination metadata (e.g., next_cursor, total_count, has_more). This makes it impossible for clients to retrieve subsequent pages.

📄 Suggested pagination fields
 server.ExecutionListResponse:
   properties:
     executions:
       items:
         $ref: '#/definitions/server.ExecutionSummary'
       type: array
+    total:
+      description: Total number of executions matching the filter
+      type: integer
+    next_cursor:
+      description: Cursor for the next page; omit if no more results
+      type: string
+    has_more:
+      description: Whether additional pages are available
+      type: boolean
   type: object

Update the endpoint to document these fields and consider adding an offset or cursor query parameter.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/docs/swagger.yaml` around lines 324 - 369,
The response schema for the GET /api/v1/executions endpoint is missing
pagination metadata: update the server.ExecutionListResponse definition to
include pagination fields (e.g., next_cursor (string|null), has_more (boolean),
and total_count (integer) or page_size) so clients can page results, and add an
optional query parameter (e.g., cursor or offset) to the endpoint parameters
alongside limit; ensure the new fields are referenced in the responses section
for /api/v1/executions and that the names match the ExecutionListResponse schema
(use ExecutionListResponse, next_cursor, has_more, total_count, and the new
query param name such as cursor).

Comment on lines +25 to +27
// Swagger 2.0 uses "swagger", OpenAPI 3.x uses "openapi"
if doc["swagger"] == nil && doc["openapi"] == nil {
t.Error("response does not look like an OpenAPI/Swagger document (missing 'swagger' or 'openapi' key)")
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.

⚠️ Potential issue | 🟠 Major

Test is too permissive for the stated OpenAPI 3.1 objective.

Allowing either swagger or openapi means a Swagger 2.0 artifact still passes. That weakens enforcement of the linked requirement to provide an OpenAPI 3.1 contract.

Proposed tightening
 import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 )
@@
-	// Swagger 2.0 uses "swagger", OpenAPI 3.x uses "openapi"
-	if doc["swagger"] == nil && doc["openapi"] == nil {
-		t.Error("response does not look like an OpenAPI/Swagger document (missing 'swagger' or 'openapi' key)")
-	}
+	openapiVersion, ok := doc["openapi"].(string)
+	if !ok || !strings.HasPrefix(openapiVersion, "3.1.") {
+		t.Fatalf("openapi = %v, want OpenAPI 3.1.x", doc["openapi"])
+	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/openapi_test.go` around lines 25 - 27, The
test currently accepts either Swagger 2.0 or OpenAPI by checking doc["swagger"]
or doc["openapi"]; tighten it to enforce OpenAPI 3.1 by requiring doc["openapi"]
to be present and to be a string beginning with "3.1" (convert doc["openapi"] to
a string and check HasPrefix "3.1"). Update the failure message to state that an
OpenAPI 3.1 document is required. Refer to the local doc variable and the test
in openapi_test.go when making this change.

- Remove hardcoded @host localhost:8080 from global API annotations so
  the spec is served host-agnostic
- Add omitempty to StartedAt on ExecutionSummary, ExecutionDetail, and
  StepSummary to match CompletedAt behaviour and align wire format with
  schema
- Replace http.Error (text/plain) with writeJSONError in budget/usage
  handlers so 400/500 responses honour the documented ErrorResponse
  contract; log internal errors via s.Logger instead of leaking details
- Pin swag install in CI spec job to v1.16.6 for reproducible builds
- Sync govulncheck step with main's known-unfixable exclusion filter
- Regenerate swagger.json/yaml

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (5)
packages/engine/internal/server/api.go (5)

421-427: Response structure doesn't match documented type.

Returns map[string]any but annotation declares WorkflowDetailResponse. Using the typed struct would provide compile-time validation.

♻️ Suggested fix
-	var def json.RawMessage = content
-	writeJSON(w, http.StatusOK, map[string]any{
-		"name":    name,
-		"version": version,
-		"definition": def,
-	})
+	writeJSON(w, http.StatusOK, WorkflowDetailResponse{
+		Name:       name,
+		Version:    version,
+		Definition: content,
+	})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/api.go` around lines 421 - 427, The handler
is returning an untyped map but should return the declared
WorkflowDetailResponse type; replace the ad-hoc map in the writeJSON call with a
strongly-typed instance of WorkflowDetailResponse (populate its Name, Version
and Definition fields using the existing name, version and def/json.RawMessage
variables) and pass that struct to writeJSON(w, http.StatusOK, resp) so the
response shape is validated at compile time.

493-499: Response structure doesn't match documented type.

Same pattern - returns map[string]any but annotation declares WorkflowDetailResponse.

♻️ Suggested fix
-	var def json.RawMessage = content
-	writeJSON(w, http.StatusOK, map[string]any{
-		"name":       name,
-		"version":    version,
-		"definition": def,
-	})
+	writeJSON(w, http.StatusOK, WorkflowDetailResponse{
+		Name:       name,
+		Version:    version,
+		Definition: content,
+	})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/api.go` around lines 493 - 499, The handler
is returning a generic map[string]any instead of the declared
WorkflowDetailResponse type; replace the map with an instance of
WorkflowDetailResponse (populate its fields such as Name, Version and Definition
using the existing name, version and def variables) and pass that struct to
writeJSON so the response matches the documented type and JSON annotations in
WorkflowDetailResponse.

169-169: Response structure doesn't match documented type.

The handler returns map[string]any{"executions": executions} but the Swagger annotation on line 57 declares ExecutionListResponse as the response type. While structurally equivalent, using the defined type directly would ensure consistency and enable compile-time verification.

♻️ Suggested fix
-	writeJSON(w, http.StatusOK, map[string]any{"executions": executions})
+	writeJSON(w, http.StatusOK, ExecutionListResponse{Executions: executions})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/api.go` at line 169, The handler is returning
a raw map instead of the documented ExecutionListResponse type; replace the map
literal passed to writeJSON with a properly typed value like
ExecutionListResponse{Executions: executions} (or convert executions into the
ExecutionListResponse struct) so the call writeJSON(w, http.StatusOK, ...) uses
an instance of ExecutionListResponse, ensuring compile-time verification and
alignment with the Swagger docs.

385-388: Response structure doesn't match documented type.

Same issue as handleListExecutions: returns map[string]any but annotation declares WorkflowListResponse.

♻️ Suggested fix
-	if workflows == nil {
-		workflows = []workflow.WorkflowSummary{}
-	}
-	writeJSON(w, http.StatusOK, map[string]any{"workflows": workflows})
+	if workflows == nil {
+		workflows = []workflow.WorkflowSummary{}
+	}
+	writeJSON(w, http.StatusOK, WorkflowListResponse{Workflows: workflows})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/api.go` around lines 385 - 388, The response
currently writes a raw map via writeJSON(w, ..., map[string]any{"workflows":
workflows}) which does not match the declared WorkflowListResponse; change the
payload to an instance of WorkflowListResponse (or the named struct used by
handleListExecutions) populated with the workflows slice (ensuring workflows is
non-nil) and pass that struct to writeJSON instead of the ad-hoc map; update any
imports/types if needed so writeJSON serializes WorkflowListResponse.

454-458: Response structure doesn't match documented type.

Returns map[string]any but annotation declares WorkflowVersionListResponse.

♻️ Suggested fix
-	if versions == nil {
-		versions = []workflow.VersionSummary{}
-	}
-	writeJSON(w, http.StatusOK, map[string]any{"name": name, "versions": versions})
+	if versions == nil {
+		versions = []workflow.VersionSummary{}
+	}
+	writeJSON(w, http.StatusOK, WorkflowVersionListResponse{Name: name, Versions: versions})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/engine/internal/server/api.go` around lines 454 - 458, The handler
currently writes a plain map (using writeJSON) for the response but the API
annotation expects a WorkflowVersionListResponse; replace the ad-hoc map with an
instance of WorkflowVersionListResponse (populate its Name and Versions fields,
using the existing name and versions variables and ensuring versions is of type
[]workflow.VersionSummary) and pass that struct to writeJSON instead of
map[string]any so the runtime response matches the documented type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/engine/internal/server/api.go`:
- Around line 421-427: The handler is returning an untyped map but should return
the declared WorkflowDetailResponse type; replace the ad-hoc map in the
writeJSON call with a strongly-typed instance of WorkflowDetailResponse
(populate its Name, Version and Definition fields using the existing name,
version and def/json.RawMessage variables) and pass that struct to writeJSON(w,
http.StatusOK, resp) so the response shape is validated at compile time.
- Around line 493-499: The handler is returning a generic map[string]any instead
of the declared WorkflowDetailResponse type; replace the map with an instance of
WorkflowDetailResponse (populate its fields such as Name, Version and Definition
using the existing name, version and def variables) and pass that struct to
writeJSON so the response matches the documented type and JSON annotations in
WorkflowDetailResponse.
- Line 169: The handler is returning a raw map instead of the documented
ExecutionListResponse type; replace the map literal passed to writeJSON with a
properly typed value like ExecutionListResponse{Executions: executions} (or
convert executions into the ExecutionListResponse struct) so the call
writeJSON(w, http.StatusOK, ...) uses an instance of ExecutionListResponse,
ensuring compile-time verification and alignment with the Swagger docs.
- Around line 385-388: The response currently writes a raw map via writeJSON(w,
..., map[string]any{"workflows": workflows}) which does not match the declared
WorkflowListResponse; change the payload to an instance of WorkflowListResponse
(or the named struct used by handleListExecutions) populated with the workflows
slice (ensuring workflows is non-nil) and pass that struct to writeJSON instead
of the ad-hoc map; update any imports/types if needed so writeJSON serializes
WorkflowListResponse.
- Around line 454-458: The handler currently writes a plain map (using
writeJSON) for the response but the API annotation expects a
WorkflowVersionListResponse; replace the ad-hoc map with an instance of
WorkflowVersionListResponse (populate its Name and Versions fields, using the
existing name and versions variables and ensuring versions is of type
[]workflow.VersionSummary) and pass that struct to writeJSON instead of
map[string]any so the runtime response matches the documented type.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 79274d1f-e372-46f9-9e18-88aac6d2583f

📥 Commits

Reviewing files that changed from the base of the PR and between 593309f and 1cdc247.

📒 Files selected for processing (5)
  • .github/workflows/engine-ci.yml
  • packages/engine/internal/server/api.go
  • packages/engine/internal/server/docs.go
  • packages/engine/internal/server/docs/swagger.json
  • packages/engine/internal/server/docs/swagger.yaml
✅ Files skipped from review due to trivial changes (1)
  • packages/engine/internal/server/docs.go

- Replace all ad-hoc map[string]any response payloads with their
  declared named types (ExecutionListResponse, WorkflowListResponse,
  WorkflowDetailResponse, WorkflowVersionListResponse) so runtime
  responses match the documented Swagger types and are validated at
  compile time
- Add || true to the ACTIONABLE grep pipeline so grep's non-zero exit
  when all vulns are excluded doesn't abort the step under set -e
- Regenerate swagger.json/yaml

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
.github/workflows/engine-ci.yml (1)

95-104: Consider aligning swag version between CI and Makefile documentation.

CI pins swag@v1.16.6, but the Makefile's spec target comment suggests @latest. If developers install a different swag version locally, the generated spec may differ, causing the CI freshness check to fail unexpectedly.

💡 Suggested alignment

Update the Makefile comment to specify the pinned version:

-spec: ## Regenerate OpenAPI spec (requires: go install github.com/swaggo/swag/cmd/swag@latest)
+spec: ## Regenerate OpenAPI spec (requires: go install github.com/swaggo/swag/cmd/swag@v1.16.6)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/engine-ci.yml around lines 95 - 104, Update the Makefile's
spec target comment to match the pinned swag version used in CI: change any
reference to "@latest" (or unspecified) in the Makefile comment for the "spec"
target to the exact version "v1.16.6" used by the "Install swag" step in the CI
workflow, so local developer instructions and the CI install (Install swag in
the engine CI workflow) are consistent and avoid mismatched generated specs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.github/workflows/engine-ci.yml:
- Around line 95-104: Update the Makefile's spec target comment to match the
pinned swag version used in CI: change any reference to "@latest" (or
unspecified) in the Makefile comment for the "spec" target to the exact version
"v1.16.6" used by the "Install swag" step in the CI workflow, so local developer
instructions and the CI install (Install swag in the engine CI workflow) are
consistent and avoid mismatched generated specs.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f264cac0-2ac3-4983-9cb5-56e6814251f5

📥 Commits

Reviewing files that changed from the base of the PR and between 1cdc247 and df573f7.

📒 Files selected for processing (2)
  • .github/workflows/engine-ci.yml
  • packages/engine/internal/server/api.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/engine/internal/server/api.go

Matches the version pinned in the CI spec freshness job to avoid
mismatched generated specs between local and CI runs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@michaelmcnees michaelmcnees merged commit f9e14d6 into main Apr 5, 2026
8 checks passed
michaelmcnees pushed a commit that referenced this pull request Apr 5, 2026
- swagger.yaml/json: add required fields to all schema definitions
- swagger.yaml/json: add format: date-time to all timestamp fields
- swagger.yaml/json: add additionalProperties: true + description to
  WorkflowDetailResponse.definition (free-form JSON object)
- swagger.yaml/json: add consumes/produces to PUT and POST operations
- swagger.yaml/json: fix misleading "paginated list" description on
  GET /api/v1/executions (limit-based truncation, not cursor pagination)
- openapi_test.go: tighten test to assert swagger version string
  rather than accepting either swagger or openapi key
- .gitignore: anchor plans/ and marketing/variants/ to repo root
  to prevent matching directories at arbitrary depth
- .goreleaser.yaml: use template skip_push to skip alias manifests
  (major.minor, major, latest) for prerelease and snapshot tags;
  previously skip_push: auto only skipped snapshots, not prereleases

https://claude.ai/code/session_01U1VTqCn9SivQkW8RydnYeA
michaelmcnees pushed a commit that referenced this pull request Apr 7, 2026
- swagger.yaml/json: add required fields to all schema definitions
- swagger.yaml/json: add format: date-time to all timestamp fields
- swagger.yaml/json: add additionalProperties: true + description to
  WorkflowDetailResponse.definition (free-form JSON object)
- swagger.yaml/json: add consumes/produces to PUT and POST operations
- swagger.yaml/json: fix misleading "paginated list" description on
  GET /api/v1/executions (limit-based truncation, not cursor pagination)
- openapi_test.go: tighten test to assert swagger version string
  rather than accepting either swagger or openapi key
- .gitignore: anchor plans/ and marketing/variants/ to repo root
  to prevent matching directories at arbitrary depth
- .goreleaser.yaml: use template skip_push to skip alias manifests
  (major.minor, major, latest) for prerelease and snapshot tags;
  previously skip_push: auto only skipped snapshots, not prereleases

https://claude.ai/code/session_01U1VTqCn9SivQkW8RydnYeA
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.

[Feature] OpenAPI specification for REST API

1 participant