Repository navigation
chore(spec): resync the v2 spec, TypeScript SDK and plugin knowledge - #149
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
9997092 to
ebdebfd
Compare
ebdebfd to
76b1493
Compare
OneAsterisk
left a comment
There was a problem hiding this comment.
Claude review, round 1
Verdict: Approved. Clean spec resync that adds one read-only tool, classified and annotated per specgen/DIRECTORY_POLICY.md.
Approval gate: 0 Critical, 0 High, 0 Medium+Low → approved.
Existing feedback
- vercel[bot]: deploy notice only (build skipped by Ignored Build Step). No claims to verify.
PR description vs code
| Claim | Status | Where |
|---|---|---|
Spec resynced, new list-audit-logs generated |
✅ | specgen/spec/openapi.yaml (/v2/audit-logs), src/specgen/generated/tools.gen.ts:2537 |
| TypeScript SDK and plugin knowledge versions unchanged | ✅ | @runpod/typescript-api-sdk stays 0.3.0. Only its transitive structured-headers moves 2.1.0 → 2.1.1 (pnpm-lock.yaml), which the body does not mention. |
| Included in directory profile with a factual description and reviewed annotations | ✅ | src/specgen/profile.ts:63, specgen/generator-config.yaml:179, tests/specgen-annotations.test.ts:120 (TFTT, same as list-billing) |
| Byte budget 64,000 → 66,000, reason given | ✅ | tests/specgen-gates.test.ts:361-363. Not re-measured (no test runs here); CI test jobs pass on Ubuntu 20/22/24 and Windows 22. |
| "Its seven filters remain visible" | The tool has 7 query params, but cursor and limit are pagination. It has 5 filters. See nit. |
|
| Minor changeset names the audit log feature | ✅ | .changeset/production-spec-update.md |
Findings
Critical
None.
High
None.
Medium
None.
Low
None.
Nits
specgen/generator-config.yaml:179listAuditLogssits at the top ofdirectoryDescriptions; the other keys are alphabetical, so it belongs just beforelistServerlessBilling.tests/specgen-gates.test.ts:362"seven filters" countscursorandlimit; five of the params are filters. Same wording in the PR body.
Simplification opportunities
None. The hand-written change is four config/test lines plus regenerated output.
Verified clean
- Directory policy:
list-audit-logstakes no credential input, is a read of the account's own data, and none of exclusion rules 1 to 6 apply. The default description contains a directive ("Followpagination.nextCursor..."), so thedirectoryDescriptionoverride is required, and it states facts and names no tool. DIRECTORY_TOOLSandexpectedHintsentries are in alphabetical position.- Generated tool:
explode: falseonactorIdsandactionsmatches the spec'sstyle: form, explode: false;limitbounds 1 to 200 matchPaginationLimitAuditLogs. The "Comma-separated" wording on array params matches existing tools (list-gpu-types,list-cpu-types), so it is not new. - Other spec changes are response-side only (new
400/413responses,RequestTooLargeError, list-pods examples now showruntime: nullanddirect: null). No curated tool or description relies onruntimeorssh.directfromlist-pods(greppedsrc/specgen/tools,server.ts,generator-config.yaml). - CI: all checks pass, including
spec-driftand the Windows test job.report-update-pris skipped by design.
Questions for the author
- The new operation requires scope
audit:read(x-runpod-scope). Does the OAuth-minted key fromapi/index.ts(flash auth mutation) carry that scope, or will hosted users always get 403 beyond the plan gate? Nothing in this repo pins key scopes, so I could not verify it. list-podsresponses now omitruntimeandssh.direct(API-side change). Should the changeset mention it, since agents that read utilization or the direct SSH mapping from a list now needget-pod?
Reviewed by Claude (claude-opus-5-5) via the pr-review skill. Tokens: 26 in / 6,534 out / 752,634 cache read / 63,286 cache write = 822,480. Estimated cost: $1.78 at the sum_usage.py rate card for claude-opus-5-5, 5-minute cache writes (1.25x input). Round 1 of this PR. This is automated assistance; a human owns the final call.
| # connections keep the descriptions above. The generator rejects keys whose | ||
| # operationId is absent from the spec. | ||
| directoryDescriptions: | ||
| listAuditLogs: >- |
There was a problem hiding this comment.
Nit: The other directoryDescriptions keys are alphabetical (createCluster ... updateEndpoint). Move listAuditLogs to just before listServerlessBilling.
There was a problem hiding this comment.
Fixed in e39a43e: moved listAuditLogs into alphabetical order before listServerlessBilling. Generation and tests pass.
| // tool cannot regrow it unnoticed; raise one only with the reason in the PR. | ||
| const GENERATED_SURFACE_BUDGET_BYTES = 64_000; | ||
| // The audit-log lookup adds 2,418 B to the default tools/list surface. Keep | ||
| // its seven filters visible; this 2 KB increase covers the new read-only tool. |
There was a problem hiding this comment.
Nit: list-audit-logs has seven query params, but cursor and limit are pagination. It has five filters (actorIds, actions, startTime, endTime, search).
There was a problem hiding this comment.
Fixed in e39a43e: the budget comment now says five filters and two pagination inputs. The PR body uses the same wording.
|
Addressed the two review questions in e39a43e and the PR body. The changeset now says that list-pods returns null for live utilization and the direct SSH mapping, with get-pod needed for those fields. For audit access, hosted sign-in obtains a regular account API key and forwards it to v2; this repository cannot prove the v2 audit:read mapping or an account plan entitlement. I have recorded the need for a hosted OAuth smoke test on an eligible account rather than claim that path was verified. |
Summary
list-audit-logstool. The TypeScript SDK and plugin knowledge package versions remain unchanged.tools/listbyte budget from 64,000 to 66,000. The new tool contributes 2,418 bytes; the measured generated surface is 65,552 bytes. Its five filters and two pagination inputs remain visible without an extra schema lookup.list-podsnow returns null for live utilization and the direct SSH mapping;get-podprovides those fields.Validation
Local checks pass:
pnpm generate:check,pnpm type-check,pnpm lint,pnpm test(523 tests),pnpm build, andpnpm spec:checkagainst the live production spec (67 operations). GitHub checks pass: spec drift, Node 20/22/24, and Windows Node 22.The branch already contains the current
maincommit, so no rebase commit was needed. This PR updates the spec; the compression change remains in #150.Auth verification
The hosted sign-in path requests a regular account API key and forwards it to the v2 API. The vendored operation declares
audit:read, but this repo does not prove how v2 maps that key to the scope or whether an account plan includes audit logs. A hosted OAuth smoke test with an eligible account remains needed after deployment.