Skip to content

feat(ci): publish spec-change manifests as workflow artifacts - #265

Merged
gjtorikian merged 2 commits into
mainfrom
feat/spec-changes-artifact
Nov 5, 2026
Merged

gjtorikian merged 2 commits into
mainfrom
feat/spec-changes-artifact

Conversation

@gjtorikian

Copy link
Copy Markdown
Collaborator

Why

spec-changes.yml and replay-spec-changes.yml POSTed each changed-services manifest to the SDK bot at /internal/spec-changes, HMAC-signed with SPEC_CHANGES_SECRET. Runner traffic cannot be IP-scoped, so that endpoint needed its own WAF exception, and every run since Oct 30 has failed on it.

Pairs with workos/sdk-automation-bot feat/spec-changes-artifact-ingest, which pulls the artifact on this workflow's workflow_run webhook.

What

  • spec-changes.yml: build the manifest with --output spec-changes/<sha>.json and upload it as artifact spec-changes-<sha> (14-day retention). No upload when there are no changed services. Re-running the workflow re-publishes, which is the recovery path for a missed delivery.
  • replay-spec-changes.yml: mode: upload replaces push. The script writes one <sha>.json per target into spec-changes/, uploaded as spec-changes-replay-<run_id>.
  • scripts/replay-spec-changes.mjs: --push/--bot-url become --output-dir, still gated on --confirm REPLAY_SPEC_CHANGES. The HMAC POST path is gone.

actionlint clean on both workflows. npm test: 163 pass.

After merge

Delete the SPEC_CHANGES_SECRET Actions secret; nothing reads it anymore.

spec-changes.yml and replay-spec-changes.yml used to POST each
changed-services manifest to the SDK automation bot at
/internal/spec-changes, HMAC-signed with SPEC_CHANGES_SECRET. That was the
bot's only inbound surface besides the GitHub webhook, and traffic from
Actions runners cannot be IP-scoped, so it needed its own WAF exception.

Both producers now upload the manifest as a workflow artifact
(spec-changes-<sha>, spec-changes-replay-<run_id>). The bot pulls it on the
run's workflow_run webhook with its App token and upserts by sha, so nothing
leaves GitHub and re-running a producer is the recovery path for a missed
delivery. The replay script's --push/--bot-url become --output-dir, still
gated on --confirm REPLAY_SPEC_CHANGES.
@greptile-apps

greptile-apps Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; the remaining test gap is non-blocking.

Findings

  1. P2 File writes lack test coverage ▶
Fix with agent prompt
### Issue 1
scripts/__tests__/replay-spec-changes.spec.mjs:undefined-105
The updated tests check `--output-dir` parsing but never exercise the new file-writing branch. Add tests using a temporary directory to check `<sha>.json` contents, skipping empty manifests, dry runs writing nothing, and a bad later target leaving no earlier files. These are the guarantees the artifact upload now depends on.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR replaces signed manifest POSTs with workflow artifacts for the SDK bot to download.

  • Regular spec-change runs publish manifests as GitHub artifacts.
  • Historical replays reach the SDK bot as workflow artifacts.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Spec merge] --> B[Build manifest]
  C[Confirmed historical replay] --> D[Rebuild manifests]
  B --> E{Changed services?}
  D --> E
  E -->|Yes| F[Upload workflow artifact]
  E -->|No| G[Skip upload]
  F --> H[SDK bot receives workflow_run]
  H --> I[Download manifests and upsert by SHA]
Loading

Reviews (2) · Last reviewed commit: "fix(ci): cover replay manifest writes an..." · Reviewed by Greptile

parsed.endpoint.href,
"https://sdk-automation-bot.workos.tools/internal/spec-changes",
);
assert.equal(parsed.outputDir, resolve("spec-changes"));

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.

P2 File writes lack test coverage

The updated tests check --output-dir parsing but never exercise the new file-writing branch. Add tests using a temporary directory to check <sha>.json contents, skipping empty manifests, dry runs writing nothing, and a bad later target leaving no earlier files. These are the guarantees the artifact upload now depends on.

Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/__tests__/replay-spec-changes.spec.mjs
Line: 102

Comment:
**File writes lack test coverage**

The updated tests check `--output-dir` parsing but never exercise the new file-writing branch. Add tests using a temporary directory to check `<sha>.json` contents, skipping empty manifests, dry runs writing nothing, and a bad later target leaving no earlier files. These are the guarantees the artifact upload now depends on.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Review follow-ups on #265. The replay guide still described push mode,
signed POSTs, and SPEC_CHANGES_SECRET; it now documents upload mode and
artifact delivery. The file-writing step moves into an exported
writeManifests that validates every target before the first write, with
tests for the written contents, skipped empty manifests, and a bad later
target leaving no files behind.
@gjtorikian
gjtorikian merged commit e02371a into main Nov 5, 2026
9 checks passed
@gjtorikian
gjtorikian deleted the feat/spec-changes-artifact branch November 5, 2026 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant