Repository navigation
feat: let local agents delegate tasks to Moca - #455
moonlight16 wants to merge 5 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
moonlight16
changed the title
Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
9157028 to
27a2f96
Compare
pdettori
left a comment
pdettori
left a comment
There was a problem hiding this comment.
Summary
This adds context delegation from local agents. The context path itself is careful: traversal checks, SHA-256 revision validation, a read-only mount at both the PVC and the mount level, and hidden claim names. Thanks for fixing DCO, lint and CodeQL in the latest push; CI is green.
P6: the PR is built entirely on deploy/knative, which RA1 is deprecating. On deploy/k8s, only prompt-only /runs works; workloads, upload and activate don't (see the comment on workload-runtime.ts). Per #378 item 5 this needs a P6-native path, or at least a Knative-only flag plus a follow-up issue. It also overlaps with #381, which plans mocactl subagent commands, so the client may fit better there than as a separate Python script.
Native workload path: it is on by default, with no size, StorageClass or quota limits, broad PVC-delete RBAC, and re-creating a live name leaks resources. Please add tests for re-create, quota/size validation, and the activate-vs-delete race along with the fixes.
Areas reviewed: TypeScript, K8s manifests/RBAC, Shell, Python, Docs, P6-on-kube compatibility
Commits: 5, all signed-off
CI status: passing
Assisted-By: Claude Code
| }; | ||
| } | ||
|
|
||
| export async function createWorkloadRuntime( |
There was a problem hiding this comment.
must-fix — doesn't work on P6-on-kube. This path runs kubectl apply for its own sleep infinity Sandbox resources (the Knative pod-exec model). On deploy/k8s the supervisor runs this same server.ts handler, but its ServiceAccount has no RBAC and automountServiceAccountToken: false (deploy/k8s/base/supervisor.yaml), so every POST /workloads returns 502. On main that route returned 501 without Context Service, so this is a regression on P6. Even with cluster access, these pods never register with sandbox-relay, and records discovery (harness/src/select-sandbox.ts) ignores sandboxPoolSelector, so runs land on a shared remote-worker with no /workspace mount.
#378 item 5 says the Knative/KEDA path isn't extended (RA1) and new work must run on deploy/k8s. Please either deliver context through the P6 sandbox model (a sandbox tier with a context volume in deploy/k8s/base/sandbox.yaml, selected via SH_SANDBOX_TIERS from #425, or upload through relay SandboxExec), or put this behind an off-by-default flag, label it Knative-only, and file a follow-up for P6.
| res.writeHead(409, JSON_HEADERS).end(JSON.stringify({ error: 'workload_name_taken' })); | ||
| return; | ||
| } | ||
| if (!usesContext) { |
There was a problem hiding this comment.
must-fix. Before this PR, POST /workloads returned 501 unless CONTEXT_SERVICE_URL was set. Native provisioning is now on in every deployment, and the harness ServiceAccount gains Sandbox create/delete and PVC delete. Please gate native provisioning and the new /uploads and /activate routes behind a feature flag that is off by default.
| @@ -547,14 +598,88 @@ async function handleCreateWorkload(req: IncomingMessage, res: ServerResponse): | |||
| res.writeHead(409, JSON_HEADERS).end(JSON.stringify({ error: 'workload_name_taken' })); | |||
There was a problem hiding this comment.
must-fix. The 409 check only refuses a live workload owned by another subject. If the same subject re-POSTs a live name, it re-applies over the existing Sandboxes, and saveWorkload overwrites the record. That drops contextName/contextId, so the Context leaks, and a smaller sandboxes count orphans the extras. With auth off, every anonymous caller counts as the same owner, so this works across clients too. Refuse any existing record whose status isn't deleted, or make re-create a true no-op.
| replicas, | ||
| workspace: { | ||
| kind: 'native', | ||
| size: spec.workspace?.size ?? '1Gi', |
There was a problem hiding this comment.
must-fix. workspace.size and storageClass go into volumeClaimTemplates without validation. A caller can request up to 100 Sandboxes per workload, with no limit on workloads, each with a PVC of any size on any StorageClass. Please add a size ceiling, a StorageClass allowlist, and a per-subject quota.
| verbs: ['get', 'list'] | ||
| verbs: ['create', 'get', 'list', 'patch', 'update', 'delete'] | ||
| - apiGroups: [''] | ||
| resources: ['persistentvolumeclaims'] |
There was a problem hiding this comment.
suggestion. persistentvolumeclaims: delete covers every PVC in the namespace, including the leaf-work and Redis volumes. Narrow it (dedicated namespace or label guard), or at least document the blast radius.
| } | ||
| const publicBase = new URL(`${publicBaseUrl()}/`); | ||
| if ( | ||
| !['http:', 'https:'].includes(publicBase.protocol) || |
There was a problem hiding this comment.
suggestion. CONTEXT_SERVICE_PUBLIC_URL may be plain http:, which sends the upload bearer token in cleartext. Require https: except for loopback.
| }) | ||
| try: | ||
| workload = create_workload( | ||
| url, workload_id, args.sandboxes, False, args.timeout, args.poll_interval, |
There was a problem hiding this comment.
suggestion. run always sends shared=False, so --sandboxes > 1 always gets 400 shared_workspace_required. Pass shared=args.sandboxes > 1.
|
|
||
|
|
||
| def export_context(context: str, bundle: Path) -> dict[str, Any]: | ||
| result = contextctl("ctx", "export", context, "--output", str(bundle)) |
There was a problem hiding this comment.
suggestion. The whole export is uploaded with no client-side check. A cheap denylist on the manifest before the PUT (.env, *.pem, id_rsa*, .git/config) would catch accidental secret uploads.
| kubectl --context "$KUBE_CONTEXT" apply -f - | ||
| kubectl --context "$KUBE_CONTEXT" rollout restart deployment/context-service -n default | ||
| else | ||
| curl -fsSL "https://raw.githubusercontent.com/rossoctl/context-service/${CONTEXT_SERVICE_REF}/deploy/context-service.yaml" | |
There was a problem hiding this comment.
suggestion. context-service.yaml is applied from the mutable main ref. Pin CONTEXT_SERVICE_REF to a tag or SHA.
| account_args=(--account "$MOCA_OP_ACCOUNT") | ||
| fi | ||
| local credential | ||
| credential="$(op read "${account_args[@]}" "$MOCA_ANTHROPIC_OP_REF")" || die "could not read the Anthropic credential from 1Password" |
There was a problem hiding this comment.
nit. Under set -u, "${account_args[@]}" on an empty array fails on macOS's bash 3.2 when MOCA_OP_ACCOUNT is unset. Use ${account_args[@]+"${account_args[@]}"}.
Moca's /workloads asked Context Service to create sandbox pools (/v1/sandbox-pools). Moca will own its workloads and sandboxes, and Context Service will only store context (rossoctl/context-service#46). Remove Moca's use of the sandbox-pool API. Until Moca provisions workloads itself, the /workloads routes, and any /runs request that names a workloadId, answer 501 workloads_unavailable. Runs without a workload are unchanged. Split out of #455. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
…#473) Resolve server.ts in main's favour for the workload lifecycle — every /workloads route and any run naming a workloadId now answers 501 workloads_unavailable (rossoctl#455) — keeping this branch's error-code changes on top. The spec follows: the /v1/runs 404 loses workload_not_found, the 501 gains workloads_unavailable, and the sandboxPoolSelector note drops the workload-resolver clause. routes.ts workload summaries note the 501. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
Context Service created Moca's execution environments through /v1/sandbox-pools: Sandboxes, sandbox profiles, and WarmPool claims. Moca now owns its sandboxes, and rossoctl/moca#469 removed its last call to this API. Remove the sandbox-pool API, the Sandbox, profile and WarmPool code, contextctl sandbox-pool, CS_SANDBOX_IMAGE, and the matching RBAC. Context storage is unchanged. Closes #46. Part of the work split out of rossoctl/moca#455. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
Moca creates a Context, gives the client a one-time upload capability, freezes the uploaded revision, and downloads that frozen revision as a portable bundle. Context Service stores the bytes and never creates sandboxes. Ported from #50 without the PVC attachment endpoint, which bundle delivery does not need. Adds the frozen-bundle export endpoint, aborts a failed export instead of returning a truncated 200, and starts without a helper image (upload and export are then unavailable). Part of the work split out of rossoctl/moca#455. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
Workloads created through the old /workloads routes are no longer released by Moca or Context Service. Document how to delete their Sandboxes, PVCs, and sh:workload:* Redis keys, and point the docs at #476 instead of #455. Follow-up to #469. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
A workload belongs to the user who created it and owns one Context in Context Service. The user creates the workload, uploads a context bundle directly to Context Service with a one-time grant, and activates it to freeze that revision. Deleting the workload deletes the Context. Off by default (MOCA_CONTEXT_WORKLOADS_ENABLED=1 and CONTEXT_SERVICE_URL). Every route requires the user's control-plane API token. Names are scoped per user (first half of rossoctl#358). Per-user quota, a storage size ceiling, a StorageClass allowlist, and atomic lifecycle transitions. Runs that name a workload still answer 501 until context delivery lands. Part of the work split out of rossoctl#455. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
A synchronous /runs request can name a workloadId. Moca checks that the session token's user owns the ready workload, downloads its frozen revision from Context Service once, and copies it into the shared sandbox through SandboxExec as raw bytes. The sandbox verifies the sha256 and publishes it read-only; it is removed after the run by default, and each user's copy is cached separately. Shared, running sandboxes cannot mount the workload's PVC, so phase 1 copies; phase 2 (#476) mounts it instead. Part of the work split out of #455. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
A living list of the ways we expect people to move context into and out of Moca: what sandbox and storage each workflow uses, and where it stands. Centralizes the workflows table that lived in the delegate skill in #455. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
3 participants
What this enables
A local agent can delegate one task or fan out many tasks to Moca. The agent selects the relevant local context files, and Context Service transports them into a shared PVC in Moca. Moca then runs the delegated tasks in isolated Sandboxes with that context mounted read-only and returns the results to the local agent.
This supports workflows such as continuing work after closing a laptop, using cloud resources unavailable locally, and parallelizing many bounded subtasks.
How it works
moca-delegate-taskskill for local agent harnessesContext Service counterpart: rossoctl/context-service#50
Related to #378.
Validation