feat(cli): name each remote handle in the deployment-filtered status list

[aparajon]

Why this matters

An operator running a deployment-filtered status scan is asking the data plane's question: what is the control plane driving on my deployment? The handle they correlate with the data plane's own storage and logs is the data-plane apply id, but the list only ever showed the control-plane identifier, forcing a detour through detail views to find it.

What it does

• The deployment-filtered list names each remote handle in its own column, matching what the detail views already call them: EXTERNAL APPLY ID for the deployment's shared data-plane apply, EXTERNAL OP ID for the per-operation remote row.
• APPLY ID stays the control-plane id, so every id on screen can be fed straight back to status <apply_id> and the footer needs no qualifier.
• Columns follow the data in deployment mode. An optional column is dropped when no row on the page fills it, so a deployment that drives its applies locally never sees a remote-id column of dashes. A list filtered to one deployment also omits DEPLOYMENT, which every row would otherwise repeat back to the operator who named it.
• Outside deployment mode the flag's single EXTERNAL ID column always renders, dash per row without a recorded id — the operator asked for the column explicitly, and an all-dash column positively answers "nothing recorded" where a missing one is indistinguishable from the flag doing nothing.
• A row whose operations never recorded their own remote apply id falls back to the parent apply row's, which is where a drive that is not operation-scoped records it — the deployment's one remote handle is never hidden by an empty operation-level field.

Rendering the columns from one ordered list replaces the four-way printf switch, which had no room left for another shape.

Rendered result

Same fixture data either side, from the status_deployment preview.

Both remote handles recorded

Before:

  APPLY ID              EXTERNAL OP ID         DATABASE   ENV         DEPLOYMENT  STATE                STARTED        SOURCE
  apply-multi-a1b2c3d4  remote-op-us-east-001  orders-db  production  us-east     Waiting for cutover  8 minutes ago  https://github.com/acme/shop/pull/412

Use 'schemabot status <apply_id>' to view details

After:

  APPLY ID              EXTERNAL APPLY ID         EXTERNAL OP ID         DATABASE   ENV         STATE                STARTED        SOURCE
  apply-multi-a1b2c3d4  remote-apply-us-east-001  remote-op-us-east-001  orders-db  production  Waiting for cutover  8 minutes ago  https://github.com/acme/shop/pull/412

Use 'schemabot status <apply_id>' to view details

Operations folded into one shared data-plane apply, so there is no per-operation id

Before, the per-operation column is a dash:

  APPLY ID                EXTERNAL OP ID  DATABASE      ENV         DEPLOYMENT  STATE    STARTED        SOURCE
  apply-sharded-d5e6f7g8  -               inventory-db  production  us-east     Running  4 minutes ago  https://github.com/acme/shop/pull/412

Use 'schemabot status <apply_id>' to view details

After, it is left out and the shared handle is named:

  APPLY ID                EXTERNAL APPLY ID         DATABASE      ENV         STATE    STARTED        SOURCE
  apply-sharded-d5e6f7g8  remote-apply-us-east-002  inventory-db  production  Running  4 minutes ago  https://github.com/acme/shop/pull/412

Use 'schemabot status <apply_id>' to view details

No data plane behind this deployment, or nothing dispatched yet

Before:

  APPLY ID                EXTERNAL OP ID  DATABASE     ENV         DEPLOYMENT  STATE    STARTED       SOURCE
  apply-pending-l3m4n5o6  -               payments-db  production  us-east     Pending  1 minute ago  https://github.com/acme/shop/pull/412

Use 'schemabot status <apply_id>' to view details

After, no remote columns at all:

  APPLY ID                DATABASE     ENV         STATE    STARTED       SOURCE
  apply-pending-l3m4n5o6  payments-db  production  Pending  1 minute ago  https://github.com/acme/shop/pull/412

Use 'schemabot status <apply_id>' to view details

How it moves us toward the northstar

One data-plane apply per deployment; operations dispatch into it.

0  per-shard reconcile                          ✅ shipped
pre vschema-only deployment-scoped shape        ✅ shipped
pre op-lease drives settle projection-safely    ✅ shipped
1  dispatch granularity
   ├─ data-plane sibling attach                 ✅ merged
   └─ deployment-keyed idempotency + echo       ✅ merged
2  correlation persistence                      🔍 in review
3  op-scoped data-plane drive (shared apply)    verified in step 6
4  read model exposes the deployment apply id   🔍 in review
5  CLI surfaces it in deployment mode           ⬅ this PR
6  test matrix

Opened by Claude (Fable 5).

[Copilot]

Pull request overview

This PR updates the CLI’s deployment-filtered status list to render the data-plane apply identifier in the APPLY ID column (falling back to the control-plane apply id when no data-plane id is recorded), and adds/adjusts tests to validate the new “one deployment, one data-plane apply” behavior across folded and per-operation rows.

Changes:

• In deployment-filtered status output, APPLY ID now renders the data-plane apply id when available (via statusApplyID), while EXTERNAL OP ID continues to show per-operation remote ids.
• Tightens GRPC client test scaffolding to scope tasks by operation and adds a sibling-shard test asserting shared remote apply id + shared idempotency key behavior.
• Adds tests for the status endpoint fold behavior and for CLI rendering/fallback behavior in deployment mode.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
pkg/tern/grpc_client_test.go	Improves task scoping in mocks and adds a sibling-shard dispatch test to ensure a single shared remote apply id is recorded across operations.
pkg/cmd/internal/templates/progress.go	Changes status list rendering to use a new statusApplyID helper so deployment-filtered lists show the data-plane apply id in the APPLY ID column.
pkg/cmd/internal/templates/progress_states_test.go	Updates/extends rendering tests to assert deployment mode shows the data-plane apply id and falls back to control-plane id when missing.
pkg/api/handlers_test.go	Adds a status endpoint test verifying folded deployment rows surface the shared data-plane apply id and omit per-operation ids.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Kiran01bm]

🤖 Review findings - created by Kiran's code review agent - for schemabot/pull/1062, b767688.

Verdict: 6 findings — 0 blocking, 2 non-blocking (an undocumented rendering change outside deployment mode, and a read-model clobber that can hide the new column), 4 suggestions.

Non-blocking

1. --external-id outside deployment mode now renders a table identical to omitting the flag when no apply on the page has a remote id. pkg/cmd/internal/templates/progress.go:1154

statusColumn{header: "EXTERNAL ID", optional: true, value: unfilteredStatusExternalID},

Pre-PR the unfiltered column was unconditional (case data.ShowExternalID: printed the header for every page) and a row with no remote id rendered -, which positively told the operator "nothing recorded". Marking it optional routes it through retainPopulatedStatusColumns, which drops it entirely when anyStatusRowFillsColumn is false.

Failure scenario: a Spirit-only fleet runs schemabot status --external-id --limit 2. Every row has ExternalID == "" and ExternalOperationID == "", so the header goes from APPLY ID EXTERNAL ID DATABASE ENV STATE STARTED SOURCE to APPLY ID DATABASE ENV STATE STARTED SOURCE — byte-indistinguishable from a run that forgot the flag. The operator cannot tell "no remote ids recorded" from "the flag did nothing / this server doesn't support it".

This contradicts the PR body's "Nothing changes outside deployment mode", and nothing pins either the old or the new behaviour: the only two tests that exercise this path both seed at least one populated id — integration/status_cli_test.go:140-144 (ExternalID: "remote-apply-123") and progress_states_test.go:180. Either keep the unfiltered column unconditional (matching the description), or keep the drop and add a test plus a description correction.

2. In deployment mode the new EXTERNAL APPLY ID column silently disappears for a single-operation deployment whose remote apply id lives on applies.external_id. pkg/api/progress_handlers.go:995

active.ExternalID = op.ExternalID

This unconditionally overwrites the value set from the parent row eight lines earlier (ExternalID: apply.ExternalID, line 977). On the control-plane side, persistRemoteApplyID writes the remote id to the parent when the drive is not operation-scoped:

if !scope.usesOperationRemoteResume() {
	apply.ExternalID = remoteID
	return nil
}

and usesOperationRemoteResume is (multiOperation || operationLeaseOnly) && operation != nil, with multiOperation: len(ops) > 1 (line 1535). So an apply with exactly one operation row, driven while holding an apply lease, records the remote id on applies.external_id and leaves op.ExternalID empty. Under schemabot status --deployment X --external-id, statusOperationForDeployment returns that single op, line 995 clobbers the populated parent id with "", and the brand-new column is then dropped by retainPopulatedStatusColumns — the operator sees no remote handle at all, which is exactly the detour the PR sets out to remove.

Marked non-blocking and PLAUSIBLE rather than confirmed: the clobber is pre-existing (introduced in #566, not touched here), it was invisible before this PR because deployment mode hard-coded -, and reachability depends on the lease shape of a single-operation deployment drive. Worth a one-line fallback (if op.ExternalID != "" { active.ExternalID = op.ExternalID }) or an explicit test that pins the intended behaviour — note the adjacent handlers_test.go:4382 currently asserts assert.Empty(t, resp.Applies[0].ExternalID) for the fold path with a populated parent id, so the suppression looks deliberate for folds but is untested for the single-match path.

General suggestions

3. The dash placeholder was generalised from the external-id column to every column, a second unadvertised rendering change outside deployment mode. pkg/cmd/internal/templates/progress.go:1198

func statusColumnValue(column statusColumn, a ActiveApplyData) string {
	if value := column.value(a); value != "" {
		return value
	}
	return "-"
}

On an unfiltered page where some applies carry a deployment and some do not, the retained DEPLOYMENT column now renders - for the deployment-less rows where pre-PR it rendered blank padding (maxDeployment, a.Deployment with %-*s). Harmless and arguably an improvement — width is unaffected because the DEPLOYMENT header is already 10 chars — but nothing pins it and the description says nothing changes outside deployment mode. SOURCE is not affected in practice: activeApplyResponseFromStorage defaults caller to "cli", so the cell is never empty from a real server.

4. The pkg/tern hunk is test-only scope creep in a PR whose stated scope is one CLI table's columns. pkg/tern/grpc_client_test.go:923

if task.ApplyOperationID != nil && *task.ApplyOperationID == applyOperationID {

mockTaskStore.GetByApplyOperationID previously returned all fixture tasks regardless of the requested operation; scoping it re-bases the task set for every test in the package that drives ResumeApplyOperation. It is safe (all 34 CI checks are green on b767688f, and a silently task-less resume would break assert.Equal(t, []string{"-80"}, req.TargetShards) at line 1440), but nothing in the CLI column change requires it — it exists only to support TestGRPCClient_SiblingShardOperationsRecordOneRemoteApply, which tests deployment-keyed idempotency and generation manifests, i.e. step 3 of the PR's own roadmap rather than step 5. Splitting it out would keep this PR's blast radius to the CLI package, per the small-blast-radius preference.

5. The new API test is a near-verbatim copy of the adjacent fold test. pkg/api/handlers_test.go:4394

TestHandleStatusDeploymentFoldSurfacesSharedDataPlaneApplyID duplicates TestHandleStatusDeploymentFilterSummarizesMatchingOperations except for two ExternalID: "remote-apply-shared" fields and one assertion flipped from assert.Empty to assert.Equal. They are the with/without arms of one table test over a shared fixture builder; as copies, the next fold-shape change has to be applied in both. AGENTS.md:204: "In tests, prefer small composable helpers over copy-pasting setup boilerplate."

6. WriteDatabaseHistory still hand-rolls the width-scan + fixed printf table this PR just replaced one function away. pkg/cmd/internal/templates/progress.go:1318

	maxID := 8      // "APPLY ID"
	maxEnv := 3     // "ENV"
	maxState := 5   // "STATE"

The PR's stated rationale is that "the four-way printf switch had no room left for another shape"; WriteDatabaseHistory repeats the identical pattern and will need the same rewrite the first time it grows an optional column. statusCell / statusListColumnWidths are already generic over (value, width, last) — only the func(ActiveApplyData) string value signature ties them to the status list. Worth a follow-up, not this PR.

The one thing that could have broken, verified

The wholesale replacement of the four-way printf switch in WriteStatusList (progress.go:1048-1071) with a data-driven column loop: 116 deleted lines of hand-written format strings, where any drift in column order, padding or the deleted statusExternalID / statusExternalIDHeader / statusListShowsDeployment helpers would silently corrupt every operator-facing status table. I verified it four ways rather than by reading alone.

• Order and padding cannot desynchronise. statusListColumns returns the already-filtered slice and statusListColumnWidths(columns, ...) is computed over that same slice, so widths[i] and columns[i] index the same column by construction; widths are computed through statusColumnValue, the same function the row renderer uses, so the - placeholder is counted and a dash can never overflow its column.
• Every deleted branch of statusListShowsDeployment is re-established. The old helper returned true iff data.Deployment != "" or any row had a deployment. The first branch is deliberately inverted (the column is now dropped in deployment mode); the second is exactly optional: true + anyStatusRowFillsColumn at progress.go:1164.
• Dropping DEPLOYMENT under a deployment filter loses no information. The SQL selects applies via (deployment = ? OR EXISTS (... ao.deployment = ?)) (applies.go:1279-1284), and activeApplyResponseFromStorage stamps active.Deployment with either op.Deployment (which statusOperationForDeployment matched to the filter) or the filter itself when no operation matches — so every row on the page provably carries the filter value.
• The - fallback in statusFailureActor is byte-identical to the deleted call. The old statusExternalID(StatusListData{}, a) had an empty Deployment, so it fell through to "ExternalOperationID, else ExternalID, else -" — reproduced exactly by unfilteredStatusExternalID(a) plus the explicit if externalID == "" { externalID = "-" } at progress.go:1240. --failed --external-id output is unchanged.

The one difference the refactor introduces that is not a faithful port is finding 1.

Verified correct

• pkg/cmd/internal/templates/progress.go:1170 — SOURCE is the only last: true column and is never optional, so retainPopulatedStatusColumns cannot strip the one column that skips padding. This matters because applySource can return an OSC-8 hyperlink whose len() far exceeds its display width; as the last column it is never padded, so alignment is safe (same as pre-PR).
• pkg/cmd/internal/templates/progress.go:1062-1067 — moving the STATE colour wrap to after padding is visually inert. The colour span now covers the two-space separator (\x1b[36mRunning \x1b[0m vs \x1b[36mRunning\x1b[0m ), but every code stateColorFunc selects is a foreground SGR (ANSICyan = "\033[36m" etc., progress_render.go:11-19), so the extra covered spaces render identically, and ANSI codes are zero-width so alignment is unaffected.
• pkg/cmd/internal/templates/progress.go:1149 — labelling a.ExternalID as EXTERNAL APPLY ID in deployment mode is truthful even when the API falls back to the parent apply row: apply.ExternalID is written only by persistRemoteApplyID, i.e. only the remote data-plane path, so a locally driven apply never populates it.
• pkg/api/progress_handlers.go:940 — the CLI's assumption that a folded row carries a shared ExternalID and no ExternalOperationID matches the server: the fold sets summary.ExternalID from storage.DeploymentExternalID(matches, deployment) and never copies a per-operation id into the summary. The new handler test at handlers_test.go:4394 pins that pairing.
• Removed helpers are fully swept: git grep for statusExternalID, statusExternalIDHeader and statusListShowsDeployment across *.go and *.md returns zero hits.
• pkg/cmd/commands/status.go:92-93 — the only production caller of WriteStatusList sets Deployment: cmd.Deployment, the same value it sends the server as the API filter, so the client-side "deployment mode" switch can never disagree with the server-side fold that produced the rows.
• Column naming matches the detail views it claims to follow: progress_multi.go:130-133 prints "External operation ID" and "External apply ID", the same two handles the new columns name.
• TEMPLATES.md:6792-6832 — the regenerated snapshot matches the three WriteStatusList calls in preview_status.go:85-149, and the unfiltered previewStatusListOutput snapshot is unchanged (no apply there has a deployment or an external id, so the optional-column rule reproduces the pre-PR output).
• All 34 CI checks pass on b767688f (Unit, Integration ×3, E2E ×12, LocalScale ×3, Lint ×4, Build), which is meaningful evidence for the pkg/tern mock-semantics change in finding 4.

This review was generated by Claude Code (claude-opus-5).

[aparajon]

🤖 Addressed the findings in two new commits — ab32cd22 (CLI) and 530be273 (API):

1. Fixed in ab32cd2. The unfiltered EXTERNAL ID column is unconditional again — the operator asked for it by flag, so an all-dash column positively answers "nothing recorded" where a dropped one is indistinguishable from the flag doing nothing. TestWriteStatusListExternalIDColumnRendersWithoutValues pins the header and the - cell with zero populated ids; mutation-checked (re-adding optional: true fails it). Deployment-mode columns keep following the data. The PR body's "nothing changes outside deployment mode" claim is corrected.

2. Fixed in 530be27. Adopted the one-line guard: active.ExternalID = op.ExternalID now only overwrites the parent row's value when the operation recorded one. The fallback is truthful because applies.external_id is written only by the remote data-plane path (persistRemoteApplyID when the drive is not operation-scoped), so it can never surface a local drive as remote. The fold suppression you flagged as looking deliberate was in fact the same clobber — the old assert.Empty fixture had a populated parent id and empty op ids, so the assertion pinned the bug; it now expects the parent id. Mutation-checked (removing the guard fails both fallback arms).

3. Pinned in ab32cd2. TestWriteStatusListMixedDeploymentRowsShowDash covers the generalized placeholder: a mixed unfiltered page renders - in DEPLOYMENT for deployment-less rows. Documented via the body correction in point 1.

4. Declined for this PR. The mockTaskStore scoping and TestGRPCClient_SiblingShardOperationsRecordOneRemoteApply are interlocked with stacked test(tern): pin the one-data-plane-apply-per-deployment invariants #1065, which amends that same test — relocating the hunks would rewrite an approved base branch and force re-review of both PRs for zero behavior change. Your CI observation (all 34 checks green on b767688, and a task-less resume would break the TargetShards assertion) is the safety evidence; leaving it in place.

5. Fixed in 530be27. The two near-verbatim tests are now one table test, TestHandleStatusDeploymentFilterRemoteHandles, over shared fixture builders with four arms: fold with shared op ids, fold falling back to the parent id, single match falling back to the parent id, and single match whose own ids win. The last two double as the pins for point 2.

6. Agreed — follow-up queued. Generalizing the statusColumn renderer so WriteDatabaseHistory routes through it is spun off as its own task rather than widening this PR.

Verification: go test -race ./pkg/api/... ./pkg/cmd/... green, TEMPLATES.md regenerated byte-identical (unfiltered previews don't set ShowExternalID), both behavior fixes mutation-checked as noted.

This reply was generated by Claude Code (Claude Fable 5).