Skip to content

fix deployment logs response contract#409

Merged
timflannagan merged 4 commits intomainfrom
api-refactor-final-sweep
Mar 27, 2026
Merged

fix deployment logs response contract#409
timflannagan merged 4 commits intomainfrom
api-refactor-final-sweep

Conversation

@ilackarms
Copy link
Copy Markdown
Contributor

Description

This PR finishes the OSS side of the deployment-logs contract cleanup for the API refactor sweep.

Motivation:
The unified deployment logs path had drift between intended behavior, and tests. Enterprise requires structured JSON logs response, but OSS tests and response typing still reflected the older 204/header-style contract.

What changed:

  • Updated the deployment logs response shape to return a structured JSON body with deploymentId, status, and logs
  • Updated the /v0/deployments/{id}/logs handler to populate the deployment status in that response
  • Tightened deployment handler coverage so logs/cancel behavior is pinned for:
    • happy-path logs
    • happy-path cancel
    • 501 not-supported mappings for logs and cancel

Change Type

/kind fix
/kind cleanup

Changelog

NONE

Return deployment logs as a structured JSON response and lock in the logs/cancel adapter mappings so clients and follow-up enterprise changes can rely on a stable contract.
@ilackarms ilackarms marked this pull request as ready for review March 26, 2026 18:03
Copilot AI review requested due to automatic review settings March 26, 2026 18:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aligns the OSS /v0/deployments/{id}/logs endpoint with the refactored “structured JSON logs” contract (matching Enterprise expectations), updating the OpenAPI spec, generated UI client types, server handler, and tests.

Changes:

  • Updated /v0/deployments/{id}/logs from a 204/header-style contract to a 200 JSON response containing deploymentId, status, and logs.
  • Updated the v0 deployments logs handler to include deployment status in the response body.
  • Expanded handler tests to cover happy-path logs/cancel and 501 Not Implemented mappings for “not supported” behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
ui/lib/api/types.gen.ts Adds DeploymentLogsBody and updates getDeploymentLogs response typing to 200 JSON.
ui/lib/api/index.ts Re-exports the newly generated DeploymentLogsBody type.
openapi.yaml Updates the logs endpoint response to 200 with JSON schema and adds DeploymentLogsBody schema.
internal/registry/api/handlers/v0/deployments_test.go Updates logs tests for 200 JSON body and adds 501 not-supported coverage for logs/cancel.
internal/registry/api/handlers/v0/deployments.go Populates deployment status in the logs response body.
internal/registry/api/apitypes/types.go Introduces DeploymentLogsBody plus a response wrapper type.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

// DeploymentLogsBody is the JSON body returned for deployment logs requests.
type DeploymentLogsBody struct {
DeploymentID string `json:"deploymentId"`
Status string `json:"status,omitempty"`
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

status is currently optional (omitempty) in the Go response body, but the handler always populates it (and the PR description implies it’s part of the new contract). Consider making status required by removing omitempty so clients can reliably depend on it being present and the API contract stays consistent across server/OpenAPI/client types.

Suggested change
Status string `json:"status,omitempty"`
Status string `json:"status"`

Copilot uses AI. Check for mistakes.
type: string
required:
- deploymentId
- logs
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

DeploymentLogsBody.status is defined as optional (not listed in required), but the OSS handler always returns a status and the PR describes deploymentId, status, and logs as the cleaned-up response contract. Consider adding status to the schema required list (and keeping server/client types in sync) so generated clients treat it as always present.

Suggested change
- logs
- logs
- status

Copilot uses AI. Check for mistakes.
Keep the deployment logs JSON contract consistent across the Go response type, OpenAPI schema, and generated UI client types so verify passes with the reviewed logs change.
@timflannagan timflannagan added this pull request to the merge queue Mar 27, 2026
Merged via the queue into main with commit 89dc215 Mar 27, 2026
8 checks passed
@timflannagan timflannagan deleted the api-refactor-final-sweep branch March 27, 2026 15:20
lleon-at-navteca added a commit to Navteca/agentregistry that referenced this pull request Mar 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants