Add agent_kit helpers: workspaces, forks, checkpoints, coordination#23
Add agent_kit helpers: workspaces, forks, checkpoints, coordination#23katieschilling wants to merge 11 commits intomainfrom
Conversation
Mirrors the @tigrisdata/agent-kit (TypeScript) public surface as Python
helpers on top of this library's existing snapshot/fork primitives.
- Workspaces: create_workspace / teardown_workspace, with optional TTL
(via lifecycle rules) and snapshot enablement
- Forks: create_forks / teardown_forks — snapshot a base bucket then
create N copy-on-write forks
- Checkpoints: checkpoint / restore / list_checkpoints — labeled
snapshots with a Checkpoint dataclass
- Coordination: setup_coordination / teardown_coordination — webhook
notifications via Tigris's PATCH /{bucket} REST endpoint
A new internal _rest.py module provides direct urllib3 + SigV4 access
to the Tigris-specific PATCH /{bucket} JSON API (mirrors bundle.py's
pattern). Per-workspace and per-fork scoped credentials are not
included; they require Tigris IAM API support outside the boto3 surface.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Greptile SummaryThis PR ports the TypeScript
Confidence Score: 3/5Safe to merge after fixing the One P1 present: invalid tigris_boto3_ext/agent_kit.py — specifically the Important Files Changed
Reviews (4): Last reviewed commit: "Address review feedback: standard S3/IAM..." | Re-trigger Greptile |
Tigris was rejecting PATCH /{bucket} with SignatureDoesNotMatch because
the canonical request used during signing did not include the body
hash header. SigV4 needs X-Amz-Content-Sha256 set before add_auth()
runs so it is included in the canonical headers and signed.
This mirrors the existing bundle.py implementation.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds `credentials_role` ("Editor" | "ReadOnly") to create_workspace and
create_forks. When set, a scoped Tigris access key is provisioned per
bucket and returned on Workspace.credentials / Fork.credentials.
teardown_workspace and teardown_forks revoke the keys before deleting.
Adds a new internal `_iam.py` HTTP transport that mirrors the JS
@tigrisdata/iam SDK: POSTs SigV4-signed (service "iam") form-encoded
requests to https://iam.storageapi.dev/ — `CreateAccessKeyWithBucketsRole`
for provisioning and the AWS-IAM-compatible `DeleteAccessKey` for
revocation. Override the endpoint with the TIGRIS_IAM_ENDPOINT env var.
TigrisIAMError surfaces non-2xx responses.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #23 +/- ##
==========================================
+ Coverage 99.24% 99.59% +0.35%
==========================================
Files 6 9 +3
Lines 266 499 +233
==========================================
+ Hits 264 497 +233
Misses 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Adds targeted tests for previously-uncovered error and edge paths so codecov/patch hits 100% on the diff: - _empty_bucket: actual versioned-listing happy path and the unversioned fallback when list_object_versions raises - _iam.py response parsing: empty body, malformed JSON, JSON missing AccessKey, malformed XML, XML missing SecretAccessKey - _rest.py: malformed JSON response agent_kit.py, _iam.py, _rest.py, and bundle.py are now at 100% line coverage. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Three P1 issues from the Greptile review: 1. _empty_unversioned aborted on first delete_object failure, leaving the bucket non-empty so teardown_workspace's later delete_bucket would raise BucketNotEmpty. Each per-object delete is now independently suppressed. 2. Checkpoint equality was broken between checkpoint() (client-side datetime.now) and list_checkpoints() (server-side CreationDate) because the two paths sourced created_at differently. created_at is now excluded from dataclass equality so a checkpoint can be matched against a listing. 3. create_forks used break on a per-fork failure, silently abandoning the rest. It now skips the failing fork and continues — partial failures under-deliver but no longer cut off subsequent attempts. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
cursor review |
Bugbot couldn't runBugbot is not enabled for your user on this team. Ask your team administrator to increase your team's hard limit for Bugbot seats or add you to the allowlist in the Cursor dashboard. |
| @@ -0,0 +1,204 @@ | |||
| """Tigris IAM HTTP transport for managing access keys. | |||
There was a problem hiding this comment.
Why do we need this? Tigris is IAM compatible, existing IAM sdk should work out of the box. Instead of doing it this way, better to do it as follows:
- use existing s3 iam sdk
- create iam policy that restricts the access key to a particular bucket
- attach the iam policy to the access key
There was a problem hiding this comment.
Done in a7926b8 — replaced the custom SigV4 transport with boto3.client("iam", endpoint_url="https://iam.storageapi.dev", ...) reusing the S3 client's credentials. The flow is now exactly what you described:
CreateAccessKeyCreatePolicywith a bucket-scoped policy document (s3:*for Editor, GetObject/ListBucket/GetBucketLocation for ReadOnly, resource =arn:aws:s3:::{bucket}+/*)AttachUserPolicy
Teardown reverses it: DetachUserPolicy → DeletePolicy → DeleteAccessKey.
One thing worth flagging that I had to discover by probing real Tigris: AttachUserPolicy rejects the UserName field returned by create_access_key ("auto") — Tigris treats the access key id itself as the UserName handle. There's a comment in _iam.py calling that out so the next person doesn't trip on it.
Verified end-to-end against the real Tigris IAM endpoint; all 13 integration tests pass.
| self.body = body | ||
|
|
||
|
|
||
| def patch_bucket_settings( |
There was a problem hiding this comment.
Tigris supports the PutBucketLifecycleConfiguration API so lifecycle rules can be set using standard S3 API.
Instead of having a generic patch_bucket_settings function, it is best to expose a function that allows users to configure object notifications. The function will be public and live in the object_notifications.py file.
There was a problem hiding this comment.
Both addressed in a7926b8.
TTL via standard S3: create_workspace now calls s3_client.put_bucket_lifecycle_configuration(...) directly. No custom transport involved.
Object notifications as a public module: _rest.py is deleted. New public file at tigris_boto3_ext/object_notifications.py exposes set_object_notifications(s3_client, bucket, *, webhook_url, event_filter, auth_token, auth_username, auth_password) and clear_object_notifications(s3_client, bucket), plus ObjectNotificationsError. The Tigris-specific PATCH /{bucket} JSON transport is inlined privately in that file (only this one feature needs it now). setup_coordination / teardown_coordination in agent_kit are now thin wrappers around the public functions.
Three threads of feedback from the maintainer review:
1. _iam.py: replaced the custom SigV4 + custom CreateAccessKeyWithBucketsRole
transport with the standard boto3 IAM client pointed at the Tigris IAM
endpoint. The bucket-scoped flow now goes through standard AWS IAM
actions: CreateAccessKey + CreatePolicy + AttachUserPolicy
(and DetachUserPolicy + DeletePolicy + DeleteAccessKey on teardown).
Tigris IAM treats the access key id itself as the "user" handle for
AttachUserPolicy/DetachUserPolicy — verified end-to-end against real
Tigris.
2. _rest.py: deleted. TTL on workspaces now uses the standard S3
PutBucketLifecycleConfiguration API (not a Tigris extension).
Object-event webhook notifications still need a Tigris-specific
PATCH /{bucket} JSON endpoint, but that transport is now inlined in a
new public object_notifications.py module exposing
set_object_notifications / clear_object_notifications. agent_kit's
setup_coordination / teardown_coordination are thin wrappers.
3. create_forks: when _provision_credentials raises mid-loop, the fork
bucket is now still appended to the result (with credentials=None) so
the caller has a handle to pass to teardown_forks. ValueError on a
bad role propagates immediately (programmer error).
The Credentials dataclass grew user_name and policy_arn fields (compared
out of equality and repr) to track the bookkeeping needed for the
DetachUserPolicy/DeletePolicy teardown sequence.
13 integration tests against real Tigris all pass; 86 unit tests pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Greptile flagged that an invalid credentials_role on create_workspace or create_forks would create the bucket (and snapshot/forks for the plural case) before _provision_credentials ran the role check, leaving the caller with an exception and no handle to clean up. Both functions now run a single upfront _validate_role check before any S3 call, so a bad role fails fast with no side effects. The redundant ValueError handling in _try_provision_credentials is gone. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
| ### 4. Agent Kit | ||
|
|
||
| Higher-level workflows for AI agents — workspaces, parallel forks, checkpoints, | ||
| and event-driven coordination — composed on top of snapshots and forks. Mirrors |
There was a problem hiding this comment.
Shouldn't be mirroring the TypeScript project. This is Python :) The file-level comment should reflect what is implemented.
Let's remove the "mirroring Typescript" comment from here and the other place.
There was a problem hiding this comment.
Done in 8f1d60e — dropped the "mirrors @tigrisdata/agent-kit" framing from the README intro and the agent_kit.py module docstring.
There was a problem hiding this comment.
You were right, there was still one stale reference left — object_notifications.py's module docstring referred to setup_coordination / teardown_coordination as thin wrappers, even though those functions were removed back in 8f1d60e. Cleaned up in aa933a4.
The README intro and the agent_kit.py module docstring no longer mention mirroring or the TS package; this was the last leftover.
| name: str, | ||
| *, | ||
| ttl_days: Optional[int] = None, | ||
| enable_snapshots: bool = False, |
There was a problem hiding this comment.
Why not have it enabled by default?
There was a problem hiding this comment.
Done in 8f1d60e — enable_snapshots=True is now the default since checkpointing is the common path. Set enable_snapshots=False to opt out.
| enable_snapshots: bool = False, | ||
| credentials_role: Optional[Role] = None, | ||
| ) -> Workspace: | ||
| """Create a workspace bucket for an agent. |
There was a problem hiding this comment.
Why call it workspace bucket? Call it just workspace
There was a problem hiding this comment.
Done in 8f1d60e — docstring now just says "workspace". The Workspace dataclass docstring is also tightened to "A single-agent workspace".
There was a problem hiding this comment.
Tightened further in b7d6f3c. The function summary was already "Create a workspace for an agent." — the bucket parameter's Args description was still leaking "Workspace bucket name", which is now just "Name for the workspace."
|
|
||
| Args: | ||
| s3_client: boto3 S3 client. | ||
| name: Bucket name. |
There was a problem hiding this comment.
Similarly, name is workspace name. I think calling it both workspace and bucket is a bit odd.
There was a problem hiding this comment.
Done in 8f1d60e — renamed the parameter from name to bucket to match checkpoint(s3, bucket, …), restore(s3, bucket, …), etc. Now consistent across the agent_kit surface.
There was a problem hiding this comment.
Same fix as above (b7d6f3c) — Args description for bucket no longer says "Bucket name" alongside "workspace". The parameter is renamed to bucket for consistency with checkpoint/restore, and described as "Name for the workspace."
| if workspace.credentials is not None: | ||
| with suppress(Exception): | ||
| _revoke(s3_client, workspace.credentials) | ||
| if force: |
There was a problem hiding this comment.
Tigris supports force deleting a bucket. The bucket doesn't need to be emptied. Check the implementation in the typescript sdk https://www.tigrisdata.com/docs/sdks/tigris/using-sdk/#deleting-a-bucket.
You can implement a delete_bucket function that takes the force parameter.
You can also remove the _empty_bucket function.
There was a problem hiding this comment.
Done in 8f1d60e. New public helper delete_bucket(s3_client, bucket, *, force=False) in helpers.py sends Tigris-Force-Delete: true (matching the JS SDK pattern). teardown_workspace / teardown_forks use it; _empty_bucket / _empty_versioned / _empty_unversioned are gone.
| return None | ||
|
|
||
|
|
||
| def _revoke(s3_client: S3Client, credentials: Credentials) -> None: |
There was a problem hiding this comment.
This function is providing no value. delete_scoped_access_key can be called directly
There was a problem hiding this comment.
Removed in 8f1d60e — teardown_workspace / teardown_forks call delete_scoped_access_key directly.
| ) | ||
|
|
||
|
|
||
| def _empty_bucket(s3_client: S3Client, bucket: str) -> None: |
There was a problem hiding this comment.
Removed in 8f1d60e — _empty_bucket is gone; teardown uses delete_bucket(force=True) which hits Tigris's force-delete extension.
| _empty_unversioned(s3_client, bucket) | ||
|
|
||
|
|
||
| def _empty_versioned(s3_client: S3Client, bucket: str) -> bool: |
| return True | ||
|
|
||
|
|
||
| def _empty_unversioned(s3_client: S3Client, bucket: str) -> None: |
There was a problem hiding this comment.
Removed in 8f1d60e (along with _empty_versioned / _empty_bucket).
| raise ValueError(msg) | ||
|
|
||
|
|
||
| def _patch_bucket( |
There was a problem hiding this comment.
Better to call it _update_bucket_settings
There was a problem hiding this comment.
Done in 8f1d60e — renamed to _update_bucket_settings.
Address review feedback from ovaistariq:
- create_workspace: rename `name` parameter to `bucket` for consistency
with the rest of the surface; default `enable_snapshots=True` since
checkpointing is the common path; on credential-provisioning failure,
roll back the bucket so the caller isn't left with an orphan
- teardown_workspace / teardown_forks: drop the manual _empty_bucket
helpers entirely and use Tigris's standard force-delete extension
via a new public helpers.delete_bucket(s3, bucket, *, force=False)
(sends Tigris-Force-Delete: true)
- restore: default fork name now embeds the snapshot id
(`f"{bucket}-restore-{snapshot_id}"`) so the bucket name reflects the
point-in-time it represents, not when restore happened
- create_forks: default prefix likewise switches to snapshot_id-based
for the same reason and to avoid timestamp collisions
- Remove the thin pass-through wrappers and dead code: list_checkpoints
(callers can use list_snapshots and parse the Name field),
setup_coordination / teardown_coordination (use
set_object_notifications / clear_object_notifications directly),
internal _revoke and _empty_bucket family
- Rename object_notifications._patch_bucket -> _update_bucket_settings
- Drop the "mirrors @tigrisdata/agent-kit" framing from the module
docstring and README; describe the implementation instead
13 integration tests against real Tigris pass (incl. force-delete on
non-empty buckets and the credential rollback path); 81 unit tests pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Conflicting hunks in README.md and tigris_boto3_ext/__init__.py resolved in favor of main (rename_object additions). Agent Kit / object notifications exports kept since they're disjoint additions that were auto-merged at the import level. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
object_notifications.py's module docstring still pointed at ``setup_coordination`` / ``teardown_coordination`` as thin wrappers, but those were removed in 8f1d60e. The reference was a leftover from the agent-kit framing the maintainer asked to drop. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ovaistariq pushed back twice on the \"workspace bucket\" framing: - 3165580620: \"Why call it \`workspace bucket\`? Call it just \`workspace\`\" - 3165581906: \"\`name\` is workspace name. Calling it both workspace and bucket is a bit odd.\" The function summary was already cleaned up. The Args entry for the \`bucket\` parameter still leaked the dual framing (\"Workspace bucket name.\"), so simplify it to \"Name for the workspace.\" Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2 participants
Summary
Ports the
@tigrisdata/agent-kit(TypeScript) public surface to Python, on top of this library's existing snapshot/fork primitives.What's new
create_workspace/teardown_workspace— bucket with optional TTL (lifecycle rule) and optional snapshot enablementcreate_forks/teardown_forks— snapshot a base bucket then create N independent copy-on-write forkscheckpoint/restore/list_checkpoints+Checkpointdataclass — labeled snapshots and snapshot-based restoresetup_coordination/teardown_coordination— webhook notifications via Tigris'sPATCH /{bucket}REST endpointThe
Workspace,Fork,ForkSet, andCheckpointdataclasses are also exported.How it works
create_snapshot,create_fork, etc.).PATCH /{bucket}JSON endpoint that's not part of the S3 API. A new internal_rest.pymodule provides this (urllib3 + SigV4, mirroringbundle.py's pattern).TigrisRestErroris exported for error handling.Out of scope
Test plan
uv run ruff check tigris_boto3_ext— passesuv run ruff format --check tigris_boto3_ext— passesuv run mypy tigris_boto3_ext— passes (8 source files)uv run pytest tests/ --ignore=tests/integration— 59 passed (39 new)AWS_ENDPOINT_URL_S3etc. to run. Worth a manual run before merge to validate thePATCH /{bucket}payload shapes for TTL and notifications.🤖 Generated with Claude Code