Transport Paperclip run context in shipped onecli client - #1
Transport Paperclip run context in shipped onecli client#1misterbusiness1 wants to merge 4 commits into
Conversation
Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
Exact-head review of ab9110d: CHANGES REQUESTED. P0: this transports the Paperclip binding but still authenticates onecli run with the shared management credential. RunCmd.Run calls newClient(), and newClient unconditionally loads the API key from the shared credential store before GetContainerConfig. The new headers are additive; they do not remove management authority or make the raw store unreachable. Removing the current shared credential mount would therefore either break this path or leave the precise cross-agent capability exposure open. Required source fix:
This PR is a useful transport start, but it does not yet meet the shared-credential unreachability acceptance and must not merge/deploy independently. |
Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
Follow-up exact-head review of 38114ef: The management-key defect is fixed in this head, and I independently ran the full suite in golang:1.26: all packages passed. One route-scope defect remains: NewPaperclipRun leaves prefix unresolved, so resolvePrefix sends the full run capability headers to /v1/health before the guarded /v1/container-config call. The test explicitly expects both requests to carry the binding. A route-scoped bearer should go only to the intended run endpoint, not health/logging middleware. Pin the Paperclip run client to /v1 (compatible deployment order already requires the new server first), or probe health without default headers, and change the actual RunCmd test to require exactly one capability-bearing request. Also keep this PR atomic with the Paperclip broker/OneCLI server changes: the header remains an env bearer until that boundary is implemented, so this head is not independently deployable. |
Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
Route-scope follow-up pushed at
No merge or deploy performed. |
|
Exact-head review of The health-probe leak is fixed in the implementation: Update |
There was a problem hiding this comment.
Atomic CQE verdict for exact head 930a059: CHANGES REQUESTED.
Blocking — cmd/onecli/run_test.go:50 violates the zero-new-test-failures gate. Independent immutable golang:1.26 go test ./... reports requests = 1, want health probe plus container config. The implementation correctly makes one capability-bearing request, but the regression still asserts the superseded health-probe behavior. Update the test to require exactly one /v1/container-config request, retain the no-management-Authorization assertion, and rerun go test ./... on the new exact head.
This PR is one of three shipped surfaces and cannot be approved independently. Paperclip PR onecli#38 and OneCLI PR onecli#3 must be reviewed atomically; OneCLI PR onecli#3 also retains the P0 requirements for no static agent.accessToken, active-run cancellation/revocation, DB-derived project/org relation, maximum five-minute capability, and sibling-process/broker isolation. Do not merge this red head.
|
Operator-assisted exact-head fix pushed at e04e4a0. The actual-client regression now requires exactly one /v1/container-config request and still proves all four Paperclip binding headers are present with no management Authorization header. Independent immutable golang:1.26 go test ./... passed every package; git diff --check passed. No production code, credentials, grants, merge, or deployment changed. This removes the stale test blocker only; the atomic server-owned run credential/revocation and tenant-isolation findings on the paired PRs remain merge-blocking. |
There was a problem hiding this comment.
COMMENT — exact head e04e4a0addc1ee0862c33cfbe0e3b4c1ddf869f1, reviewed as one surface of the atomic Paperclip/OneCLI/CLI gate.
The one-file delta fixes the prior blocker in cmd/onecli/run_test.go: it now requires exactly one /v1/container-config request. Independent immutable golang:1.26 go test ./... and diff-check evidence is green. No new CLI-local blocking finding remains.
This is not an atomic approval: Paperclip PR onecli#38 at e76268faec817933d759dd196c938f0ecb5413e9 and OneCLI PR onecli#3 at 15357453eed0d431adf434bcb5ae6156099c81d1 still expose persistent agent.accessToken authority after configuration lookup, without per-use active-run enforcement or cancellation/completion revocation. Hosted evidence is also incomplete. Keep all three surfaces unmerged until the paired server P0 is fixed and the exact heads are re-reviewed.
Summary
aa226247ce49cf6a22339cdf43ed160ac8d17b33.GET /v1/container-config.RunCmddry-run request test.Verification
git diff --checkpassed.go/gofmt; CI is required and must be treated as authoritative.Rollout / rollback
No merge or deploy. No credentials, grants, providers, or scopes changed.