[Feature] Add a full-object Get op to the agent protocol

[HiranAdikari]

What this changes

The agent's only read op was get_status, which returns a status-only partial. But the cluster-access seam's Get is contractually a full-object read (the direct client returns the whole object) — so AgentBacked.Get was quietly mapping a full read onto a status-only one. The upcoming read-modify-patch write ops (per-VPC NAT and DNS, the cloud-provider service account) read .spec/.data to modify it, so they need a real full-object read through the agent. This adds one.

End to end

• dc-agent: Executor.GetObject (dynamic get of one object, returned verbatim — spec + status + metadata) and an opGetObject dispatch handler.
• dc-api: a matching opGetObject wire frame, Session.GetObject, and AgentBacked.Get repointed at it — re-hydrating the response into the same full Unstructured the direct client returns, so provider code is identical across the direct and agent seams.
• No RBAC change: a full read uses the same get permission the agent already holds for get_status; the request reuses the existing ResourceRef shape.

Rolling-deploy safety

If a newer dc-api talks to an older agent that predates get_object, Get falls back to get_status (the prior status-only read) instead of erroring — so a rolling deploy never hard-fails a read. get_status and watch_status (the async status poll/stream) are untouched; a status-only caller (the VM read flow) sees the identical shape on the fallback path.

Why now

This is the second protocol prerequisite of the remaining-ops work (after the list op in the parent PR). With full reads in place, the remaining slices — image, cloud-provider SA, NAT, DNS — are capability onboarding plus a raw-Patch → server-side-apply refactor, no new read primitives.

Verification

Both modules build, vet, and pass race-enabled unit tests. New tests cover the executor get, the opGetObject wire round-trip, the full-object re-hydration, the OP_UNSUPPORTED → get_status fallback, and the existing VM read flow. The live-cluster suites need a real connected agent and weren't run here — one live full read through a connected agent is the remaining check before relying on it.

Stacked on #199

Branches off feat/agent-list-op (#199, the list op). Review/merge that first; this PR's base retargets to controlplane once it lands.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added a new get_object protocol operation to retrieve full Kubernetes objects (metadata, spec, status) as raw JSON.
  • Agent-backed reads now prefer the full-object path, with transparent fallback behavior when full reads aren’t supported.

• Bug Fixes

  • Improved “object not found” handling to consistently return success with Found=false.

• Tests

  • Added and expanded unit and end-to-end coverage for get_object, including found/absent, cluster-scoped refs, and error/unsupported-op scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4da531b4-4f69-4230-bc3a-c72579392f8e

📥 Commits

Reviewing files that changed from the base of the PR and between b735b7c and 5dfda17.

📒 Files selected for processing (2)

• dc-agent/main.go
• dc-api/internal/providers/clusteraccess/agent.go

🚧 Files skipped from review as they are similar to previous changes (2)

• dc-agent/main.go
• dc-api/internal/providers/clusteraccess/agent.go

---

📝 Walkthrough

Walkthrough

This PR adds a new get_object operation that returns full Kubernetes objects as raw JSON, wires it through the agent/API session layers, routes clusteraccess Get through it with status fallback, and updates dispatcher and test stubs to match the expanded interface.

Changes

get_object Operation

Layer / File(s)	Summary
Executor contract and kube read dc-agent/internal/executor/executor.go, dc-agent/internal/executor/kube.go, dc-agent/internal/executor/kube_test.go	Adds the get_object protocol verb, result type, executor method, stub support, and kube executor read path with found/absent/cluster-scoped/unknown-kind tests.
Agent dispatcher wiring dc-agent/main.go, dc-agent/main_test.go	Advertises get_object from the active dispatchers, adds the handler, and extends dispatcher end-to-end tests and request validation.
API session GetObject dc-api/internal/agentgw/agentgw.go, dc-api/internal/api/handlers/agentchannel.go, dc-api/internal/api/handlers/agentchannel_getobject_test.go, dc-api/internal/api/handlers/agentgw_adapter.go	Adds the shared wire result and session method, implements the protocol handler, updates adapter docs, and adds protocol tests for found, absent, and error cases.
clusteraccess Get routing and fallback dc-api/internal/providers/clusteraccess/agent.go, dc-api/internal/providers/clusteraccess/clusteraccess_test.go	Routes Get through GetObject, falls back to GetStatus on unsupported ops, converts missing objects to Kubernetes not found errors, and updates the route/fallback tests.
Provider test stub coverage dc-api/internal/providers/harvester/*_seam_test.go, dc-api/internal/providers/kubeovn/network_seam_test.go, dc-api/internal/providers/registry_test.go	Adds no-op GetObject implementations to provider session stubs so they satisfy the expanded interface.

Estimated code review effort: 3 (Moderate) | ~35 minutes

Poem

A rabbit hopped with JSON bright,
and fetched whole objects in one bite 🐇
When old paths yawned, a fallback sprang,
with status-only echoes that softly sang.
Now get_object scurries through the stack,
and every stub knows how to talk back.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 65.38% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately summarizes the main change: adding a full-object Get operation to the agent protocol.
Description check	✅ Passed	The description covers the problem, changes, rollout safety, and testing, with only the template checklist section missing.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/agent-full-get-op

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[HiranAdikari]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[HiranAdikari]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[HiranAdikari]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

🧹 Nitpick comments (2)

dc-agent/main.go (1)

172-172: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Hand-maintained ops list risks drift.

The logged ops slice duplicates the op set already registered in buildDispatchers' returned map (this PR itself had to manually add OpGetObject here). Since the actual wire advertisement is derived from the dispatcher/streamDispatcher maps elsewhere (per TestBuildDispatchers_RegistersAllOps's comment), consider deriving this log field from dispatcher's keys after buildDispatchers returns, to avoid a future op being registered but omitted from the log (or vice versa).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dc-agent/main.go` at line 172, The logged ops slice in the main startup
logging is manually duplicated and can drift from the actual registered ops.
Update the logging around the buildDispatchers result to derive the ops field
from the returned dispatcher map keys (and any relevant stream dispatcher keys
if needed) instead of hardcoding executor.Op* values, so the log always reflects
the registered operations.

dc-api/internal/providers/clusteraccess/agent.go (1)

147-163: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Avoid computing translateErr(err) twice.

a.translateErr(err) is called once inside errors.Is(...) and again in the fallthrough return. Since it's a pure function, hoist it into a local variable.

♻️ Suggested refactor

 	res, err := a.sess.GetObject(ctx, ref)
 	if err != nil {
-		// An older agent that doesn't implement get_object replies OP_UNSUPPORTED,
-		// which translateErr maps to ErrOpNotRoutable. In that one case fall back to
-		// the status-only read so a rolling deploy never errors. Any other error
-		// (agent unavailable, RBAC, timeout) propagates.
-		if errors.Is(a.translateErr(err), agentgw.ErrOpNotRoutable) {
+		// An older agent that doesn't implement get_object replies OP_UNSUPPORTED,
+		// which translateErr maps to ErrOpNotRoutable. In that one case fall back to
+		// the status-only read so a rolling deploy never errors. Any other error
+		// (agent unavailable, RBAC, timeout) propagates.
+		terr := a.translateErr(err)
+		if errors.Is(terr, agentgw.ErrOpNotRoutable) {
 			a.log.Info().
 				Str("seam", "agent").
 				Str("region", a.region).Str("zone", a.zone).
 				Str("api_version", ref.APIVersion).Str("kind", ref.Kind).
 				Str("namespace", ref.Namespace).Str("name", ref.Name).
 				Msg("agent does not support get_object; falling back to status-only get_status")
 			return a.getStatusFallback(ctx, gvr, ref, ns, name)
 		}
-		return nil, a.translateErr(err)
+		return nil, terr
 	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dc-api/internal/providers/clusteraccess/agent.go` around lines 147 - 163,
Avoid calling translateErr(err) twice in the GetObject error path; compute it
once and reuse the result. In agent.go’s GetObject handling, assign
a.translateErr(err) to a local variable before the errors.Is check, use that
variable for the ErrOpNotRoutable comparison, and return it in the non-fallback
error case so the logic stays the same while avoiding duplicate work.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@dc-agent/main.go`:
- Line 172: The logged ops slice in the main startup logging is manually
duplicated and can drift from the actual registered ops. Update the logging
around the buildDispatchers result to derive the ops field from the returned
dispatcher map keys (and any relevant stream dispatcher keys if needed) instead
of hardcoding executor.Op* values, so the log always reflects the registered
operations.

In `@dc-api/internal/providers/clusteraccess/agent.go`:
- Around line 147-163: Avoid calling translateErr(err) twice in the GetObject
error path; compute it once and reuse the result. In agent.go’s GetObject
handling, assign a.translateErr(err) to a local variable before the errors.Is
check, use that variable for the ErrOpNotRoutable comparison, and return it in
the non-fallback error case so the logic stays the same while avoiding duplicate
work.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6568f1b5-31f9-41fc-85cf-12abd04acb91

📥 Commits

Reviewing files that changed from the base of the PR and between a6452cf and b735b7c.

📒 Files selected for processing (16)

• dc-agent/internal/executor/executor.go
• dc-agent/internal/executor/kube.go
• dc-agent/internal/executor/kube_test.go
• dc-agent/main.go
• dc-agent/main_test.go
• dc-api/internal/agentgw/agentgw.go
• dc-api/internal/api/handlers/agentchannel.go
• dc-api/internal/api/handlers/agentchannel_getobject_test.go
• dc-api/internal/api/handlers/agentgw_adapter.go
• dc-api/internal/providers/clusteraccess/agent.go
• dc-api/internal/providers/clusteraccess/clusteraccess_test.go
• dc-api/internal/providers/harvester/getvm_seam_test.go
• dc-api/internal/providers/harvester/list_seam_test.go
• dc-api/internal/providers/harvester/vmwrite_seam_test.go
• dc-api/internal/providers/kubeovn/network_seam_test.go
• dc-api/internal/providers/registry_test.go