Skip to content

chore: refresh generated beta spec - #207

Open
github-actions[bot] wants to merge 1 commit into
mainfrom
chore/refresh-beta-spec-20261109
Open

github-actions[bot] wants to merge 1 commit into
mainfrom
chore/refresh-beta-spec-20261109

Conversation

@github-actions

@github-actions github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Automated regeneration of beta/openapi.beta.yaml (the embedded dci beta
command surface) detected drift against the source spec. Review the shape
changes below; merging updates what the next release embeds.

If an operation listed in beta/manifest.yaml disappeared from the source
spec, the generator fails instead of opening this PR — that case needs a
manifest edit (likely a GA promotion; see BETA-SPEC.md §2.8).

@github-actions
github-actions Bot requested review from a team, apgiorgi, chaim0m and taltultc as code owners November 9, 2026 06:29

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Findings marked 🟡 are optional suggestions and need no follow-up push.

Comment thread beta/openapi.beta.yaml
Comment on lines +922 to +923
- RB8DndcxODriK83IBXXf
- D7r4znsTj2UC95zGnunW

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 The embedded beta spec now ships two real-looking attribution IDs as OpenAPI examples, which dci beta help and published docs then show to users. Lines 922-923 swap the synthetic placeholders ExampleAttribution10/ExampleAttribution20 for 20-char Firestore-style IDs copied from the source spec. tools/betaspec copies examples unchanged, so nothing filters them out. Fix: use synthetic, clearly fake IDs for every example ID in the source spec, then regenerate. Or have tools/betaspec replace example IDs so later regenerations stay clean.

Why this was flagged

The PR regenerates beta/openapi.beta.yaml from https://api-dev.doit.com/openapi.yaml. The ExternalConfig example's attribution filter values (beta/openapi.beta.yaml:922-923) changed from the placeholders ExampleAttribution10/ExampleAttribution20 to two realistic 20-character auto-ID strings. The tenant-identifier-hygiene skill, required by AGENTS.md, says OpenAPI examples must use synthetic resource IDs and that a public value or a generated artifact is no exception. The base branch embedded only obvious placeholders. tools/betaspec/main.go does no example sanitization: a grep for example, sanitiz and scrub finds nothing. Once merged, these IDs are in the binary's embedded spec and in git history. If they are real tenant attribution IDs, removing them later does not erase that history.

Verification: The diff replaces the placeholders ExampleAttribution10 / ExampleAttribution20 with two realistic-looking 20-character auto-ID strings at beta/openapi.beta.yaml:922-923. beta_commands.go:36 //go:embed beta/openapi.beta.yaml embeds it in the released dci binary. tools/betaspec does not filter or rewrite example values.

Comment thread beta/openapi.beta.yaml
Comment on lines 330 to +333
rows:
type: array
nullable: true
description: Null when the fileOutput parameter was supplied and the file was rendered — the rows are in the file.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 dci beta get-report-results --file-output pdf|png|csv --output csv now fails with "response is not table-shaped" and never prints the signed fileOutput URL. The spec now allows result.rows: null, but the CLI's report row extractor still rejects a null rows key (main.go:3168-3171). Fix: have extractGetReportRows treat a null rows as no inline rows (unhandled, or an empty set), so CSV and table output fall back to the metadata row carrying fileOutput. The table output only works today because it falls back to indented JSON.

Why this was flagged

The regenerated spec adds the fileOutput query parameter (beta/openapi.beta.yaml:269-279), which restish mounts as a new flag on dci beta get-report-results. It also declares that result.rows is null whenever the file is rendered (lines 330-333; see the withPdfFileOutput example, rows: null). In extractGetReportRows (main.go:3163-3171) a present rows key that is not []interface{} returns handled=true with "error building table. result.rows must be an array". dciCSVContentType.Marshal (csv_output.go:25-27) turns that into the error "response is not table-shaped; use --output json instead", so a CSV invocation with --file-output pdf, png or csv exits with an error and no URL. On the base branch the flag and the null-rows shape do not exist in the embedded spec. reportResultContainer (response_transform.go:534) tolerates nil rows, but the CSV path does not go through it. dciTableContentType (main.go:2851-2858) hides the problem in table mode only by printing raw JSON. With json, the rows key is omitted, so that case is not affected.

Verification: Trigger: a user runs dci beta get-report-results <id> --file-output <fmt> --output csv and the server sends result.rows: null. The base spec had no such parameter. In extractGetReportRows (main.go:3163-3171), rawRows.([]interface{}) fails on nil and returns "error building table. result.rows must be an array". The CSV output never shows the fileOutput URL or the metadata.

Comment thread beta/openapi.beta.yaml
Comment on lines +269 to +270
- name: fileOutput
in: query

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 (optional) If prod has not yet deployed what the dev spec describes, dci beta get-report-results --file-output users get inline rows and no signed URL, with no error. Async-operation readers also look for createTime, which prod may not send. The header at line 1 shows this spec is generated from api-dev.doit.com, but the binary calls the production API. Fix: gate the refresh, or these new and renamed fields, on the source change being live in prod, for example by generating from the prod spec or diffing against it before embedding.

Why this was flagged

If api.doit.com still runs the pre-change handler, the server ignores the unknown fileOutput query parameter. It answers 200 with full rows and no fileOutput. The spec at line 270 says that response shape means a transient render failure, so the user retries and still gets no file. The same applies to the required createdAt→createTime rename at lines 542/557: prod would still return createdAt. On the base branch the embedded spec matched what prod served, and the CLI offered no flag that prod ignores. The finder dismissed this because the generator is designed to read dev (tools/betaspec header). That design is the mechanism: nothing in the refresh checks whether prod is live before the next release embeds the spec. Every early-access user of get-report-results and the async operation commands hits this during the lag window.

Verification: Line 1 shows the spec is generated from https://api-dev.doit.com/openapi.yaml, but beta commands call prod. This diff adds the fileOutput query parameter (lines 269-279). A server that predates the feature would most likely ignore it and answer 200 with inline rows and no fileOutput. The prod check at BETA-SPEC.md:127-129 covers whether an operation is included, not shape changes.

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.

0 participants