Conversation
Greptile SummaryThis PR stamps every outbound cloud API request with
Confidence Score: 4/5Safe to merge after addressing three missing X-Arkor-Client / deprecation-wiring gaps in the Studio server. The CLI path (whoami, main.ts, core/client.ts) is well-implemented and the prior P1 concern about the 426 fallback is fully addressed. However, studio/server.ts has three outbound call sites that were not updated with the new version header or deprecation wiring, leaving Studio-originated traffic invisible to the cloud API's version gate. packages/arkor/src/studio/server.ts — three call sites need X-Arkor-Client and/or clientVersion/onDeprecation. Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as arkor CLI / Studio SPA
participant Main as cli/main.ts
participant Whoami as commands/whoami.ts
participant Client as core/client.ts
participant Dep as core/deprecation.ts
participant Cloud as Cloud API
CLI->>Main: main(argv)
Main->>Whoami: runWhoami()
Whoami->>Cloud: GET /v1/me (X-Arkor-Client: arkor/x.y.z)
alt 426 Upgrade Required
Cloud-->>Whoami: 426 + body
Whoami->>Whoami: formatSdkUpgradeError(body)
Whoami->>CLI: stderr: upgrade message
Whoami->>Main: process.exitCode = 1
else 200 OK + Deprecation: true
Cloud-->>Whoami: 200 + Deprecation/Sunset/Warning headers
Whoami->>Dep: onDeprecation callback → recordDeprecation()
Whoami-->>Main: return (success)
else 200 OK
Cloud-->>Whoami: 200
Whoami-->>Main: return (success)
end
Note over Main: finally block
Main->>Dep: getRecordedDeprecation()
alt notice present
Dep-->>Main: DeprecationNotice
Main->>CLI: log.warn(message + detectedUpgradeCommand())
end
Note over Client,Cloud: core/client.ts SSE & chat fetches
Client->>Cloud: fetch (X-Arkor-Client: arkor/x.y.z)
Cloud-->>Client: response
Client->>Dep: tapDeprecation(res, SDK_VERSION)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13cfddc1af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (upgrade) { | ||
| process.stderr.write(`${upgrade}\n`); | ||
| // Hard-block: scripts that gate on `arkor whoami`'s exit code must |
There was a problem hiding this comment.
whoami on every 426 response
process.exitCode = 1 is guarded by if (upgrade), so a 426 Upgrade Required response with a non-JSON or unexpected body (already possible because res.json() is wrapped in .catch(() => null)) can fall through to the generic path and return without setting a non-zero exit code. That makes arkor whoami succeed (0) even when the SDK is rejected by the server, which breaks scripts that rely on this command as a version gate.
Useful? React with 👍 / 👎.
| const message = | ||
| upgrade ?? fields.error ?? text ?? `cloud-api ${res.status}`; |
There was a problem hiding this comment.
The new message selection uses nullish coalescing with text, but text is initialized to "", so empty-body error responses now produce an empty CloudApiError.message instead of cloud-api <status>. This regresses diagnostics for non-OK responses that have no parsable payload, because callers lose the status-based fallback message entirely.
Useful? React with 👍 / 👎.
| * (the SSE and chat endpoints in `core/client.ts`, and the studio proxy). | ||
| */ | ||
| export function tapDeprecation(res: Response, sdkVersion: string): void { | ||
| if (res.headers.get("Deprecation") !== "true") return; |
There was a problem hiding this comment.
tapDeprecation only records notices when Deprecation equals the literal string "true", but RFC 9745 defines Deprecation as a structured date value (for example @1688169599). Any compliant date-valued header on raw fetch paths (SSE/chat and studio proxy flows that rely on tapDeprecation) will be ignored, so users will miss deprecation warnings and upgrade hints.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- Added deprecation notice recording and retrieval functionality in `core/deprecation.ts`. - Enhanced CLI commands to log deprecation warnings and include SDK version in responses. - Updated `tsdown.config.ts` to define `__SDK_VERSION__` from `package.json`. - Implemented version reporting in the CLI and API responses, ensuring users receive relevant upgrade hints. - Introduced package manager detection to provide tailored upgrade commands based on the user's environment. - Added tests for package manager detection and upgrade command generation. This update improves user experience by providing clear deprecation warnings and version information, facilitating smoother upgrades.
- Introduced a new test suite for the `arkor whoami` command to validate its interaction with the cloud-api SDK version gate. - Implemented a fake HTTP server to simulate API responses, including handling of 426 status codes for unsupported SDK versions. - Added tests to ensure correct upgrade command suggestions based on the package manager used (npm, pnpm, bun, yarn). - Verified that the X-Arkor-Client header is forwarded correctly and that deprecation warnings are surfaced appropriately. This enhances the test coverage for the CLI, ensuring users receive accurate feedback regarding SDK support and upgrade paths.
- Introduced a new test case to ensure that the `arkor whoami` command exits with code 1 when receiving a 426 status with a non-JSON body. - Updated the implementation in `whoami.ts` to handle 426 responses consistently, ensuring that users are informed about unsupported SDK versions. - Enhanced error messaging to provide clear upgrade instructions, improving user experience during version conflicts. This update strengthens the reliability of the CLI by preventing silent failures when the SDK is no longer supported.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db5f50e43d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const rpc = createClient({ | ||
| baseUrl, | ||
| token: getToken, | ||
| clientVersion: SDK_VERSION, | ||
| onDeprecation: recordDeprecation, |
There was a problem hiding this comment.
This change only wires clientVersion/onDeprecation into the /api/me proxy, but buildStudioApp still issues other cloud API requests (/api/jobs, /api/jobs/:id/events, /api/inference/chat) without X-Arkor-Client. If the server enforces the 426 SDK gate on those routes, Studio job lists/streams/chat can fail even when whoami works, because those requests are still effectively unversioned.
Useful? React with 👍 / 👎.
| try { | ||
| await program.parseAsync(argv, { from: "user" }); | ||
| } finally { | ||
| const notice = getRecordedDeprecation(); | ||
| if (notice) { |
There was a problem hiding this comment.
The deprecation warning flush now runs in main()'s finally, which executes as soon as parseAsync returns; for arkor dev, runDev returns immediately after starting the server, so this block runs before any later Studio proxy traffic can call recordDeprecation. In practice that means long-running invocations miss upgrade notices entirely unless they happened during startup.
Useful? React with 👍 / 👎.
1 participant
Summary
X-Arkor-Client: arkor/<semver>so the server can gate on the SDK version.426 Upgrade Requiredresponse to stderr with a package-manager-aware install command, and exit1.Deprecation(RFC 9745),Sunset(RFC 8594), andWarning(RFC 7234) response headers and surface a one-line notice at the end of every CLI invocation.What's new
SDK_VERSIONconstant inlined at build time via tsdown'sdefine, soarkor --versionand the wire-level header always matchpackage.json.X-Arkor-Clientrequest header added to every outbound call: the typed Hono RPC client, the SSE event stream and chat fetches incore/client.ts, and the Studio proxies.Package-manager detection in
core/upgrade-hint.tsinspectsnpm_config_user_agentfirst, then falls back to aprocess.argv[1]path heuristic:npm install -g arkor@latestpnpm add -g arkor@latestyarn global add arkor@latestbun add -g arkor@latestStudio proxy (
studio/server.ts) copies upstreamDeprecation/Sunset/Warningresponse headers to the browser SPA so Studio-only users see warnings too.arkor whoamion 426 setsprocess.exitCode = 1rather than callingprocess.exit(), so the deprecation-warning flush incli/main.tsstill runs before exit. Scripts gating onarkor whoaminow correctly fail-fast against an unsupported SDK.Requires
The matching
@arkor/cloud-api-clientalpha that exportsclientVersion,onDeprecation,upgradeMessageFromBody, andparseErrorBody.Test plan
pnpm test— newcore/upgrade-hint.test.tscovers the user-agent and path heuristics, including Windows-style separators, plus the npm/pnpm/yarn/bun command mapping.SKIP_E2E_INSTALL=1 pnpm --filter @arkor/e2e-cli test— newarkor-whoami.test.tsspins up an in-process fake cloud API and asserts:arkor whoamiagainst a 426 → exit1+ pm-aware stderr message for each of npm / pnpm / yarn / bunarkor whoamiagainst200+Deprecation: true→ body rendered + deprecation warning at end with pm-aware install commandX-Arkor-Client: arkor/<version>is forwarded on every requestpnpm build && pnpm pack --pack-destination /tmp/— leak greptar -xzOf /tmp/arkor-*.tgz | grep -cE 'drizzle|control-plane'returns0.