ntf-client: error-returning Manager core, shaping and captures

[ripienaar]

Use orbit.go/ntf/api types; TB methods wrap the core unchanged.

[synadia-claude-reviewer]

Claude finished @ripienaar's task in 3m 59s —— View job

---

Review complete

• Gather context (PR diff, metadata)
• Read changed files (manager.go, captures.go, shaping.go, client.go, errors.go)
• Review for correctness, concurrency, security, error-handling, API compatibility
• Cross-check against ntf server handlers (ntf/handlers.go) for error-code semantics
• Post summary review

Note: go build/go test/staticcheck could not be run in this sandbox (command execution needs approval that wasn't available here). The review below is from static reading of the diff plus cross-referencing the ntf server handlers.

Summary

No correctness, security, or concurrency bugs found. Specifically checked:

• Context/timeout handling (withDefaultTimeout, Connect, Manager.Captures) — consistent, every context.WithTimeout has a matching deferred cancel, and the "use ctx's deadline if set, else a default" pattern is applied uniformly.
• The 404 → ErrInstanceNotFound mapping in Manager.request (manager.go) — cross-checked against ntf/handlers.go: "404" is used there exclusively for instance-not-found / instance-destroyed-during-start responses, while unknown-server-name errors use a different code ("001"). So this mapping won't mislabel a bad server name as a missing instance.
• Captures' polling loop (captures.go) — correctly handles want <= 0 (returns immediately without waiting), non-transient list errors (returned without retrying forever), and context expiry (ErrCaptureWait wrapping ctx.Err()); matches the accompanying tests.
• Manager/Client/Instance delegation — straightforward wrapping; nats.Conn is safe for concurrent use and Manager holds no other mutable state, so sharing a Manager/Client across goroutines is safe.

Two things worth calling out (not bugs, but worth confirming intentional):

1. Breaking import path. ntf-client/api is deleted; everything now imports github.com/synadia-io/orbit.go/ntf/api instead. Any existing consumer importing the old ntf-client/api package will fail to build. Given this repo's independent-per-module versioning convention, that implies a major version bump when ntf-client is next tagged.
2. go.mod pins an untagged pseudo-version of ntf (v0.0.2-0.20260922120557-cb4a1fd0e756) rather than a release tag — fine for in-flight co-development, but worth swapping for a tagged version before this is released.

Minor/non-blocking: Client (client.go) still carries its own nc/address fields even though every method now delegates through c.m *Manager, which duplicates state nothing else reads anymore.

[ripienaar]

this seems fine to me, especially while these things are like actively developed

[piotrpio]

LGTM!