session: Harness-Session-Variable schliesst die Luecke im Session-Id-Fallback (Telemetrie-Join, Iteration-Budget-Gate); SIGPIPE-Nebenbefund im Emitter behoben
Files changed: - CHANGES.md - EVALS.md - INSTALL.md - VERSION - instructions/session-setup.md - tools/CONTRACT.md - tools/chemenu/cli.py - tools/chemenu/commands/doctor.py - tools/chemenu/commands/run_budget.py - tools/chemenu/session.py - tools/chemenu/telemetry/writer.py - tools/chemenu/tests/conftest.py - tools/chemenu/tests/test_cli.py - tools/chemenu/tests/test_run_budget.py - tools/chemenu/tests/test_telemetry_emit.py
This commit is contained in:
+7
-4
@@ -214,7 +214,7 @@ tools/wikitool <command> --help
|
||||
|
||||
| Command | Purpose |
|
||||
|---------|---------|
|
||||
| `doctor [--json]` | Check that this instance is correctly configured: dependencies (Python, ripgrep), author resolution, stack version, git identity/branch/remote, published skills, kb/raw/reports/work/instructions structure, personalization (`USER.md`/`SOUL.md` present **and** filled - a file still carrying the template's sentinel is a `FAIL`, since a renamed template is not a filled one), the KB conventions (`kb/CONVENTIONS.md` present, unsentinelled, and naming all three tool-owned section headings - a `FAIL` on any of the three, because `xref`/`cite` write out of it), the environment note (`ENVIRONMENT.md` - optional, so absent is `OK`; a still-templated one is a `WARN`), generated files, whether the MCP `submit` tool is armed (`.wikitool-upload.json` present/absent/malformed, its limits, and how many submissions are waiting in `mcp-upload/` - absent is `OK` and means the write path does not exist at all, malformed is the one `FAIL` here, since a broken opt-in must not silently disable the limits it exists to enforce), `WIKITOOL_SESSION_ID`, and telemetry state (on/off, why - installation-form default, `.wikitool-telemetry.json`, or `WIKI_TRACE` - and the current session count/byte total against both caps; never `FAIL`, see [EVALS.md](../EVALS.md)). Read-only, exit 1 only on a `FAIL` (a missing remote, session id, or `VERSION` is a `WARN`, not a fault). Exempt from the Iteration Budget Gate |
|
||||
| `doctor [--json]` | Check that this instance is correctly configured: dependencies (Python, ripgrep), author resolution, stack version, git identity/branch/remote, published skills, kb/raw/reports/work/instructions structure, personalization (`USER.md`/`SOUL.md` present **and** filled - a file still carrying the template's sentinel is a `FAIL`, since a renamed template is not a filled one), the KB conventions (`kb/CONVENTIONS.md` present, unsentinelled, and naming all three tool-owned section headings - a `FAIL` on any of the three, because `xref`/`cite` write out of it), the environment note (`ENVIRONMENT.md` - optional, so absent is `OK`; a still-templated one is a `WARN`), generated files, whether the MCP `submit` tool is armed (`.wikitool-upload.json` present/absent/malformed, its limits, and how many submissions are waiting in `mcp-upload/` - absent is `OK` and means the write path does not exist at all, malformed is the one `FAIL` here, since a broken opt-in must not silently disable the limits it exists to enforce), the session id source (`OK` for `WIKITOOL_SESSION_ID` or a registered harness variable, `WARN` only for the bare parent-pid fallback - see `chemenu.session`), and telemetry state (on/off, why - installation-form default, `.wikitool-telemetry.json`, or `WIKI_TRACE` - and the current session count/byte total against both caps; never `FAIL`, see [EVALS.md](../EVALS.md)). Read-only, exit 1 only on a `FAIL` (a missing remote, session id, or `VERSION` is a `WARN`, not a fault). Exempt from the Iteration Budget Gate |
|
||||
|
||||
## Design notes
|
||||
|
||||
@@ -268,9 +268,12 @@ tools/wikitool <command> --help
|
||||
section): every invocation is recorded and checked in `main()` (`cli.py`)
|
||||
before Typer dispatches to any subcommand, so it applies uniformly without
|
||||
each command needing its own opt-in. State lives in the gitignored
|
||||
`tools/.wikitool_session/budget.json`, keyed by `WIKITOOL_SESSION_ID` (or
|
||||
the caller's parent process id as a fallback), so a new terminal/session
|
||||
starts with a clean budget. Default ceiling: 60 calls/session, or 3
|
||||
`tools/.wikitool_session/budget.json`, keyed by `chemenu.session`'s fallback
|
||||
chain (`WIKITOOL_SESSION_ID`, else a registered harness session variable,
|
||||
else the caller's parent process id), so a new terminal/session starts with
|
||||
a clean budget - and a bucket whose recorded origin no longer matches the
|
||||
current one starts a fresh count rather than inheriting a stranger's.
|
||||
Default ceiling: 60 calls/session, or 3
|
||||
identical calls in a row (whichever trips first). A call that left through
|
||||
`_util.fail()` - a rejected argument, or a read-only check reporting
|
||||
findings - is refunded: it declined instead of acting, and the contract's own
|
||||
|
||||
+96
-10
@@ -3,6 +3,8 @@
|
||||
The root AGENTS.md holds the invariants that say when these commands are
|
||||
mandatory; tools/CONTRACT.md is the full per-command reference.
|
||||
"""
|
||||
import errno
|
||||
import os
|
||||
import sys
|
||||
import time
|
||||
|
||||
@@ -50,6 +52,82 @@ except ModuleNotFoundError as exc:
|
||||
|
||||
from chemenu.telemetry import emit # noqa: E402 - after the dependency check
|
||||
|
||||
|
||||
class _BrokenPipeSwallow:
|
||||
"""Wraps a stream so a write into a closed pipe is dropped instead of
|
||||
raised - installed on `sys.stdout`/`sys.stderr` before Typer/Click ever
|
||||
run, so Click's own broken-pipe handling (`click.core.BaseCommand.main`)
|
||||
never gets the chance to fire.
|
||||
|
||||
Why not just read Click's outcome afterwards: Click already catches this
|
||||
exact case (`OSError` with `errno.EPIPE`) and turns it into `sys.exit(1)`
|
||||
to avoid a traceback - a clean-looking exit, but indistinguishable from a
|
||||
real failure to whatever reads that exit code next. `cli._run_traced`
|
||||
does exactly that: it is the trace, which recorded a truncated-but-
|
||||
otherwise-successful `types describe source | head -1` as a tool error
|
||||
(Gitea #110, measured against a real trace: `exit_code: 1` for a call the
|
||||
very next, unpiped, retry of which showed `exit_code: 0`).
|
||||
|
||||
Swallowing the write here instead means Click's own handler never
|
||||
triggers, so the command finishes through its normal exit path - `0` for
|
||||
an otherwise-successful run - and `sigpipe` on this wrapper is the signal
|
||||
`_run_traced` reads to note the truncation without miscasting it as an
|
||||
error.
|
||||
"""
|
||||
|
||||
def __init__(self, wrapped):
|
||||
self._wrapped = wrapped
|
||||
self.sigpipe = False
|
||||
|
||||
def _is_epipe(self, exc: OSError) -> bool:
|
||||
return exc.errno == errno.EPIPE
|
||||
|
||||
def write(self, data):
|
||||
try:
|
||||
return self._wrapped.write(data)
|
||||
except OSError as exc:
|
||||
if not self._is_epipe(exc):
|
||||
raise
|
||||
self.sigpipe = True
|
||||
return len(data)
|
||||
|
||||
def flush(self):
|
||||
try:
|
||||
self._wrapped.flush()
|
||||
except OSError as exc:
|
||||
if not self._is_epipe(exc):
|
||||
raise
|
||||
self.sigpipe = True
|
||||
|
||||
def __getattr__(self, attr):
|
||||
return getattr(self._wrapped, attr)
|
||||
|
||||
|
||||
def _pacify_real_fd(stream) -> None:
|
||||
"""Redirect a broken stream's real file descriptor to `os.devnull`.
|
||||
|
||||
Swallowing the write in `_BrokenPipeSwallow` is not enough on its own:
|
||||
CPython still flushes the *real* underlying stream automatically at
|
||||
interpreter shutdown, by code this module does not control, and that
|
||||
flush hits the same closed pipe - printing "Exception ignored while
|
||||
flushing sys.stdout" (the well-known CPython caveat; see the standard
|
||||
library docs' "Note on SIGPIPE"). Once a pipe is known broken there is
|
||||
nothing left worth writing to it, so pointing the fd at `/dev/null`
|
||||
makes every later flush - ours or the interpreter's own - a normal
|
||||
write that always succeeds.
|
||||
"""
|
||||
try:
|
||||
devnull = os.open(os.devnull, os.O_WRONLY)
|
||||
try:
|
||||
os.dup2(devnull, stream.fileno())
|
||||
finally:
|
||||
os.close(devnull)
|
||||
except (OSError, AttributeError):
|
||||
# AttributeError: a stream with no real fd at all (a test double, or
|
||||
# a harness that already replaced sys.stdout with something that
|
||||
# isn't a file) - nothing to redirect, same as the OSError case.
|
||||
pass
|
||||
|
||||
app = typer.Typer(
|
||||
help="wikitool - deterministic operations for Chemenu (see AGENTS.md).",
|
||||
no_args_is_help=True,
|
||||
@@ -127,6 +205,10 @@ def _run_traced(command: str, args: list[str], charged: bool = False) -> None:
|
||||
"""
|
||||
started = time.monotonic()
|
||||
exit_code = 0
|
||||
real_stdout, real_stderr = sys.stdout, sys.stderr
|
||||
stdout_wrap = _BrokenPipeSwallow(real_stdout)
|
||||
stderr_wrap = _BrokenPipeSwallow(real_stderr)
|
||||
sys.stdout, sys.stderr = stdout_wrap, stderr_wrap
|
||||
try:
|
||||
app()
|
||||
except SystemExit as exc:
|
||||
@@ -137,18 +219,22 @@ def _run_traced(command: str, args: list[str], charged: bool = False) -> None:
|
||||
exit_code = 1
|
||||
raise
|
||||
finally:
|
||||
if stdout_wrap.sigpipe:
|
||||
_pacify_real_fd(real_stdout)
|
||||
if stderr_wrap.sigpipe:
|
||||
_pacify_real_fd(real_stderr)
|
||||
sys.stdout, sys.stderr = real_stdout, real_stderr
|
||||
if charged and _util.declined():
|
||||
run_budget.refund()
|
||||
emit(
|
||||
"wikitool",
|
||||
"wikitool.call",
|
||||
{
|
||||
"command": command,
|
||||
"args": args,
|
||||
"exit_code": exit_code,
|
||||
"duration_ms": round((time.monotonic() - started) * 1000, 1),
|
||||
},
|
||||
)
|
||||
attrs = {
|
||||
"command": command,
|
||||
"args": args,
|
||||
"exit_code": exit_code,
|
||||
"duration_ms": round((time.monotonic() - started) * 1000, 1),
|
||||
}
|
||||
if stdout_wrap.sigpipe or stderr_wrap.sigpipe:
|
||||
attrs["stdout_truncated"] = True
|
||||
emit("wikitool", "wikitool.call", attrs)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
@@ -24,6 +24,7 @@ from chemenu import config, conventions, kb_collections, version as version_mod
|
||||
from chemenu.commands import git_publish, instructions_cmd
|
||||
from chemenu.commands._util import rel_path
|
||||
from chemenu.session import ENV_VAR as SESSION_ENV_VAR
|
||||
from chemenu.session import session_id_source as _session_id_source
|
||||
|
||||
console = Console()
|
||||
|
||||
@@ -414,12 +415,23 @@ def check_upload_intake() -> Check:
|
||||
|
||||
|
||||
def check_session_id() -> Check:
|
||||
"""Three-valued, not two: an explicit `WIKITOOL_SESSION_ID` and a
|
||||
recognised harness variable (see `chemenu.session.HARNESS_ENV_VARS`) both
|
||||
keep a session's calls in one telemetry/budget bucket, so both are `OK`.
|
||||
Only the `getppid()` fallback - a fresh "session" on every call, on a
|
||||
harness that runs each tool call in its own shell - is a `WARN` (see
|
||||
Gitea #110)."""
|
||||
import os
|
||||
|
||||
if os.environ.get(SESSION_ENV_VAR, "").strip():
|
||||
return Check("session-id", "OK", f"{SESSION_ENV_VAR}={os.environ[SESSION_ENV_VAR]}")
|
||||
source = _session_id_source()
|
||||
if source != "getppid() fallback":
|
||||
return Check("session-id", "OK", f"scoped by harness variable {source}")
|
||||
return Check(
|
||||
"session-id", "WARN", f"{SESSION_ENV_VAR} is not set - budget falls back to the parent PID",
|
||||
"session-id", "WARN",
|
||||
f"{SESSION_ENV_VAR} is not set and no harness session variable was found - "
|
||||
"budget falls back to the parent PID",
|
||||
"See instructions/session-setup.md",
|
||||
)
|
||||
|
||||
|
||||
@@ -144,6 +144,27 @@ def _session_id_source() -> str:
|
||||
return _shared_session_id_source()
|
||||
|
||||
|
||||
def _entry_for(state: dict, session_id: str) -> dict:
|
||||
"""The state entry for this session id, starting a fresh counter if the
|
||||
same id string now carries a different origin than the one that wrote it.
|
||||
|
||||
Two different id spaces (a `getppid()` integer, a harness UUID, an
|
||||
explicit `WIKITOOL_SESSION_ID`) are vanishingly unlikely to collide as
|
||||
strings - but "unlikely" is not "impossible", and inheriting a stranger's
|
||||
count on collision is exactly the silent mis-key #110 exists to close.
|
||||
An entry written before this field existed carries no `source` at all and
|
||||
is treated as compatible: it keeps its count rather than being reset the
|
||||
first time this ships, which would throw away real, in-flight state.
|
||||
"""
|
||||
source = _session_id_source()
|
||||
entry = state.get(session_id)
|
||||
if entry is None or (entry.get("source") is not None and entry["source"] != source):
|
||||
entry = {"count": 0, "recent": []}
|
||||
state[session_id] = entry
|
||||
entry.setdefault("source", source)
|
||||
return entry
|
||||
|
||||
|
||||
def _load_state() -> dict:
|
||||
if not STATE_FILE.exists():
|
||||
return {}
|
||||
@@ -247,7 +268,7 @@ def record_and_check(
|
||||
with _state_lock():
|
||||
session_id = _session_id()
|
||||
state = _load_state()
|
||||
entry = state.setdefault(session_id, {"count": 0, "recent": []})
|
||||
entry = _entry_for(state, session_id)
|
||||
recent = entry["recent"]
|
||||
|
||||
call_signature = f"{command} {' '.join(args)}".strip()
|
||||
|
||||
@@ -4,10 +4,35 @@ One definition, because the two must agree: if telemetry grouped events
|
||||
differently from the way the budget counts calls, a trace could not be read
|
||||
against the gate that refused it.
|
||||
|
||||
A "session" is approximated by the parent process of this CLI invocation - the
|
||||
agent's shell - unless the caller sets `WIKITOOL_SESSION_ID`. Skills set it
|
||||
explicitly so a session is scoped to a task rather than to a terminal window
|
||||
(see instructions/session-setup.md).
|
||||
Three-step fallback chain, in order:
|
||||
|
||||
1. `WIKITOOL_SESSION_ID`, if the caller set one explicitly. Skills set it so a
|
||||
session is scoped to a task rather than to a terminal window (see
|
||||
instructions/session-setup.md).
|
||||
2. A harness's own session variable, from `HARNESS_ENV_VARS` below - checked
|
||||
only when nothing set the variable above.
|
||||
3. `os.getppid()` - the parent process of this CLI invocation. On a harness
|
||||
that runs every tool call in a freshly initialised shell (Claude Code's
|
||||
Bash tool does), this is a new "session" per call and neither the
|
||||
iteration-budget gate's ceiling nor its loop-breaker can ever trip - see
|
||||
Gitea #110, which measured a 33-call run splitting into 21 telemetry
|
||||
buckets under this fallback alone.
|
||||
|
||||
Step 2 is what closes that gap without asking every skill to `export` a
|
||||
variable a harness already re-derives per call: `CLAUDE_CODE_SESSION_ID` is
|
||||
stable across a Claude Code session's tool calls (verified 2026-09-16,
|
||||
against a live session, across separate Bash invocations - the shell's own
|
||||
PID changed on every call, this variable did not) and is **exactly** the id
|
||||
the `UserPromptSubmit` hook writes into a trace's `session.start` and
|
||||
`prompt.submitted` events. Using it unmodified as the budget/telemetry key -
|
||||
no prefix, no rewriting - is what lets the hook's events and this module's
|
||||
events land in the same bucket.
|
||||
|
||||
`HARNESS_ENV_VARS` only ever grows by a verified entry: a variable a real
|
||||
session was observed setting, confirmed to be the same id a harness's own
|
||||
hooks use elsewhere in a trace. A guessed name that happens to exist and
|
||||
means something else would be worse than the `getppid()` fallback it would
|
||||
replace - it would look like a fix and quietly mis-key a session instead.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -16,15 +41,46 @@ import re
|
||||
|
||||
ENV_VAR = "WIKITOOL_SESSION_ID"
|
||||
|
||||
HARNESS_ENV_VARS: tuple[tuple[str, str], ...] = (
|
||||
("CLAUDE_CODE_SESSION_ID", "claude-code"),
|
||||
)
|
||||
|
||||
_UNSAFE = re.compile(r"[^A-Za-z0-9._-]+")
|
||||
|
||||
|
||||
def _harness_session() -> tuple[str, str] | None:
|
||||
"""The first harness variable that is actually set, as `(value, harness)`."""
|
||||
for var, harness in HARNESS_ENV_VARS:
|
||||
value = os.environ.get(var)
|
||||
if value:
|
||||
return value, harness
|
||||
return None
|
||||
|
||||
|
||||
def session_id() -> str:
|
||||
return os.environ.get(ENV_VAR) or str(os.getppid())
|
||||
explicit = os.environ.get(ENV_VAR)
|
||||
if explicit:
|
||||
return explicit
|
||||
harness = _harness_session()
|
||||
if harness:
|
||||
return harness[0]
|
||||
return str(os.getppid())
|
||||
|
||||
|
||||
def session_id_source() -> str:
|
||||
return ENV_VAR if os.environ.get(ENV_VAR) else "getppid() fallback"
|
||||
"""Where the id in `session_id()` came from - `ENV_VAR`, a harness
|
||||
variable name (with the harness named alongside it), or the `getppid()`
|
||||
fallback. `doctor`, `budget status` and the `wikitool` source's
|
||||
`session.start` event all read this so a session - or a trace - can say
|
||||
what it was keyed on, not just what the id happened to be."""
|
||||
if os.environ.get(ENV_VAR):
|
||||
return ENV_VAR
|
||||
harness = _harness_session()
|
||||
if harness:
|
||||
_, name = harness
|
||||
var = next(v for v, h in HARNESS_ENV_VARS if h == name)
|
||||
return f"{var} ({name})"
|
||||
return "getppid() fallback"
|
||||
|
||||
|
||||
def session_slug(value: str | None = None) -> str:
|
||||
|
||||
@@ -30,6 +30,7 @@ from pathlib import Path
|
||||
|
||||
from chemenu import config
|
||||
from chemenu.session import session_id as current_session_id
|
||||
from chemenu.session import session_id_source as current_session_id_source
|
||||
from chemenu.session import session_slug
|
||||
from chemenu.telemetry import policy, schema, scrub
|
||||
|
||||
@@ -155,14 +156,22 @@ def _seed_session_header(target: Path, source: str, session: str) -> None:
|
||||
except FileExistsError:
|
||||
return
|
||||
with handle:
|
||||
attrs = {
|
||||
"harness": source,
|
||||
"completeness": list(schema.HARNESS_CAPABILITIES.get(source, ())),
|
||||
"synthesized": True,
|
||||
}
|
||||
# Only the `wikitool` source resolves its own session id through
|
||||
# chemenu.session's fallback chain - every other source hands `emit()`
|
||||
# an id its own hook payload already carried. Naming the chain's
|
||||
# outcome here is what lets a trace say what it was keyed on, not just
|
||||
# what the id happened to be (Gitea #110).
|
||||
if source == "wikitool":
|
||||
attrs["session_origin"] = current_session_id_source()
|
||||
header = schema.make_event(
|
||||
source,
|
||||
"session.start",
|
||||
{
|
||||
"harness": source,
|
||||
"completeness": list(schema.HARNESS_CAPABILITIES.get(source, ())),
|
||||
"synthesized": True,
|
||||
},
|
||||
attrs,
|
||||
session_id=session,
|
||||
seq=_next_seq(),
|
||||
)
|
||||
|
||||
@@ -6,6 +6,7 @@ import pytest
|
||||
|
||||
from chemenu import config, conventions
|
||||
from chemenu.frontmatter_io import write_page
|
||||
from chemenu.session import HARNESS_ENV_VARS
|
||||
from chemenu.telemetry import policy as telemetry_policy
|
||||
from chemenu.type_resolver import resolver
|
||||
|
||||
@@ -13,6 +14,13 @@ from chemenu.type_resolver import resolver
|
||||
# that a test which needs one sets it itself and the rest run against the
|
||||
# tool's own defaults. `WIKI_TRACE_DIR` is deliberately absent: it is not a
|
||||
# leak but the redirect `isolated_trace_dir` installs one fixture below.
|
||||
#
|
||||
# The harness variables from `chemenu.session.HARNESS_ENV_VARS` are pulled in
|
||||
# here rather than duplicated: this suite runs *inside* Claude Code, so
|
||||
# `CLAUDE_CODE_SESSION_ID` is genuinely set in the real environment - without
|
||||
# clearing it, every session-fallback test would silently pick up this
|
||||
# session's real id instead of exercising the fallback it means to test
|
||||
# (Gitea #110).
|
||||
_WIKITOOL_ENV = (
|
||||
"WIKI_AUTHOR",
|
||||
"WIKI_TRACE",
|
||||
@@ -24,7 +32,7 @@ _WIKITOOL_ENV = (
|
||||
"WIKITOOL_UPDATE_URL",
|
||||
"WIKITOOL_UPDATE_TOKEN",
|
||||
"CHEMENU_ROOT",
|
||||
)
|
||||
) + tuple(var for var, _harness in HARNESS_ENV_VARS)
|
||||
|
||||
# Environment git reads for identity or for where its repo lives. A stray
|
||||
# `GIT_DIR` would point every fixture repo at the developer's checkout; the
|
||||
|
||||
@@ -0,0 +1,167 @@
|
||||
"""The CLI dispatch wrapper: the budget/trace hook every command runs
|
||||
through (`cli.main`/`cli._run_traced`), and the broken-pipe handling that
|
||||
sits alongside it.
|
||||
|
||||
Gitea #110's SIGPIPE side finding: a successful call whose output is cut off
|
||||
by a closed pipe (`wikitool types describe source | head -1`) used to record
|
||||
`exit_code: 1` in the trace - indistinguishable from a real tool failure, and
|
||||
reproduced verbatim by the very next, unpiped retry of the same command
|
||||
showing `exit_code: 0`. `cli._BrokenPipeSwallow` and `cli._pacify_real_fd`
|
||||
exist to close that gap; these tests exercise them without depending on a
|
||||
real OS pipe or Click's own internal handling, which is exactly the moving
|
||||
part being routed around.
|
||||
"""
|
||||
import errno
|
||||
import json
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
|
||||
from chemenu import cli
|
||||
|
||||
|
||||
def read_lines(path):
|
||||
return [json.loads(line) for line in path.read_text(encoding="utf-8").splitlines()]
|
||||
|
||||
|
||||
class _FailingStream:
|
||||
"""Raises EPIPE on the `fail_on`-th write - a fake stream standing in for
|
||||
a real pipe whose reader has already closed."""
|
||||
|
||||
def __init__(self, fail_on=1):
|
||||
self.fail_on = fail_on
|
||||
self.calls = 0
|
||||
self.written = []
|
||||
self.flushed = False
|
||||
|
||||
def write(self, data):
|
||||
self.calls += 1
|
||||
if self.calls >= self.fail_on:
|
||||
raise OSError(errno.EPIPE, "Broken pipe")
|
||||
self.written.append(data)
|
||||
return len(data)
|
||||
|
||||
def flush(self):
|
||||
self.flushed = True
|
||||
|
||||
def isatty(self):
|
||||
return False
|
||||
|
||||
|
||||
# --- _BrokenPipeSwallow ---
|
||||
|
||||
def test_broken_pipe_swallow_absorbs_epipe_on_write():
|
||||
swallow = cli._BrokenPipeSwallow(_FailingStream(fail_on=1))
|
||||
n = swallow.write("hello")
|
||||
assert n == len("hello")
|
||||
assert swallow.sigpipe is True
|
||||
|
||||
|
||||
def test_broken_pipe_swallow_absorbs_epipe_on_flush():
|
||||
class _FlushFails:
|
||||
def flush(self):
|
||||
raise OSError(errno.EPIPE, "Broken pipe")
|
||||
|
||||
swallow = cli._BrokenPipeSwallow(_FlushFails())
|
||||
swallow.flush() # does not raise
|
||||
assert swallow.sigpipe is True
|
||||
|
||||
|
||||
def test_broken_pipe_swallow_reraises_unrelated_oserrors():
|
||||
class _Explodes:
|
||||
def write(self, data):
|
||||
raise OSError(errno.ENOSPC, "No space left on device")
|
||||
|
||||
swallow = cli._BrokenPipeSwallow(_Explodes())
|
||||
with pytest.raises(OSError):
|
||||
swallow.write("x")
|
||||
assert swallow.sigpipe is False
|
||||
|
||||
|
||||
def test_broken_pipe_swallow_passes_through_normal_writes():
|
||||
wrapped = _FailingStream(fail_on=99)
|
||||
swallow = cli._BrokenPipeSwallow(wrapped)
|
||||
swallow.write("hello")
|
||||
assert wrapped.written == ["hello"]
|
||||
assert swallow.sigpipe is False
|
||||
|
||||
|
||||
def test_broken_pipe_swallow_proxies_unknown_attributes():
|
||||
wrapped = _FailingStream()
|
||||
swallow = cli._BrokenPipeSwallow(wrapped)
|
||||
assert swallow.isatty() is False
|
||||
|
||||
|
||||
# --- _pacify_real_fd ---
|
||||
|
||||
def test_pacify_real_fd_is_a_best_effort_noop_without_a_real_descriptor():
|
||||
class _RaisesOSError:
|
||||
def fileno(self):
|
||||
raise OSError("not a real fd in this test")
|
||||
|
||||
class _HasNoFilenoAtAll:
|
||||
pass
|
||||
|
||||
cli._pacify_real_fd(_RaisesOSError()) # must not raise
|
||||
cli._pacify_real_fd(_HasNoFilenoAtAll()) # must not raise either
|
||||
|
||||
|
||||
# --- _run_traced: the trace records what actually happened ---
|
||||
|
||||
def test_a_write_cut_off_by_a_closed_pipe_is_not_recorded_as_an_error(monkeypatch, tmp_path):
|
||||
monkeypatch.setenv("WIKI_TRACE_DIR", str(tmp_path))
|
||||
monkeypatch.setenv("WIKITOOL_SESSION_ID", "sigpipe-unit")
|
||||
stub = _FailingStream(fail_on=2) # first write succeeds, second hits EPIPE
|
||||
monkeypatch.setattr(sys, "stdout", stub)
|
||||
|
||||
def fake_app():
|
||||
sys.stdout.write("line one\n")
|
||||
sys.stdout.write("line two\n") # truncated here, like `| head -1`
|
||||
raise SystemExit(0)
|
||||
|
||||
monkeypatch.setattr(cli, "app", fake_app)
|
||||
|
||||
with pytest.raises(SystemExit) as exc:
|
||||
cli._run_traced("types", ["describe", "source"])
|
||||
assert exc.value.code == 0
|
||||
|
||||
records = read_lines(tmp_path / "sigpipe-unit" / "trace.jsonl")
|
||||
call = next(r for r in records if r["event"] == "wikitool.call")
|
||||
assert call["attrs"]["exit_code"] == 0
|
||||
assert call["attrs"]["stdout_truncated"] is True
|
||||
|
||||
|
||||
def test_a_real_failure_is_still_recorded_as_one(monkeypatch, tmp_path):
|
||||
"""The unrelated-error path stays exactly as before: an actual failure
|
||||
keeps its exit code and carries no truncation flag."""
|
||||
monkeypatch.setenv("WIKI_TRACE_DIR", str(tmp_path))
|
||||
monkeypatch.setenv("WIKITOOL_SESSION_ID", "real-failure-unit")
|
||||
|
||||
def fake_app():
|
||||
raise SystemExit(1)
|
||||
|
||||
monkeypatch.setattr(cli, "app", fake_app)
|
||||
|
||||
with pytest.raises(SystemExit) as exc:
|
||||
cli._run_traced("new", ["entity", "--name", ""])
|
||||
assert exc.value.code == 1
|
||||
|
||||
records = read_lines(tmp_path / "real-failure-unit" / "trace.jsonl")
|
||||
call = next(r for r in records if r["event"] == "wikitool.call")
|
||||
assert call["attrs"]["exit_code"] == 1
|
||||
assert "stdout_truncated" not in call["attrs"]
|
||||
|
||||
|
||||
def test_an_ordinary_call_restores_the_real_streams_afterwards(monkeypatch, tmp_path):
|
||||
monkeypatch.setenv("WIKI_TRACE_DIR", str(tmp_path))
|
||||
monkeypatch.setenv("WIKITOOL_SESSION_ID", "restore-unit")
|
||||
real_stdout, real_stderr = sys.stdout, sys.stderr
|
||||
|
||||
def fake_app():
|
||||
raise SystemExit(0)
|
||||
|
||||
monkeypatch.setattr(cli, "app", fake_app)
|
||||
with pytest.raises(SystemExit):
|
||||
cli._run_traced("lint", [])
|
||||
assert sys.stdout is real_stdout
|
||||
assert sys.stderr is real_stderr
|
||||
@@ -1,8 +1,12 @@
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
import typer
|
||||
|
||||
from chemenu import config
|
||||
from chemenu.commands import run_budget
|
||||
|
||||
|
||||
@@ -137,6 +141,39 @@ def test_override_bypasses_both_gates():
|
||||
assert state["test-session"]["count"] == 40
|
||||
|
||||
|
||||
def test_a_bucket_without_a_recorded_origin_keeps_its_count():
|
||||
"""Grandfathering (#110 decision 1): an entry written before this field
|
||||
existed must not be reset the moment this ships - that would throw away
|
||||
real, in-flight state on every existing instance's first call after
|
||||
upgrading."""
|
||||
run_budget._save_state({"test-session": {"count": 5, "recent": ["lint"]}})
|
||||
run_budget.record_and_check("lint", [], override=False)
|
||||
entry = run_budget._load_state()["test-session"]
|
||||
assert entry["count"] == 6
|
||||
assert entry["source"] == "WIKITOOL_SESSION_ID"
|
||||
|
||||
|
||||
def test_a_bucket_with_a_different_recorded_origin_starts_over():
|
||||
"""The same id string, stamped by a different origin than the one
|
||||
recorded, is treated as a stranger's bucket rather than inherited - the
|
||||
mechanism behind #110's 'no bucket is silently reinterpreted' criterion."""
|
||||
run_budget._save_state(
|
||||
{"test-session": {"count": 40, "recent": ["lint"], "source": "getppid() fallback"}}
|
||||
)
|
||||
run_budget.record_and_check("lint", [], override=False)
|
||||
entry = run_budget._load_state()["test-session"]
|
||||
assert entry["count"] == 1
|
||||
assert entry["source"] == "WIKITOOL_SESSION_ID"
|
||||
|
||||
|
||||
def test_a_bucket_with_the_same_recorded_origin_keeps_counting():
|
||||
run_budget._save_state(
|
||||
{"test-session": {"count": 7, "recent": ["lint"], "source": "WIKITOOL_SESSION_ID"}}
|
||||
)
|
||||
run_budget.record_and_check("lint", [], override=False)
|
||||
assert run_budget._load_state()["test-session"]["count"] == 8
|
||||
|
||||
|
||||
def test_save_state_writes_atomically_and_leaves_no_tmp_file(isolated_state):
|
||||
run_budget._save_state({"test-session": {"count": 1, "recent": []}})
|
||||
assert isolated_state.exists()
|
||||
@@ -228,3 +265,51 @@ def test_status_command_reports_count(capsys):
|
||||
run_budget.status_command()
|
||||
out = capsys.readouterr().out
|
||||
assert "Calls so far: 1" in out
|
||||
|
||||
|
||||
# --- gate reproduced across separate processes (Gitea #110) ---
|
||||
#
|
||||
# `isolated_state`'s in-process monkeypatching cannot exercise the actual bug:
|
||||
# `os.getppid()` is constant within one test process. These spawn a fresh
|
||||
# Python subprocess per call - the same shape as Claude Code's Bash tool,
|
||||
# which runs every `wikitool` invocation in a freshly initialised shell - so
|
||||
# the parent pid really does differ call to call, and only a harness variable
|
||||
# (standing in for `CLAUDE_CODE_SESSION_ID`) can hold the run together.
|
||||
# Before the fallback chain existed, both tests below would be green *and*
|
||||
# blind: every call landed in its own one-or-two-call bucket, and neither
|
||||
# gate could ever see enough of one session to trip.
|
||||
|
||||
def _spawn_call(tmp_path, monkeypatch, *, command="lint", args=(), override=False):
|
||||
monkeypatch.setenv("CHEMENU_ROOT", str(tmp_path))
|
||||
monkeypatch.setenv("CLAUDE_CODE_SESSION_ID", "harness-run")
|
||||
monkeypatch.delenv("WIKITOOL_SESSION_ID", raising=False)
|
||||
code = (
|
||||
"from chemenu.commands import run_budget\n"
|
||||
f"run_budget.record_and_check({command!r}, {list(args)!r}, override={override!r})\n"
|
||||
)
|
||||
return subprocess.run(
|
||||
[sys.executable, "-c", code],
|
||||
cwd=config._PACKAGE_ROOT / "tools",
|
||||
capture_output=True, text=True,
|
||||
)
|
||||
|
||||
|
||||
def test_the_iteration_budget_gate_trips_across_separate_shells(tmp_path, monkeypatch):
|
||||
for i in range(run_budget.DEFAULT_CALL_LIMIT):
|
||||
result = _spawn_call(tmp_path, monkeypatch, args=[f"--pass-{i}"])
|
||||
assert result.returncode == 0, result.stdout + result.stderr
|
||||
|
||||
result = _spawn_call(tmp_path, monkeypatch, args=["--one-too-many"])
|
||||
assert result.returncode != 0
|
||||
assert "Iteration Budget Gate" in result.stdout
|
||||
|
||||
|
||||
def test_the_loop_breaker_trips_across_separate_shells(tmp_path, monkeypatch):
|
||||
args = ["add", "--a", "X", "--b", "Y"]
|
||||
for _ in range(run_budget.DEFAULT_LOOP_WINDOW):
|
||||
result = _spawn_call(tmp_path, monkeypatch, command="xref", args=args)
|
||||
assert result.returncode == 0, result.stdout + result.stderr
|
||||
|
||||
result = _spawn_call(tmp_path, monkeypatch, command="xref", args=args)
|
||||
assert result.returncode != 0
|
||||
assert "Loop-Breaker" in result.stdout
|
||||
|
||||
@@ -145,6 +145,30 @@ def test_session_id_falls_back_to_the_parent_process(monkeypatch):
|
||||
assert "getppid" in session.session_id_source()
|
||||
|
||||
|
||||
def test_session_id_prefers_a_harness_variable_over_getppid(monkeypatch):
|
||||
"""The middle link of the chain (Gitea #110): a harness that sets its own
|
||||
session variable, but not WIKITOOL_SESSION_ID, still gets a stable id
|
||||
rather than falling all the way to the per-call parent pid."""
|
||||
monkeypatch.delenv("WIKITOOL_SESSION_ID", raising=False)
|
||||
monkeypatch.setenv("CLAUDE_CODE_SESSION_ID", "30d734d5-live-session")
|
||||
assert session.session_id() == "30d734d5-live-session"
|
||||
assert session.session_id_source() == "CLAUDE_CODE_SESSION_ID (claude-code)"
|
||||
|
||||
|
||||
def test_explicit_session_id_still_wins_over_a_harness_variable(monkeypatch):
|
||||
monkeypatch.setenv("WIKITOOL_SESSION_ID", "ingest-handbook/u2")
|
||||
monkeypatch.setenv("CLAUDE_CODE_SESSION_ID", "30d734d5-live-session")
|
||||
assert session.session_id() == "ingest-handbook/u2"
|
||||
assert session.session_id_source() == "WIKITOOL_SESSION_ID"
|
||||
|
||||
|
||||
def test_the_harness_id_is_used_unmodified_as_the_key():
|
||||
"""Decision 1 in #110: no prefix, no rewriting - the value has to be
|
||||
exactly what a harness's own hook writes into a trace, or the two would
|
||||
stop joining on the same string."""
|
||||
assert session.HARNESS_ENV_VARS == (("CLAUDE_CODE_SESSION_ID", "claude-code"),)
|
||||
|
||||
|
||||
def test_the_core_event_set_is_what_every_harness_can_produce():
|
||||
"""Guards the degradation rule: if a core event stops being available on one
|
||||
harness, this fails rather than the scorer silently reporting zero."""
|
||||
@@ -185,6 +209,25 @@ def test_a_reported_session_start_is_not_shadowed_by_a_synthetic_one(monkeypatch
|
||||
assert "synthesized" not in records[0]["attrs"]
|
||||
|
||||
|
||||
def test_wikitool_s_own_header_names_where_its_session_id_came_from(monkeypatch, tmp_path):
|
||||
"""Only the `wikitool` source resolves its own id through the fallback
|
||||
chain - naming that origin in its own header is what lets a trace say
|
||||
what it was keyed on, not just what the id happened to be (Gitea #110)."""
|
||||
monkeypatch.setenv("WIKI_TRACE_DIR", str(tmp_path))
|
||||
monkeypatch.setenv("CLAUDE_CODE_SESSION_ID", "harness-session")
|
||||
emit_mod.write_event("wikitool", "wikitool.call", {"command": "lint"},
|
||||
session="harness-session")
|
||||
header = read_lines(tmp_path / "harness-session" / "trace.jsonl")[0]
|
||||
assert header["attrs"]["session_origin"] == "CLAUDE_CODE_SESSION_ID (claude-code)"
|
||||
|
||||
|
||||
def test_another_source_s_header_carries_no_session_origin(monkeypatch, tmp_path):
|
||||
monkeypatch.setenv("WIKI_TRACE_DIR", str(tmp_path))
|
||||
emit_mod.write_event("copilot-cli", "tool.post", {"tool_name": "bash"}, session="v4")
|
||||
header = read_lines(tmp_path / "v4" / "trace.jsonl")[0]
|
||||
assert "session_origin" not in header["attrs"]
|
||||
|
||||
|
||||
# --- reading back ---
|
||||
|
||||
def test_a_trace_reads_back_in_time_order(monkeypatch, tmp_path):
|
||||
|
||||
Reference in New Issue
Block a user