Add wire-contract CI gate and agent review workflow skills - #1326
Merged
Conversation
Reviews are probabilistic; live integrator breakage should be caught deterministically. The gate renders the shared endpoint types and the public SDK API into a structurally expanded snapshot, so any change to what partners consume - including transitive type/enum drift - fails CI until the snapshot is regenerated and the diff is reviewed on purpose.
vortex-review encodes the multi-lens finder fleet with loop-until-dry and adversarial verification; address-feedback turns one PR review round into a single verify-fix-gate-push command.
ship chains plan approval, implementation with tests, vortex-review until dry, and PR creation; babysit-pr watches an open PR and reacts to new reviews and CI failures so feedback rounds need no ferrying.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a CI-enforced partner API snapshot generator and reusable agent workflows for reviewing and shipping changes.
Changes:
- Generates and verifies shared/SDK wire-contract snapshots.
- Adds serializer fixtures and CI coverage.
- Adds four PR review and delivery skills.
Reviewed changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
.gitignore |
Tracks shared Claude skills. |
.github/workflows/ci.yml |
Runs wire-contract tests and checks. |
.claude/skills/address-feedback/SKILL.md |
Defines feedback handling. |
.claude/skills/babysit-pr/SKILL.md |
Defines PR monitoring. |
.claude/skills/ship/SKILL.md |
Defines end-to-end delivery. |
.claude/skills/vortex-review/SKILL.md |
Defines deep review workflow. |
docs/api/README.md |
Documents the snapshot. |
docs/api/wire-contract.snapshot.md |
Captures the generated public contract. |
package.json |
Adds snapshot commands. |
scripts/wire-contract/generate-report.ts |
Implements contract serialization. |
scripts/wire-contract/generate-report.test.ts |
Tests serializer output. |
scripts/wire-contract/fixtures/fixture-surface.ts |
Provides serializer fixtures. |
scripts/wire-contract/fixtures/tsconfig.json |
Configures fixture compilation. |
Suppressed comments (1)
.claude/skills/address-feedback/SKILL.md:17
gh pr view --json reviews,commitsreturns review summaries but not their inline review comments, so the bare-PR flow silently drops inline findings. Fetch the paginated review-comments endpoint too and retain comments whosepull_request_review_idbelongs to reviews submitted since the last push.
- Bare PR number → fetch all reviews and review comments submitted since the last push
(`gh pr view <n> --json reviews,commits`).
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
✅ Deploy Preview for vortexfi ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for vrtx-dashboard canceled.
|
✅ Deploy Preview for vortex-sandbox ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Copilot review on #1326 found three blind spots: generic conditional aliases rendered as self-references, signature type parameters and their constraints were dropped, and readonly was erased. All three let partner-facing drift pass the gate; the serializer now expands conditional bodies, emits type-parameter constraints and defaults, and preserves readonly, with fixture coverage for each.
Inline review comments are not returned by gh pr view and live on a paginated endpoint; the address-feedback and babysit-pr instructions now fetch them explicitly so findings cannot be silently dropped.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (6)
scripts/wire-contract/generate-report.ts:136
- Readonly index signatures are serialized identically to writable ones because
info.isReadonlyis ignored. The current endpoint surface already contains direct index signatures, so changing one toreadonly [key: string]would leave the snapshot unchanged and bypass the gate. Preserve the modifier and add a readonly-index fixture assertion.
lines.push(`[key: ${keyType}]: ${valueType};`);
scripts/wire-contract/generate-report.ts:235
- Tuple mutability is lost here:
[string, number]andreadonly [string, number]have the same type arguments and therefore produce the same snapshot, despite being different public contracts. Include the tuple target's readonly metadata in the rendering and cover this in the tuple fixture.
if (checker.isTupleType(type)) {
const elements = checker.getTypeArguments(type as ts.TypeReference).map(element => serializeType(ctx, element, depth + 1));
return `[${elements.join(", ")}]`;
scripts/wire-contract/generate-report.ts:159
- Dropping every
undefinedconstituent also erases explicitly declaredundefined. WithexactOptionalPropertyTypesdisabled in both target configs,field?: stringandfield?: string | undefinedserialize identically, although consumers enabling that option observe different contracts. Distinguish compiler-added optionality from an explicit union using the declaration node, and add a regression fixture.
const optional = (property.flags & ts.SymbolFlags.Optional) !== 0;
const isReadonly = modifiers.some(modifier => modifier.kind === ts.SyntaxKind.ReadonlyKeyword);
const propertyType = serializeType(ctx, typeOfSymbol(ctx, property), depth + 1, { dropUndefined: optional });
lines.push(`${isReadonly ? "readonly " : ""}${property.name}${optional ? "?" : ""}: ${propertyType};`);
.claude/skills/vortex-review/SKILL.md:19
- This executes code from the checked-out PR branch (
package.jsonand the generator are both PR-controlled) under the reviewer's local credentials. An untrusted PR can therefore run arbitrary code even though this skill is described as review-only. Require an explicit trusted-head check, or run branch-controlled commands only in a credential-free sandbox; otherwise inspect untrusted PRs without execution.
- Run `bun run wire-contract:check` (after `bun run build:shared` if shared changed).
.claude/skills/address-feedback/SKILL.md:49
- After checking out an arbitrary PR, these gates execute branch-controlled scripts and tests with the agent's local credentials. A malicious fork can exfiltrate GitHub or provider credentials before any feedback is addressed. Add a trusted-head/author gate or require a credential-free sandbox before running commands from the PR; untrusted feedback can still be inspected without execution.
## 4. Gates before pushing
Run what the change touches:
- `bun lint:fix` (Biome — except `packages/sdk`, which uses `bun lint` / ESLint inside
.claude/skills/babysit-pr/SKILL.md:31
- The CI-failure path runs repository gates on whichever PR branch was checked out, without establishing that the branch is trusted. For an external PR this lets branch-controlled scripts execute repeatedly under the babysitter's credentials. Apply the same trusted-head or credential-free sandbox requirement before any local gate execution.
1. Fetch current PR state and diff against the baseline.
2. New CI failure on the current head → read the failing logs
(`gh run view <run-id> --log-failed`), fix, run the repo gates, push.
3. New review or new inline review comments → run the `/address-feedback` flow on them:
Second Copilot round on #1326: readonly index signatures and readonly tuples serialized identically to their mutable forms, so mutability changes to those constructs would pass the gate unseen.
Review, feedback, and babysit skills execute branch-local commands; on a public repo with forks that would let an untrusted PR run code under local credentials. Fork or external-author PRs are now inspect-only in all three skills.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
An analysis of our recent development sessions showed two systematic weaknesses:
This PR adds a deterministic wire-contract gate plus four repo-level agent skills that encode the review/feedback workflow we kept reinventing per session.
Wire-contract gate (deterministic)
scripts/wire-contract/generate-report.tsrenders the typed partner-facing surface — the shared endpoint request/response types (packages/shared/src/endpoints) and the public SDK API (packages/sdk/src/index.ts) — intodocs/api/wire-contract.snapshot.md(checked in, ~5.8k lines).Record<FiatToken, …>resolved), so a change to a transitively referenced type surfaces in the snapshot even when no endpoint file was edited. External types (viem, polkadot, lib) are kept by name.bun run wire-contract:checkfails when the snapshot is stale;bun run wire-contract:updateregenerates it. CI runs the check in the build job, so every partner-facing change becomes an explicit, reviewable diff in the PR that made it.Note: the SDK entry resolves
@vortexfi/sharedthrough its built declarations — runbun run build:sharedbefore regenerating when shared changed (CI's build job already does).Agent skills (
.claude/skills/)/vortex-review— multi-lens deep review: parallel finder agents with Vortex-specific lenses (financial integrity, phase-recovery/idempotency, presigned-tx identity, silent failures, partner surface, migrations vs deployed schema, corridor-matrix consistency, test adequacy), looped until two consecutive rounds find nothing new, every finding adversarially verified before it is reported. Review-only; never edits code./address-feedback <review-url>— one feedback round in one command: fetch the review viagh, verify each finding against the code, fix agreed items with regression tests, run the repo gates, push, and report a full disposition table. Business decisions are surfaced, never guessed./babysit-pr <PR#>— watches an open PR: reacts to new reviews and CI failures, pushes mechanical fixes, re-requests Copilot review after addressing a round, and notifies on business decisions or mergeability. Never merges./ship— the pipeline: plan (with explicit decision questions, human gate 1) → implement with tests →/vortex-reviewuntil dry → PR + Copilot review +/babysit-pr. Merge remains human gate 2..gitignoregets a!.claude/skills/exception so the skills are version-controlled and available in worktree sessions once this lands on staging.Verification
cd scripts/wire-contract && bun test— 3 pass.bun run wire-contract:check— snapshot up to date.bun run verify— clean.