feat(docker): support OMP execution and credentials in isolation - #872
feat(docker): support OMP execution and credentials in isolation#872tomdps wants to merge 2 commits into
Conversation
…tials in isolation
tomdps
changed the title
Greptile SummaryAdds OMP execution support under Docker isolation.
Confidence Score: 4/5The PR is not yet safe to merge because multi-user startup can still fail and failed Docker container creation still leaks OMP provider state. The shared temporary root remains owned and writable only according to the first creator's ordinary permissions, while provider-state directories are created before Files Needing Attention: src/isolation-manager.js
|
| Filename | Overview |
|---|---|
| src/isolation-manager.js | Adds OMP writable-state mounts, credential forwarding controls, image-tag hashing, and cleanup, but both previously reported lifecycle defects remain. |
| src/lib/gc.js | Extends orphan collection to report and remove provider-state directories while protecting known cluster IDs. |
| src/agent-cli-provider/provider-registry.ts | Declares OMP installation, credential environment variables, and writable runtime-state subpaths in the canonical provider registry. |
| src/agent-cli-provider/adapters/omp.ts | Adds the OMP credentials used by provider-contract environment handling and redaction. |
| cli/index.js | Extends garbage-collection output to include orphaned provider-state directories. |
Sequence Diagram
sequenceDiagram
participant O as Orchestrator
participant I as IsolationManager
participant F as Host filesystem
participant D as Docker
O->>I: "createContainer(clusterId, provider=omp)"
I->>F: mount ~/.omp read-only
I->>F: create per-cluster writable state
I->>D: create/start OMP image
D-->>O: isolated OMP container
O->>I: cleanup(clusterId)
I->>D: remove container
I->>F: remove provider-state directory
Reviews (2): Last reviewed commit: "fix(docker): isolate OMP credential envi..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Ancestor path components (the shared `zeroshot-provider-state` root, the per-cluster and | ||
| // per-provider dirs) get ordinary default permissions — they hold no data themselves, only | ||
| // need to stay traversable so a different host user's own clusterId subtree isn't blocked. | ||
| fs.mkdirSync(hostRoot, { recursive: true }); |
There was a problem hiding this comment.
When a second OS user starts an OMP Docker cluster after another user created the shared provider-state root, the ordinary owner-writable directory permissions make fs.mkdirSync(hostRoot, { recursive: true }) fail with EACCES, preventing the container from starting.
Context Used: CLAUDE.md (source)
Knowledge Base Used: Orchestrator: Cluster, Agent-Wrapper, and Message Bus Coordination
| containerHome, | ||
| providerName | ||
| ); | ||
| this._applyProviderStateMounts(args, config, clusterId, containerHome, providerName); |
There was a problem hiding this comment.
When _spawnContainer rejects after this call creates the provider-state directories, startup exits before the cluster records its isolation manager, so the failure cleanup cannot remove those directories and each failed OMP Docker launch leaves an orphan under the temporary state root.
Context Used: CLAUDE.md (source)
Knowledge Base Used: Orchestrator: Cluster, Agent-Wrapper, and Message Bus Coordination
1 participant
Closes #868