From d19f114f1ffd713e703ce6fbd3087b2dc899a9e9 Mon Sep 17 00:00:00 2001 From: Jack <72348727+Jack-GitHub12@users.noreply.github.com> Date: Thu, 12 Nov 2026 15:57:31 -0600 Subject: [PATCH 1/4] test(capture): the hook tests pass when run inside an agent or a host app --- packages/coding.sh/test/capture/_helpers.ts | 22 ++++++++++++++++++- packages/coding.sh/test/capture/hook.test.ts | 12 +++++++++- .../test/cli/rebuild-redactions.test.ts | 3 ++- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/packages/coding.sh/test/capture/_helpers.ts b/packages/coding.sh/test/capture/_helpers.ts index 6d8d71a8..bf25baad 100644 --- a/packages/coding.sh/test/capture/_helpers.ts +++ b/packages/coding.sh/test/capture/_helpers.ts @@ -20,6 +20,26 @@ export function removeTmp(): void { for (const d of cleanup.splice(0)) rmSync(d, { recursive: true, force: true }); } +/** + * What the agent or host app running the tests may have set, and the hook reads: an eval run, a + * host app's thread, a parent agent's session, a model base URL. Left in, they send hook.sh to + * hook.mjs and add fields to the spool line, so a suite run inside an agent fails where CI passes. + */ +export const CALLER_ENV = [ + "CODING_SH_RUN_ID", "T3CODE_THREAD_ID", "CODEX_INTERNAL_ORIGINATOR_OVERRIDE", + "CLAUDE_CODE_ENTRYPOINT", "CLAUDE_CODE_SESSION_ID", "CODEX_THREAD_ID", "CODEX_TOOL_CALL_ID", + "ANTHROPIC_BASE_URL", "OPENAI_BASE_URL", "OPENAI_API_BASE", + "LITELLM_PROXY_API_BASE", "LITELLM_PROXY_URL", "LITELLM_BASE_URL", "PORTKEY_GATEWAY_URL", "RAMP_ROUTER_BASE_URL", + "TFY_GATEWAY_URL", "KONG_PROXY_URL", "BIFROST_URL", "BIFROST_BASE_URL", "CLIPROXYAPI_URL", +]; + +/** `env` without the caller's variables (CALLER_ENV). */ +export function withoutCallerEnv(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + const out = { ...env }; + for (const k of CALLER_ENV) delete out[k]; + return out; +} + /** Isolate git config and coding.sh data dir for this test process. Returns CODING_SH_HOME. */ export function isolate(): string { const base = tmp("csh-home-"); @@ -29,7 +49,7 @@ export function isolate(): string { process.env.GIT_CONFIG_NOSYSTEM = "1"; process.env.CODING_SH_HOME = join(base, "home"); delete process.env.CODING_SH_DISABLE; - delete process.env.CODING_SH_RUN_ID; + for (const k of CALLER_ENV) delete process.env[k]; return process.env.CODING_SH_HOME; } diff --git a/packages/coding.sh/test/capture/hook.test.ts b/packages/coding.sh/test/capture/hook.test.ts index caba0ea6..7ba13932 100644 --- a/packages/coding.sh/test/capture/hook.test.ts +++ b/packages/coding.sh/test/capture/hook.test.ts @@ -10,7 +10,7 @@ import { after, before, describe, test } from "node:test"; import { pathToFileURL } from "node:url"; import type { SpoolLine } from "../../src/core/types.ts"; import { MAX_INLINE_LINE_BYTES, SPOOL_FILE_CAP, appendSpool, resolveRaw, safeAgentId, spoolFile } from "../../src/spool/write.ts"; -import { PKG_ROOT, gitc, isolate, makeRepo, removeTmp, tmp, tsNodeArgs } from "./_helpers.ts"; +import { CALLER_ENV, PKG_ROOT, gitc, isolate, makeRepo, removeTmp, tmp, tsNodeArgs } from "./_helpers.ts"; import { median, pairedRatio, timeMs } from "../helpers-timing.ts"; const HOOK_TS = join(PKG_ROOT, "src", "spool", "hook.ts"); @@ -437,6 +437,16 @@ describeSh("hook.sh", () => { for (const k of BASE_URL_VARS) assert.ok(src.includes(`"\${${k}-}"`), k); }); + test("the tests drop every variable of the calling agent that hook.sh reads", () => { + // A suite run inside an agent or a host app inherits its variables. Each one hook.sh reads is + // either the test's own to set (OWN) or dropped by isolate() (CALLER_ENV), so a new one cannot slip in. + const OWN = ["HOME", "CODING_SH_HOME", "CODING_SH_DISABLE", "CODING_SH_ECHO_CONTINUE", "CODING_SH_HOOK_REEXEC", "CODING_SH_NO_AUTO_INGEST", "CODING_SH_NO_PROXY_AUTOSTART", "BASH_VERSION", "ZSH_VERSION", "SHELLOPTS", "EPOCHREALTIME"]; + const read = [...new Set([...readFileSync(HOOK_SH_SRC, "utf8").matchAll(/\$\{([A-Z][A-Z0-9_]*)-\}/g)].map((m) => m[1]!))]; + assert.ok(read.length > 20, "the test's premise: hook.sh reads its variables as ${NAME-}"); + assert.deepEqual(read.filter((k) => !OWN.includes(k) && !CALLER_ENV.includes(k)), []); + for (const k of CALLER_ENV) assert.equal(process.env[k], undefined, k); + }); + test("runs no git and no network tool; the fast path needs only cat, date, mkdir and wc", () => { const src = readFileSync(HOOK_SH_SRC, "utf8").replace(/^\s*#.*$/gm, ""); assert.doesNotMatch(src, /\bgit\b|\.git\b|packed-refs|\bcurl\b|\bwget\b|\bnc\b|\/dev\/tcp/); diff --git a/packages/coding.sh/test/cli/rebuild-redactions.test.ts b/packages/coding.sh/test/cli/rebuild-redactions.test.ts index 23640519..c214183a 100644 --- a/packages/coding.sh/test/cli/rebuild-redactions.test.ts +++ b/packages/coding.sh/test/cli/rebuild-redactions.test.ts @@ -6,6 +6,7 @@ import { chmodSync, mkdirSync, mkdtempSync, readdirSync, renameSync, rmSync, wri import { tmpdir } from "node:os"; import { join } from "node:path"; import { after, test } from "node:test"; +import { withoutCallerEnv } from "../capture/_helpers.ts"; import { CLI } from "../query/_seed.ts"; const root = mkdtempSync(join(tmpdir(), "csh-rebuild-redact-")); @@ -22,7 +23,7 @@ test("ingest --rebuild keeps the Redacted row of a prompt whose spool line was s const hookMjs = join(root, "dist", "hook.mjs"); mkdirSync(join(root, "dist"), { recursive: true }); writeFileSync(hookMjs, "// stand-in: this test runs hook.sh\n"); - const env = { ...process.env, CODING_SH_HOOK_SOURCE: hookMjs, PATH: `${bin}:${process.env.PATH}`, HOME: join(root, "h"), CODING_SH_HOME: join(root, "cs"), CLAUDE_CONFIG_DIR: join(root, "c"), CODEX_HOME: join(root, "x"), CODING_SH_NO_UPDATE_CHECK: "1", CODING_SH_NO_AUTO_INGEST: "1", NO_COLOR: "1" }; + const env = { ...withoutCallerEnv(process.env), CODING_SH_HOOK_SOURCE: hookMjs, PATH: `${bin}:${process.env.PATH}`, HOME: join(root, "h"), CODING_SH_HOME: join(root, "cs"), CLAUDE_CONFIG_DIR: join(root, "c"), CODEX_HOME: join(root, "x"), CODING_SH_NO_UPDATE_CHECK: "1", CODING_SH_NO_AUTO_INGEST: "1", NO_COLOR: "1" }; for (const d of [env.HOME, env.CLAUDE_CONFIG_DIR]) mkdirSync(d, { recursive: true }); const cli = (...args: string[]) => spawnSync(process.execPath, ["--no-warnings", CLI, ...args], { env, encoding: "utf8", timeout: 120_000 }); const init = cli("init", "--agent", "claude-code", "--yes", "--no-import", "--no-mcp"); From c78cd82c60a209f5bcfe2ac35362c1795416aef5 Mon Sep 17 00:00:00 2001 From: Jack <72348727+Jack-GitHub12@users.noreply.github.com> Date: Thu, 12 Nov 2026 15:58:37 -0600 Subject: [PATCH 2/4] test(sdk-python): the weights test that runs the CLI needs Node 22.16, as the CLI does --- packages/sdk-python/tests/test_weights.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/sdk-python/tests/test_weights.py b/packages/sdk-python/tests/test_weights.py index 01f072ce..a1dcbcd2 100644 --- a/packages/sdk-python/tests/test_weights.py +++ b/packages/sdk-python/tests/test_weights.py @@ -469,12 +469,12 @@ def _node_ok() -> bool: try: v = subprocess.run([NODE, "-v"], capture_output=True, text=True, timeout=10).stdout.strip().lstrip("v") major, minor = (int(x) for x in v.split(".")[:2]) - return (major, minor) >= (22, 15) + return (major, minor) >= (22, 16) except Exception: return False -@unittest.skipUnless(_node_ok(), "needs node >= 22.15 and the core package") +@unittest.skipUnless(_node_ok(), "needs node >= 22.16 and the core package") class WeightsEditServerTest(unittest.TestCase): """A real coding.sh serve on a temp CODING_SH_HOME: the event comes back with the secret redacted.""" From c63e286529e055217a4fecc7d297f7703a5472cc Mon Sep 17 00:00:00 2001 From: Jack <72348727+Jack-GitHub12@users.noreply.github.com> Date: Thu, 12 Nov 2026 15:59:30 -0600 Subject: [PATCH 3/4] docs(stores): say what happens when a sink sends a server its own sessions --- docs/reference/stores.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/reference/stores.md b/docs/reference/stores.md index f233a832..eed5e72d 100644 --- a/docs/reference/stores.md +++ b/docs/reference/stores.md @@ -173,6 +173,10 @@ mode = "metadata" On a team, set `CODING_SH_FORWARD_URL=http://collector:4319` on serve instead, and it forwards every trace it commits. +Point a sink or the forward at a collector, not back at the coding.sh server whose sessions it +sends. Spans that come back over `/v1/traces` are stored as new events of the same session, so +its tool calls, model calls and weight edits are counted twice. + #### The span table What lands in `otel_traces`, with `ServiceName = 'coding.sh'`: From e97151bbe2fd9b0152a7cf1712d59c216bf669f0 Mon Sep 17 00:00:00 2001 From: Jack <72348727+Jack-GitHub12@users.noreply.github.com> Date: Thu, 12 Nov 2026 16:03:20 -0600 Subject: [PATCH 4/4] fix(sdk-python): weights_edit warns about a dropped edit once per process, not once per init --- CHANGELOG.md | 1 + packages/sdk-python/src/coding_sh/_weights.py | 17 ++++++++++++++++- packages/sdk-python/tests/test_weights.py | 11 +++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce6564f8..8d7626a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ grouped by area, with links to the docs for the details. - **Evals.** `exp metrics` prints a count or score below 0.0005 with two significant digits (`0.0001`) instead of `0.000`, so a small value such as a gain per edited weight no longer reads as zero. [Metric A/B](https://github.com/jackulau/coding.sh/blob/main/docs/guides/evals.md#metric-ab-ms-bytes) - **SDK.** `coding_sh.weights_edit()` reads a 0-d tensor or array as the number it holds, so `l2=delta.norm()` records the edit. Before, a torch scalar was refused and the edit was dropped with a warning, though the docs said torch scalars count as numbers. [Docs](https://coding.sh/docs/api) +- **SDK.** `coding_sh.weights_edit()` warns about a dropped edit once per process, as its docs say and as the TypeScript SDK does. Before, every `init()` warned again. [Docs](https://coding.sh/docs/sdk) - **MCP.** A request cancelled with `notifications/cancelled` gets no reply (a cancel for `initialize` is ignored), a cursor on `tools/list`, `prompts/list` or `resources/templates/list` is an error, a request with a null id gets an error instead of no answer (a null-id `notifications/` message is still a notification), and a 2025-03-26 session accepts batches. [MCP server](https://coding.sh/docs/mcp#protocol) - **MCP.** Tool answers stay under 25,000 characters: a longer page has fewer rows and `limitUsed`, and a tool with a `cursor` also returns `nextCursor`. `session_replay` pages its files and cuts text and lists only when an answer would not fit, saying so with `truncated` (a range was 5 million characters). [MCP server](https://coding.sh/docs/mcp#tools) - **MCP.** `eval_init` and `score_add` are marked destructive, since both can replace stored data, and `score_list`'s error names its `since` argument instead of the CLI's `--since`. [MCP server](https://coding.sh/docs/mcp#tools) diff --git a/packages/sdk-python/src/coding_sh/_weights.py b/packages/sdk-python/src/coding_sh/_weights.py index abe17a71..fa256d7b 100644 --- a/packages/sdk-python/src/coding_sh/_weights.py +++ b/packages/sdk-python/src/coding_sh/_weights.py @@ -69,9 +69,24 @@ def _sha(v: Any, what: str) -> str: return v.lower() +# Keys already warned about. The transport's own table starts empty at each init(), and these +# warnings are once per process, as in the TypeScript SDK. +_warned: "set[str]" = set() + + +def _reset_warned() -> None: + """For tests: warn again.""" + _warned.clear() + + def _warn(key: str, msg: str) -> None: try: - _get()._warn_once(key, msg) + t = _get() + if key in _warned: + t._debug("%s", msg) + return + _warned.add(key) + t._warn_once(key, msg) except Exception: pass diff --git a/packages/sdk-python/tests/test_weights.py b/packages/sdk-python/tests/test_weights.py index a1dcbcd2..b412ca14 100644 --- a/packages/sdk-python/tests/test_weights.py +++ b/packages/sdk-python/tests/test_weights.py @@ -23,6 +23,7 @@ from support import IngestServer, by_kind, collect, start_sdk import coding_sh +from coding_sh import _weights A = "a" * 64 B = "b" * 64 @@ -122,6 +123,7 @@ def _edit(**kw: Any) -> bool: class WeightsEditTest(unittest.TestCase): def setUp(self) -> None: + _weights._reset_warned() self.server = IngestServer() def tearDown(self) -> None: @@ -253,6 +255,15 @@ def test_warns_once_per_process(self) -> None: _edit(l2=-1.0) self.assertEqual(len([r for r in cm.records if "weights_edit dropped" in r.getMessage()]), 1) + def test_a_second_init_does_not_warn_again(self) -> None: + start_sdk(self.server) + with self.assertLogs("coding_sh", level="WARNING") as cm: + _edit(changed=-1) + coding_sh.shutdown(1.0) + start_sdk(self.server) + _edit(changed=-2) + self.assertEqual(len([r for r in cm.records if "weights_edit dropped" in r.getMessage()]), 1) + def test_note_that_is_not_a_string_is_dropped_with_a_warning(self) -> None: start_sdk(self.server) with self.assertLogs("coding_sh", level="WARNING") as cm: