chore(sdk): expose MCP approval bindings (ALIEN-824) - #631
lilienblum wants to merge 22 commits into
Conversation
…627) ## Summary - define one canonical typed Operations manifest across Rust and TypeScript - bind version-qualified operation dispatch to an immutable result contract persisted before execution - validate, store, redact, and verify command results through Platform without direct Manager reads - harden custom bundle ZIP validation and deterministic generated SDK output ## Stack - based on #629 (ALIEN-845 generated Platform SDK baseline) - companion Platform PR: pending creation - reviewed OSS tip: b479115 - reviewed Platform tip: eaf4ca2e5648aa3b3c909c2d591e0796bbd54e33 ## Validation - cargo test -p alien-cli commands::operations (37 passed) - cargo test -p alien-cli remote_sandbox -- --nocapture (7 passed) - cargo test -p alien-operations-sdk - cargo test -p alien-helm --test generator operator_manifest - RUSTDOCFLAGS=-Dwarnings cargo doc -p alien-operations-sdk --no-deps - RUSTFLAGS=-Dwarnings cargo check -p alien-operations-sdk --all-targets - cargo check -p alien-cli - fresh detached pinned Speakeasy 1.680.11 pnpm generate:platform-api reproduction; generation, TypeScript build, lock refresh, and 13 wire tests passed and left the checkout clean - Platform focused operations suites: 6 files, 97 tests passed - Platform API and platform-core TypeScript checks passed - cargo check -p alien-managerx passed in the Platform companion - git diff --check Independent pr-review passed with no correctness, security, compatibility, scope, or public-boundary blockers.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
|
| pluginVersion: z.string().optional(), | ||
| operationContractHash: z.string().optional(), |
There was a problem hiding this comment.
Binding constraints are missing
The OpenAPI contract requires pluginVersion to be non-empty and operationContractHash to contain exactly 64 lowercase hexadecimal characters, but these outbound validators accept any string. Since operationsCreateAccessRequest uses this schema to validate requests, malformed bindings are sent to the API instead of returning an SDK validation error. This is a non-blocking consistency issue between the generated client and its public contract.
Knowledge Base Used: Client SDKs and TypeScript packages
Prompt To Fix With AI
This is a comment left during a code review.
Path: client-sdks/platform/typescript/src/models/createaccessrequest.ts
Line: 115-116
Comment:
**Binding constraints are missing**
The OpenAPI contract requires `pluginVersion` to be non-empty and `operationContractHash` to contain exactly 64 lowercase hexadecimal characters, but these outbound validators accept any string. Since `operationsCreateAccessRequest` uses this schema to validate requests, malformed bindings are sent to the API instead of returning an SDK validation error. This is a non-blocking consistency issue between the generated client and its public contract.
**Knowledge Base Used:** [Client SDKs and TypeScript packages](https://app.greptile.com/alien/-/custom-context/knowledge-base/alienplatform/alien/-/docs/client-sdks-and-typescript-packages.md)
---
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!
There was a problem hiding this comment.
Confirmed: the source OpenAPI contract and server enforce these constraints. Speakeasy v1.680.11 currently emits structural z.string() validators for these string constraints throughout this generated SDK. The repository policy forbids hand-editing generated files, and persistent edits are disabled, so patching just these two generated fields would make regeneration non-reproducible. I kept the official generator output unchanged; malformed values still fail at the API boundary. This generator limitation is now documented in the PR.
| pristine_git_object: 8bfe2a424a7d98d235de9f1038cefa0a23b92c35 | ||
| docs/sdks/operations/README.md: | ||
| last_write_checksum: sha1:04d63807f0f204e4890a8ebb18dcf96658eee0eb | ||
| last_write_checksum: sha1:8766a646ae956a1cb0fe1fb5c21d9d4385e6dde7 |
There was a problem hiding this comment.
Several updated last_write_checksum values do not match the generated files committed in this PR. For example, this entry records 8766a646… for docs/sdks/operations/README.md, while that file has the restored checksum 04d63807…, which was also the previous lock value. Similar mismatches affect other restored endpoint and model files. This leaves the generator metadata inconsistent with the committed tree and may cause later regeneration or drift checks to treat restored files as changed against output that was never committed.
Knowledge Base Used: Client SDKs and TypeScript packages
Prompt To Fix With AI
This is a comment left during a code review.
Path: client-sdks/platform/typescript/.speakeasy/gen.lock
Line: 24375
Comment:
**Lock checksums are stale**
Several updated `last_write_checksum` values do not match the generated files committed in this PR. For example, this entry records `8766a646…` for `docs/sdks/operations/README.md`, while that file has the restored checksum `04d63807…`, which was also the previous lock value. Similar mismatches affect other restored endpoint and model files. This leaves the generator metadata inconsistent with the committed tree and may cause later regeneration or drift checks to treat restored files as changed against output that was never committed.
**Knowledge Base Used:** [Client SDKs and TypeScript packages](https://app.greptile.com/alien/-/custom-context/knowledge-base/alienplatform/alien/-/docs/client-sdks-and-typescript-packages.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Fixed in 4d75fc7. I restored the prior known-good lock state, regenerated the complete SDK from the checked-in OpenAPI input with official Speakeasy v1.680.11, and committed the required output. Running the repository's supported pnpm run generate:platform-api wrapper again from that clean head completed successfully and produced no diff, so the lock and committed tree are aligned.
💡 Codex Reviewhttps://github.com/alienplatform/alien/blob/21b92711917d64322c3fc21c826fc1dbf580e48a/client-sdks/platform/openapi.json#L1 When an ECS operator installation enables S3 or SQS operations, this contract now requires callers to supply AGENTS.md reference: client-sdks/AGENTS.md:L37-L42 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Fixed in |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d75fc7665
ℹ️ 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".
| s3BucketArns?: Array<string> | undefined; | ||
| /** | ||
| * Installer-owned SQS ceiling. Required when enabled SQS operations read queue attributes; the generated role contains only these queue ARNs. | ||
| */ | ||
| sqsQueueArns?: Array<string> | undefined; |
There was a problem hiding this comment.
Remove unrelated ECS installer changes
This approval-binding feature regeneration also adds the unrelated ECS S3/SQS resource-ceiling API and updates its documentation, response text, and error handling. Shipping those changes here couples a separate public API feature to ALIEN-824 and violates the SDK policy requiring unrelated generated endpoints/models to be restored and refreshed in a dedicated SDK-sync PR. Keep the access-request binding changes in this commit and move the ECS installer regeneration to that separate sync.
AGENTS.md reference: client-sdks/AGENTS.md:L37-L42
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…/alien-824-mcp-approval-sdk-sync # Conflicts: # client-sdks/platform/openapi.json # client-sdks/platform/rust/openapi-3.0.json # client-sdks/platform/rust/openapi.json # client-sdks/platform/typescript/.speakeasy/gen.lock
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5d71b052f
ℹ️ 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".
| pluginVersion?: string | undefined; | ||
| operationContractHash?: string | undefined; |
There was a problem hiding this comment.
Add behavioral tests for approval binding fields
At the exact head, client-sdks/platform/scripts/typescript-sdk.test.mjs has no access-request case, so the SDK build and all 13 existing contract tests remain green if a later regeneration drops pluginVersion or operationContractHash during outbound serialization or from any inbound response model. Add a behavioral contract test that sends a plan-backed request and parses a returned command while asserting both security-binding values survive the wire round trip.
AGENTS.md reference: AGENTS.md:L457-L464
Useful? React with 👍 / 👎.
…k-sync' into lilienblum/alien-824-mcp-approval-sdk-sync
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6d66b1c83
ℹ️ 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".
| params: { sql: "select 1" }, | ||
| tier: "read-only", | ||
| pluginVersion: "1.2.3", | ||
| operationContractHash: "sha256:contract-test", |
There was a problem hiding this comment.
Use a schema-valid hash in the round-trip test
The new OpenAPI contract restricts operationContractHash to 64 lowercase hexadecimal characters, but this fixture includes a prefix and non-hex characters. Because the mocked fetcher does not perform server validation and the current generated Zod schema omits the pattern check, the test passes with a request that the real endpoint rejects; it also becomes a false failure if a future generator starts honoring the declared pattern. Use a valid 64-character lowercase hexadecimal value so the test exercises a production-valid round trip.
AGENTS.md reference: AGENTS.md:L457-L464
Useful? React with 👍 / 👎.
💡 Codex ReviewThe new OpenAPI contract restricts AGENTS.md reference: AGENTS.md:L457-L464 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary
Dependency
Validation
pnpm run generate:platform-apipnpm -C client-sdks/platform/typescript lintNODE_OPTIONS=--max-old-space-size=16384cargo build -p alien-platform-apiGenerator note
Speakeasy currently emits structural Zod string validators for these OpenAPI string constraints. The API contract and server enforce
pluginVersionnon-emptiness and the 64-character lowercase hexadecimal operation hash. This repository forbids hand-editing generated SDK output, so generator output is kept reproducible rather than patched manually.