Migrate Claude harness from builtin to container-script provisioning#279
Migrate Claude harness from builtin to container-script provisioning#279ptone wants to merge 3 commits into
Conversation
Move the Claude harness from the compiled-in builtin provisioner to the extensible container-script model, matching what OpenCode and Codex already use. This is the first concrete step in retiring the builtin provisioner path (refs #100). The container-side provision.py handles: - Auth resolution with Claude's 4-way precedence (API key → OAuth token → auth-file → Vertex AI) - API key pre-approval fingerprint in .claude.json - Project workspace path setup in .claude.json - MCP server translation via scion_harness helper - Auth env var overlay output for the runtime The compiled ClaudeCode harness is kept intact as a fallback for existing installations that haven't upgraded their harness-config. Includes parity tests and Python integration tests that verify the script produces identical outputs to the compiled harness.
There was a problem hiding this comment.
Code Review
This pull request transitions the Claude Code harness provisioner to a container-script model, introducing a container-side Python script (provision.py) to handle configuration, MCP server translation, and authentication resolution. It also adds extensive parity and integration tests in Go. The review feedback highlights a critical issue in the Python script's auto-detection logic for Vertex AI, where requiring a local Application Default Credentials (ADC) file prevents successful authentication in GCP environments that use metadata-server-based service accounts.
| return "oauth-token", "CLAUDE_CODE_OAUTH_TOKEN" | ||
| if has_authfile: | ||
| return "auth-file", "" | ||
| if has_adc and has_gcp_project and has_gcp_region: |
There was a problem hiding this comment.
In GCP environments (such as GKE, Cloud Run, or Compute Engine), containers often authenticate using the VM or Pod's attached service account via the GCP metadata server (assign mode). In these cases, no local Application Default Credentials (ADC) file is mounted, meaning has_adc will be False.
Since has_adc is required in the current auto-detect block, auto-detection of vertex-ai will fail even if GOOGLE_CLOUD_PROJECT and GOOGLE_CLOUD_REGION are correctly set, raising an unexpected ValueError and blocking provisioning.
Removing the has_adc requirement from the auto-detect fallback allows the GCP SDK/Vertex AI client to seamlessly fall back to the metadata server as expected.
| if has_adc and has_gcp_project and has_gcp_region: | |
| if has_gcp_project and has_gcp_region: |
Address review feedback: remove the has_adc requirement from the Vertex AI auto-detect path in provision.py. In GCP environments (GKE, Cloud Run, Compute Engine), containers authenticate via the metadata server using attached service accounts — no local ADC file is mounted. The GCP SDK handles this fallback natively, so requiring project + region is sufficient for auto-detection. Add TestClaudeProvisionScript_Integration_VertexAI_NoADC to verify the fix works for GCP service account environments.
Summary
provision.pyfor Claude that handles auth resolution, API key pre-approval, project path setup, and MCP server translationRefs #100
What changed
pkg/harness/claude/embeds/config.yaml: Changedprovisioner.typefrombuiltintocontainer-scriptand added MCP capabilities block.pkg/harness/claude/embeds/provision.py(new): Container-side provisioner script handling:.claude.json(mirrorsClaudeCode.ApplyAuthSettings).claude.json(mirrorsClaudeCode.provisionClaudeJSON)scion_harness.apply_mcp_servers_simple()pkg/harness/claude_parity_test.go(new): 11 tests covering:TestClaudeEmbedsSeedRootSupportFiles)TestClaudeActivateScriptIsIdempotent)TestClaudeContainerScriptHarnessParity)What remains (tracked in #100)
provision.py(follow-up PR)newBuiltin()fromresolve.goand the compiled provisioning methodsThe compiled
ClaudeCodeharness is intentionally kept as a fallback for existing installations that haven't upgraded their harness-config.Test plan
go vet ./pkg/harness/...cleango build ./...succeeds