mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-07 16:09:43 +02:00
Merge pull request #357 from dignfei/feat/docker-adopt-existing-container
feat(docker): attach a case to an already-running container Conflicts came from work that landed after the PR was opened, and each is resolved onto the newer abstraction rather than by keeping the older code: - `defaultDockerCommandForMode` is registry-driven since #347, so the PR's `runsAsRoot` arm became `overlays.docker.rootCommand` (claude only). Claude Code still refuses `--dangerously-skip-permissions` as root in 2.1.261 and the refusal is visible only inside the container, so an adopted root container otherwise just shows a dead pane. Which flag to drop is a per-CLI fact, and `test/cli-registry-no-id-branching.test.ts` forbids expressing it as a branch. - The probe's mode list and its mode -> binary table both duplicated the registry. They now read `enabledCliIds()` / `discovery.binaries[0]`, which is also what fixes the merge's silent regression: the hand-written list predates `omp`, and the run menu gates every docker case on this probe, so owned containers would have lost that mode. `shell` needs no arm — it declares no binary, so it is dropped from the lookup and reported available regardless. - The per-mode `mode === 'claude' && !cliDir` chain in `tmux-manager.ts` is one `missingCliMessage(mode)` gate since #347; the PR's docker exemption moved onto it. Its test now pins the single gate instead of counting seven arms. - The create arm keeps #349's swap-limit warning filter, which the adopted arm never reaches; the run-mode list gains `omp` from #353. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TecFD9hvPYJ1mkkMtBQbT1
This commit is contained in:
@@ -18,7 +18,9 @@ import {
|
||||
removeDockerContainer,
|
||||
checkDockerConfigDrift,
|
||||
dockerConfigHash,
|
||||
dockerAdoptProbeModes,
|
||||
} from '../src/docker-hosts.js';
|
||||
import { enabledCliIds, getCli } from '../src/config/cli-registry/index.js';
|
||||
import {
|
||||
buildDockerLaunchCommand,
|
||||
buildDockerStopCommand,
|
||||
@@ -201,15 +203,17 @@ describe('adopted container: claude as root', () => {
|
||||
describe('adopted container: the host is not required to have the CLI', () => {
|
||||
const src = readFileSync(new URL('../src/tmux-manager.ts', import.meta.url), 'utf8');
|
||||
|
||||
it('skips every host CLI requirement for a docker session', () => {
|
||||
it('skips the host CLI requirement for a docker session', () => {
|
||||
// A docker session runs its CLI inside the container. Demanding it on the
|
||||
// host threw, the catch fell back to a direct PTY, and that PTY tried to
|
||||
// exec the CLI on the HOST — surfacing as a bare `execvp(3) failed` with
|
||||
// nothing naming the real cause.
|
||||
const guarded = src.match(/!cliRunsInContainer && mode === '/g) || [];
|
||||
const unguarded = src.match(/\n if \(mode === '[a-z]+' && !cliDir\)/g) || [];
|
||||
expect(guarded.length).toBeGreaterThanOrEqual(7);
|
||||
expect(unguarded).toHaveLength(0);
|
||||
//
|
||||
// The CLI registry collapsed the old per-mode `mode === 'claude' && !cliDir` chain
|
||||
// into ONE `missingCliMessage(mode)` gate, so the guarantee is now that the single
|
||||
// gate carries the docker exemption and that no per-mode arm has grown back.
|
||||
expect(src).toContain('if (!cliRunsInContainer && !cliDir) {');
|
||||
expect(src.match(/if \(mode === '[a-z]+' && !cliDir\)/g)).toBeNull();
|
||||
});
|
||||
|
||||
it('derives the flag from the docker metadata the session already carries', () => {
|
||||
@@ -364,3 +368,23 @@ describe('adopted container: drift is not evaluated', () => {
|
||||
expect(status.drifted).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: probe modes come from the CLI registry', () => {
|
||||
it('probes every enabled CLI, so a newly-enabled one needs no second list', () => {
|
||||
// A hand-written list here silently froze: `omp` shipped in 1.24.0 and was
|
||||
// missing from it, which hid the omp run mode on EVERY docker case — owned
|
||||
// ones included, since the run menu gates on this same probe.
|
||||
const modes = dockerAdoptProbeModes();
|
||||
expect(modes).toEqual(enabledCliIds());
|
||||
expect(modes).toContain('omp');
|
||||
expect(modes).toContain('shell');
|
||||
});
|
||||
|
||||
it('resolves the real binary name, not the mode name', () => {
|
||||
// `antigravity` ships as `agy` and `deepseek` as `dsh`, so a mode-name probe
|
||||
// would report both as missing on a container that has them.
|
||||
expect(getCli('antigravity')?.discovery.binaries[0]).toBe('agy');
|
||||
expect(getCli('deepseek')?.discovery.binaries[0]).toBe('dsh');
|
||||
expect(getCli('shell')?.discovery.binaries[0]).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user