Skip to content

Commit 0e655a1

Browse files
authored
fix(inference): ignore terminal state in Docker authority (#9636)
<!-- markdownlint-disable MD041 --> ## Summary Managed llama.cpp authority persisted during interactive onboarding included terminal attachment state, so later `status` and `doctor` commands could calculate a different authority in a detached session. This change excludes `TERM` from Docker authority while preserving the checks that detect genuine endpoint, executable, delegated-command, credential-helper, provider, operation, and binding drift. ## Related Issue Fixes #9599 ## Changes - Add a regression test proving host-local inference authority remains stable when `TERM` is present during onboarding and absent during later diagnostics. - Stop forwarding and hashing `TERM` as part of the qualified Docker command environment. - Preserve fail-closed authority checks for Docker state that can change which daemon or executable NemoClaw uses. The detection gap was a missing terminal-attachment case. Existing tests covered irrelevant `PATH` additions and SSH session metadata changes, but not `TERM`, even though it varied between the interactive wizard and later detached diagnostics. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: The change removes only non-semantic terminal attachment state. Docker endpoint, executable identity, delegated-command set, credential-helper identity, provider, operation, and binding checks remain unchanged, and existing negative tests still cover genuine authority drift. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run --project cli src/lib/onboard/runtime-provider/docker-operation-authority.test.ts src/lib/inference/llama-cpp/managed-status.test.ts src/lib/actions/sandbox/doctor-inference.test.ts src/lib/actions/sandbox/status.test.ts src/lib/onboard/setup-nim-flow-managed-llama-cpp-profile.test.ts` passed 78 tests in 5 files. The new regression failed before the production edit because the two authority IDs differed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Additional verification: `npm run typecheck:cli`, `npm run test:titles:check`, `npm run test:projects:check`, `npm run checks:repository`, scoped format checking, and `git diff --check` passed. Independent documentation review found no documentation change necessary because the current guides already describe the intended healthy `status` and `doctor` result. <!-- docs-review-disposition: no-docs-needed --> <!-- docs-review-head-sha: 107563e --> <!-- docs-review-agents-blob-sha: 513518c --> --- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Prevented terminal attachment changes from altering host-local inference authority identifiers and binding digests. * Docker operations no longer include terminal-related environment details when determining authority. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
1 parent 76a354e commit 0e655a1

2 files changed

Lines changed: 19 additions & 1 deletion

File tree

src/lib/onboard/runtime-provider/docker-operation-authority.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,25 @@ describe("Docker operation authority", () => {
168168
expect(second.engine.capture(["version"]).stdout).toBe("unset\nunset\nunset\n");
169169
});
170170

171+
it("keeps host-local inference authority stable across terminal attachment changes (#9599)", () => {
172+
const executableRoot = fakeDocker("qualified");
173+
const environment = {
174+
HOME: "/tmp/nemoclaw-home",
175+
DOCKER_HOST: "unix:///tmp/nemoclaw-docker.sock",
176+
PATH: executableRoot,
177+
};
178+
const interactive = createDockerOperationAuthority("host-local-inference", {
179+
...environment,
180+
TERM: "xterm-256color",
181+
});
182+
const detached = createDockerOperationAuthority("host-local-inference", environment);
183+
184+
expect(detached.engine.authorityId).toBe(interactive.engine.authorityId);
185+
expect(dockerOperationBindingSha256(detached.engine)).toBe(
186+
dockerOperationBindingSha256(interactive.engine),
187+
);
188+
});
189+
171190
it("fails closed when an earlier Docker credential helper appears", () => {
172191
const executableRoot = fakeDocker("qualified");
173192
const prefixRoot = fakeExecutableRoot();

src/lib/onboard/runtime-provider/docker-operation-authority.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ const DOCKER_COMMAND_ENV_NAMES = new Set([
6868
"SHELL",
6969
"PATH",
7070
"SSH_AUTH_SOCK",
71-
"TERM",
7271
"HOSTNAME",
7372
"LANG",
7473
"TMPDIR",

0 commit comments

Comments
 (0)