Skip to content

Document get_deployment_payload / delete_deployment_payload response contracts#600

Open
kriszyp wants to merge 1 commit into
mainfrom
kris/1893-payload-ops-docs
Open

Document get_deployment_payload / delete_deployment_payload response contracts#600
kriszyp wants to merge 1 commit into
mainfrom
kris/1893-payload-ops-docs

Conversation

@kriszyp

@kriszyp kriszyp commented Jul 22, 2026

Copy link
Copy Markdown
Member

Companion to HarperFast/harper#1898 "Implement get_deployment_payload and delete_deployment_payload operations", which implements the two operations (documented since 5.1 but returning Operation not foundharper#1893).

Adds the previously-unspecified behavioral contracts:

  • get_deployment_payload — response is raw tarball bytes (octet-stream + download filename), never JSON/base64; 404 when the payload was reclaimed.
  • delete_deployment_payload — cluster-wide reclaim note, terminal-status requirement (409 otherwise), idempotency (freed_bytes: 0), response shape, and the payload_dropped audit event.

Hold merge until harper#1898 lands.

🤖 Generated with Claude Code (Opus 4.8)

…contracts

Companion to HarperFast/harper#1898 (which implements the two operations —
they were documented but returned "Operation not found", harper#1893):
raw-bytes response for get_deployment_payload, and the terminal-status
requirement, idempotency, response shape, and audit event for
delete_deployment_payload.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the API documentation in reference/operations-api/operations.md to detail the response format for retrieving a deployment's raw tarball and to expand on the behavior, constraints, and response schema of the delete_deployment_payload endpoint. The review feedback suggests a minor formatting improvement to use an em-dash instead of a hyphen for consistency.

}
```

The response is the raw tarball bytes (`Content-Type: application/octet-stream`, with a `Content-Disposition` download filename) - not JSON and not base64-encoded, so payloads of any size stream without inflation. Returns `404` if the deployment does not exist or its payload has already been reclaimed (by payload retention or `delete_deployment_payload`).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency with the rest of the document, use an em-dash () instead of a hyphen (-) to separate the clauses.

Suggested change
The response is the raw tarball bytes (`Content-Type: application/octet-stream`, with a `Content-Disposition` download filename) - not JSON and not base64-encoded, so payloads of any size stream without inflation. Returns `404` if the deployment does not exist or its payload has already been reclaimed (by payload retention or `delete_deployment_payload`).
The response is the raw tarball bytes (`Content-Type: application/octet-stream`, with a `Content-Disposition` download filename) not JSON and not base64-encoded, so payloads of any size stream without inflation. Returns `404` if the deployment does not exist or its payload has already been reclaimed (by payload retention or `delete_deployment_payload`).

@github-actions

Copy link
Copy Markdown

🚀 Preview Deployment

Your preview deployment is ready!

🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-600

This preview will update automatically when you push new commits.

@kriszyp
kriszyp marked this pull request as ready for review July 23, 2026 00:23
@kriszyp
kriszyp requested a review from a team as a code owner July 23, 2026 00:23
}
```

The response is the raw tarball bytes (`Content-Type: application/octet-stream`, with a `Content-Disposition` download filename) - not JSON and not base64-encoded, so payloads of any size stream without inflation. Returns `404` if the deployment does not exist or its payload has already been reclaimed (by payload retention or `delete_deployment_payload`).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium: get_deployment_payload's super_user requirement is stricter than delete_deployment_payload's, but the doc doesn't say so

Both ops are registered with requires_su: true in operation_authorization.ts, and that map has a Gate-2 bypass: an op that is SU-only by default can still be granted to a non-super_user role via that role's operations allowlist. delete_deployment_payload (and nearly every other super_user op in this table) goes through that path unmodified. get_deployment_payload is the exception — harper#1898 adds a requireSuperUser() check directly inside handleGetDeploymentPayload, so it 403s even for a role that explicitly lists it in operations (matching the secrets-store ops, per that PR's own comment). An admin who grants get_deployment_payload the same way they successfully granted delete_deployment_payload will hit an unexplained 403.

This PR's stated goal is documenting "previously-unspecified behavioral contracts" for these two ops, so this asymmetry belongs here rather than staying implicit in the super_user table label both ops share.

Suggested fix: append a sentence to this paragraph, e.g. "Unlike most other super_user operations, this check is enforced directly in the handler and cannot be satisfied by granting the operation through a role's operations allowlist — only an actual super_user role can call it."


Generated by Barber AI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants