wave6: the reference half of the coordinated pass (supersedes #77) - #78
Open
mjerris wants to merge 65 commits into
Open
wave6: the reference half of the coordinated pass (supersedes #77)#78mjerris wants to merge 65 commits into
mjerris wants to merge 65 commits into
Conversation
The multi-OS PACKAGE-SMOKE step invoked bare `python3`, which on the macOS runner
does NOT resolve to the interpreter actions/setup-python provisioned. Nightly run
30238061313, job macos-latest:
[FAIL] build: exit 1
/Library/Frameworks/Python.framework/Versions/3.12/bin/python3:
No module named build
Mechanism. `setup-python@v6` with python-version "3.12" provisions
/Users/runner/hostedtoolcache/Python/3.12.10/arm64 (the step's own log confirms
pythonLocation), and the earlier `pip install` step installed `build` THERE. But
the macOS image ships a pre-installed framework Python whose bin dir sits ahead of
the toolcache for the name `python3`, so bare `python3` selected
/Library/Frameworks/Python.framework/Versions/3.12/bin/python3 — a DIFFERENT
interpreter, without `build`.
The same run is the proof: the TEST step immediately above uses `python -m pytest`
(the setup-python shim) and passed, as did every `pip install`. Only the one step
spelled `python3` missed.
package_smoke.py is NOT at fault — it correctly uses sys.executable, so it
faithfully used whichever interpreter this line handed it, and it reported that
path verbatim in the error. Fixed by invoking `python` (setup-python's shim).
NOT fixed by `pip install build`: installing into the wrong interpreter would mask
the resolution bug rather than repair it, and would leave the step running an
interpreter nobody selected.
Sweep of the sibling workflows that call setup-python: doc-audit.yml had the only
other bare-`python3` `run:` lines (2). They are ubuntu-only, where setup-python
front-loads its dir so both names currently resolve to the toolcache — latent, not
live — but switched to the shim for consistency so the trap cannot activate if that
job ever gains a macOS runner. nightly.yml and live-smoke.yml call setup-python but
contain no bare `python3`, and are ubuntu-only.
Verified: `grep -n '\bpython3\b' .github/workflows/*.yml` now matches only
explanatory comments, no executable line. actionlint on both changed files reports
the same 6 pre-existing shellcheck info/style findings as before the change (all at
doc-audit.yml's untouched Summary step) — no new findings.
Known-latent, deliberately NOT changed here (needs an owner call): scripts/run-ci.sh
uses bare `python3` in 51 places. Both workflows that invoke it are ubuntu-only, and
that script also runs on developer machines where `python3` is the correct name and
`python` may not exist — so a blanket rename is a behavior change beyond this fix.
Corrects my own previous commit on this branch, which mis-attributed the failure to
interpreter resolution. Dispatching the workflow disproved that fix: with `python`
instead of `python3` the macOS job failed IDENTICALLY, one word different —
/Library/Frameworks/Python.framework/Versions/3.12/bin/python: No module named build
i.e. the SAME framework interpreter, reached via the other name. So on the
macOS/arm64 runner BOTH names resolve to the pre-installed framework Python;
setup-python does not win PATH there at all.
The real root cause, from the same log: `pip` is that framework Python's pip —
every dependency reports installing to
`/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages`.
The job is therefore CONSISTENTLY one interpreter, and there was never a mismatch
to fix. `build` is simply not installed, because **it is not a declared dependency
anywhere** — not in requirements-dev.txt, not in requirements.txt, not in
pyproject.toml. The gate relied on the runner image happening to ship it.
AGENT_RULES §7: a tool a gate needs is DECLARED, not assumed present. So:
requirements-dev.txt gains `build>=1.0.0`, and the step above already installs that
file. This is not "pip install build into the wrong interpreter" (which the brief
rightly forbids as masking a resolution bug) — there is no wrong interpreter here,
and declaring a real, undeclared dev dependency is the fix rather than the mask.
Note this gate has never passed for python on any OS: multi-os.yml is the ONLY
place PACKAGE-SMOKE runs for this port (nightly.yml and scripts/run-ci.sh do not
invoke it), so the "ubuntu ships build" path was never exercised either.
`python` (not `python3`) is KEPT, on its own merits: it is the same name `pip`
above pairs with, so the step provably runs the interpreter the deps went into, and
it stays correct if the image's PATH precedence ever changes. The workflow comment
is rewritten to state this true mechanism instead of the interpreter-mismatch story.
The doc-audit.yml python3→python change from the previous commit also stands — that
job is ubuntu-only and green either way; the shim is the consistent choice.
Windows remains red on this workflow for an unrelated, separately-assigned reason
(RED 3: four test-portability defects in the TEST step — WinError 32 on an unclosed
sqlite temp file, a hardcoded "/tmp/custom" assertion, a POSIX 0o755 mode check, and
cp1252 UnicodeEncodeErrors). That step fails before PACKAGE-SMOKE runs, so the
Windows job cannot confirm this fix either way.
…rmission bits, UTF-8 encoding
Four distinct cross-platform defects, all measured from nightly Multi-OS run
30238061313 (job windows-latest, step TEST: 36 failed / 2 errors / 5671 passed).
Two turned out to be product bugs, not test bugs.
1. sqlite handle outlives the temp file (PermissionError [WinError 32])
PRODUCT BUG. search/index_builder.py validate_index() closed its connection
only on the success path; the "Missing tables" early return and the except
both leaked it. Windows refuses to delete a file with a live handle, so the
fixture teardown's os.remove() raised. Fixed with contextlib.closing (NOT
`with sqlite3.connect(...)` — a Connection context manager commits but does
not close). Audited all 14 sqlite3.connect sites in search/ and found two
more unguarded on their error paths: migration.get_index_info() and
search_service._get_model_name(). Also fixed the test-side leak in
test_search_engine.py, where NamedTemporaryFile(delete=False) held its own
handle open while os.unlink ran inside the `with` block.
2. Hardcoded POSIX path assertion
str(Path) renders with the platform separator, so `== "/tmp/custom"` can
never hold on Windows (it yields "\tmp\custom"). Now compares Path objects.
Also removes the /tmp usage itself per the project rule (tmp_path instead).
Same fix in test_init_project.py::test_main_custom_dir, which failed the
same way via `'/tmp/custom' in 'D:\tmp\custom\testproject'`.
3. POSIX permission bits
Windows has no execute bit, so st_mode never carries 0o755. The two
observable-mode assertions are skipif(win32) with the reason recorded, and
the contract they were checking is now ALSO asserted platform-independently
(that chmod 0o755 is requested, and only for deploy.sh). POSIX coverage is
unchanged — nothing was deleted or weakened.
4. UnicodeEncodeError: 'charmap'
PRODUCT BUG. cli/dokku.py:1964 wrote generated files with write_text() and
no encoding, i.e. the platform default (cp1252 on Windows). The templates
embed box-drawing rules (U+2500/U+2550, 59-char runs — matching the log's
"position 130-188") and arrows, which cp1252 cannot represent. This caused 8
direct failures plus 4 more surfacing as generate() returning False
("Failed to generate project: 'charmap' codec can't encode..."). Fixed at
_write_file plus the two app.json reads. cli/init_project.py had the same
latent defect — 98 cp1252-hostile characters across 29 unguarded write_text
sites — fixed there too before it bites.
Proven on POSIX (not merely "looks right"):
- Defect 1: 3 new tests assert the platform-independent invariant (every
connection opened is closed, via a connect spy). Verified they FAIL against
the unfixed product code and pass with it.
- Defect 4: reproduced the Windows failure class on macOS with
`LC_ALL=C python -X utf8=0`, which yields
"'ascii' codec can't encode characters in position 2-80" — the same position
range as CI's charmap error. The 3 new UTF-8 round-trip tests fail without
the fix; with it, all 210 cli tests pass even under that hostile locale.
- Defects 2 and 3 are structural (Path comparison / platform skip) and remain
pending real confirmation on the Windows runner.
Full local gate run: `bash scripts/run-ci.sh` → CI PASS, exit 0 (37 gates).
Unit suite 5621 passed / 100 skipped on macOS.
Note: the Windows TEST step failing is what kept PACKAGE-SMOKE from running at
all on that job (TEST: failure -> PACKAGE-SMOKE: skipped). If TEST now passes,
PACKAGE-SMOKE will execute on Windows for the first time and may surface
unrelated failures.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GFKJhLvfV8yGrASwqxdgaf
The new test_deploy_script_round_trips_as_utf8 asserted
read_bytes().decode('utf-8') == read_text(encoding='utf-8'). Those two views
legitimately differ on Windows: text-mode writes translate \n -> \r\n and
read_text translates it back (universal newlines), so the assertion compared
raw CRLF against normalized LF and failed on the runner -- while the encoding
fix it was guarding was working correctly (every U+2550/U+2192/U+2705/U+1F310
round-tripped intact).
Line endings are not what the test is about. It now compares against the source
DEPLOY_SCRIPT_TEMPLATE line-by-line and asserts the non-ASCII character set
survives byte-for-byte, which is the actual regression being guarded. Still
verified to fail against the unfixed product code (all 3 tests in the class fail
under LC_ALL=C -X utf8=0 without the encoding fix).
Caught by Multi-OS run 30260346853 (windows-latest), which this branch
dispatched -- Windows TEST went 36 failed -> 16 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GFKJhLvfV8yGrASwqxdgaf
…gnal handlers, skills paths Closes the 15 Windows TEST failures left by PR #76 (which took the count 36 -> 15), measured from Multi-OS run 30261304144 / windows-latest. One of the three defect classes is a PRODUCT bug, not a test bug. 1. PRODUCT — RelayClient.run() was broken for every Windows user (7 tests) _run_forever() called loop.add_signal_handler(SIGINT, ...) unguarded on its FIRST statement. Loop-level signal handling is a Unix-only asyncio capability: both Windows event loops raise NotImplementedError unconditionally, so the exception escaped before connect() was ever reached — RelayClient.run() could not establish a RELAY connection on Windows at all. Guarded with a NotImplementedError fallback that degrades to KeyboardInterrupt-driven shutdown (Ctrl+C still stops the client; only the graceful _shutdown() handshake is lost). Also replaced the __import__("signal") inline with a module-level import. Covered by a new platform-independent regression test that forces the exact Windows condition (patching the loop method to raise what Windows raises) rather than skipping on win32, so the contract is exercised on every OS. Verified to FAIL against the unfixed product on macOS with the same NotImplementedError at client.py:684 as the Windows traceback. The 7th relay failure was a separate mechanism: the ping-loop test patched _EXECUTE_TIMEOUT=0.01 around client.connect(), putting the auth round-trip under a 10ms deadline. The handshake needs several event-loop turns, and 10ms is at/below the Windows asyncio timer granularity (~15.6ms clock tick), so connect() could time out before the loop ran the recv task. Scoped the patch to the pings the test is actually about. A deadline sweep on POSIX reproduces the identical "Request timeout for signalwire.connect" error deterministically (20/20) once the deadline drops below the turns required. 2. TESTS — POSIX-separator expectations (3 tests) test_schema_utils (2) and test_registry (1) compared product output to hardcoded POSIX literals. The product is separator-correct in all three cases: it returns str(Path(...)), composes with os.path.join(), and splits on os.pathsep. Build the expectation the same way the product builds the value (compare Path objects / computed joins) instead of a POSIX-only spelling. Confirmed under Windows path semantics (ntpath/PureWindowsPath) on POSIX: the old literals match only on POSIX, the new expectations match on both — so Windows keeps real coverage rather than losing it to a skip. 3. TESTS — claude_skills paths (5 tests) - WinError 267 ("directory name is invalid"): the test passed Path("/tmp") as the subprocess cwd. /tmp does not exist on Windows, so the spawn failed before the timeout could fire. Switched to pytest tmp_path. The command was also `sleep 10`, which is not a Windows shell builtin and exits immediately; replaced with a portable python -c sleep so the timeout path is genuinely exercised (test now takes ~1.01s, i.e. it really blocks). - 8.3 short-path mismatches (3 tests): setup() canonicalizes skills_path via .resolve(), which expands the Windows 8.3 temp dir to its long form (RUNNER~1 -> runneradmin). The tests compared an unresolved tempfile path to resolved product output. Resolve both sides. - Removed all 15 hardcoded /tmp uses from this file (a standing project rule bans /tmp outright); they were inert placeholders except where noted above. No test was blanket-skipped: every disposition is either a product fix or an expectation corrected to match platform-correct product behavior. Verification: bash scripts/run-ci.sh exit 0, all gates PASS (TEST, FMT, LINT, TYPECHECK, DRIFT, SPEC-PARITY, REST-COVERAGE, ...); tests/unit 5710 passed, 3 skipped, 0 failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GFKJhLvfV8yGrASwqxdgaf
`strip_control_chars` never used its `logger` or `method_name` parameters — they existed only to satisfy structlog's `(logger, method_name, event_dict)` processor calling convention. That made the recorded public contract describe structlog plumbing rather than what the function does, and asked every port to carry two permanently-dead parameters. The public function is now `strip_control_chars(event_dict)` — one parameter, the thing it actually uses. A private `_as_processor` adapter supplies the structlog protocol at the two registration sites (the structlog chain and the ProcessorFormatter chain). The adapter is private, so it adds no port surface. Adapters wrapping the same transform compare equal, which lets a test assert chain membership. Behaviour is unchanged: control characters are still stripped from log event values at both registration sites. Tests (tests/unit/core/test_logging_config.py::TestStripControlChars) exercise the REAL configured chain rather than the function in isolation, and were verified to fail against two deliberately broken adapters — a no-op adapter body, and dropping the adapter from only the ProcessorFormatter site (which the end-to-end output assertion alone cannot see, since the structlog chain strips first). `_drop_internal_keys` has the same 3-parameter shape but is private and therefore not port surface; it is deliberately left unchanged.
Per the owner's ruling: this is ONE wave of changes on ONE set of PRs. PR #77's fixes belong in the wave-6 PR, not a separate one, so its tip is merged here rather than landed independently. WHAT COMES IN (6 commits, 2 of them merges): 7b7b35a fix(ci): multi-OS must use the interpreter setup-python provisioned d8f9966 fix(ci): declare `build` — PACKAGE-SMOKE never had the module it needs 95e542b fix(tests): Windows portability — sqlite handles, path assertions, permission bits, UTF-8 0f5d0ee fix(tests): compare UTF-8 round-trip to the template, not a second read + the two merge commits that assembled them These close python's two main-branch Multi-OS reds: macOS PACKAGE-SMOKE — `build` was never declared in requirements-dev.txt/requirements.txt/ pyproject.toml; package_smoke.py::plan_python runs `sys.executable -m build` and relied on the runner image shipping it. Deterministic, not a race. This gate had NEVER passed for python on any OS. Windows TEST (36 failed) — four defects, TWO OF THEM PRODUCT BUGS in shipped SDK source: unclosed sqlite handles -> WinError 32; write_text() without encoding -> cp1252 UnicodeEncodeError on box-drawing chars; plus hardcoded POSIX path assertions and POSIX permission bits. Zero line-ending involvement — NOT the dotnet CRLF family. WHY THIS MERGE WAS NEEDED AT ALL: the branches had diverged by exactly one commit each — wave6 carried b1ab620 (strip_control_chars) which #77 lacked, and #77 carried the six above which wave6 lacked. `git merge-base --is-ancestor` confirmed it was not fast-forwardable. So merging #77 to main alone would NOT have greened the coordinated pass; the wave branch needed them too. NOTE FOR THE REVIEWER: 95e542b touches REFERENCE SDK SOURCE (5 files). Under the spine that re-drifts all nine ports, so the ports' gates must be re-run against the regenerated oracle before this wave merges. RESIDUAL RISK, stated plainly: no CI run has ever exercised the combined tip. Windows was proven on one branch and macOS on another. Once Windows TEST passes, Windows PACKAGE-SMOKE runs for the FIRST TIME EVER and may surface something new. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
…uite
Four checks porting-sdk DEFINES for python and this script never scheduled, so they had never run
against the reference. TLS-VERIFY and CA-VAR are SECURITY properties; the reference should not be
the one port where they go unchecked.
BURNED TO ZERO BEFORE WIRING, per the standing rule. Each run standalone against the current tree
first — all four already PASS, so this adds coverage without adding a red:
python CA-VAR PASS
python TLS-VERIFY PASS
python SECRET-SCRUB PASS
python LEDGER PASS (SUPPRESSION-LEDGER + IGNORE-LEDGER-VERIFY, 2 rules)
TIER: BLOCKING (per-PR), measured not assumed — 0s, 0s, 0s and 2s respectively. Nothing here
approaches the cost that would justify the nightly tier, where a regression sits unseen until the
next scheduled run.
A CORRECTION TO THE AUDIT THAT FOUND THIS: the fleet gate-wiring audit listed SUPPRESSION-LEDGER
among python's missing BEHAVIOURAL rules. It is not one — `behavioral.py --rules SUPPRESSION-LEDGER`
errors with "unknown rule id(s)" and lists the 14 rules python actually knows. SUPPRESSION-LEDGER
belongs to the LEDGER suite (suites/ledger.py), which python was not scheduling at all. Every other
port already schedules it; python was the only gap. Wired as the suite rather than as a lone rule,
matching how the other nine do it.
python schedules gates individually rather than through a BEHAVIORAL suite line, so these are four
new sched_gate entries rather than additions to a --rules list.
Verification: each gate run exactly as run-ci.sh now invokes it (same script, same --port/--repo,
same --rules), all PASS. `bash -n scripts/run-ci.sh` clean.
Found by the fleet-wide gate-wiring audit (task #55); completes that item's tier-1 across
perl 0db0b55, cpp 54abe7f, ruby dde0516, java 63acebd and this commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
… is refused OWNER-RULED TWICE (2026-07-27 as task #56 option (b), re-confirmed 2026-07-29): a tool registered `secure=True` MUST REQUIRE a `__token`. Accepting the call when the token is ABSENT is a security defect, not the contract. A flag named `secure` that permits unauthenticated calls is a trap. THE DEFECT. core/agent_base.py:1413-1415 read: # Validate security token if present. token = request.query_params.get("__token") or request.query_params.get("token") if token: The ENTIRE validation block sat inside that conditional, so omitting the parameter skipped the check and the tool ran. Refusal only ever happened on a present-but-wrong token — an attacker did not need to forge anything, only to leave the parameter off. WHAT THAT MEANT IN PRACTICE, from the pre-fix test output: a `secure=True` tool invoked with NO token returned `{"response":"SECRET-BALANCE-9999"}` — the guarded payload, straight back over the wire. THE FIX. Entry into the validation block is now "a session manager exists and the function is registered"; an absent token is simply one way to FAIL validation rather than a way to SKIP it. Two things deliberately left alone: * THE `secure` PREDICATE IS THE EXISTING EXPRESSION, REUSED, NOT DUPLICATED (`func_entry.secure if hasattr(...) else func_entry.get("secure", True)`). A non-secure tool still runs with no token — there are two tests pinning exactly that, because getting this wrong would break every insecure tool in the fleet. * THE REFUSAL SHAPE IS UNCHANGED: the same FunctionResult dict at HTTP 200 as the invalid-token path. No status code was invented. mod_openai has NO handling for a SWAIG refusal status (grep for "invalid or expired" / "security token" across its .c files returns nothing), so the tool reports it cannot execute and the model relays that. A test asserts BYTE-EQUALITY of the absent and invalid refusals so no port can later conclude one of them may be an HTTP error. Verification — real HTTP through FastAPI TestClient against a real AgentBase with a real SessionManager minting genuine HMAC tokens. Nothing on the token path is stubbed. BEFORE (source stashed, tests in place): 4 failed, 5 passed test_absent_token_is_refused AssertionError: {"response":"SECRET-BALANCE-9999"} test_absent_token_does_not_leak_the_secure_payload test_empty_token_is_refused test_absent_and_invalid_refusals_are_the_same_shape The 5 that already passed are cases (i) valid-accepted, (ii) invalid-refused and both non-secure cases — which is what proves case (iii) was not bought by breaking the others. AFTER: 9 passed. FULL SUITE: 5730 passed / 3 skipped -> 5734 passed / 3 skipped, ZERO failures. The +4 is exactly the four assertions that were red. ruff format --check signalwire · ruff check signalwire · mypy (359 files) — all clean. I CORRECTED THE LANE'S BASELINE CLAIM. It reported "9 failed / 5774 passed" at 81d412e and attributed the failures to a pre-existing structlog-on-stdout defect in tests/test_examples.py. That is a measurement artifact: `git stash` does not remove an UNTRACKED file, so its own new test file stayed on disk and ran against the stashed-out source. Measured properly, the clean-tree baseline is 5730 passed / 3 skipped with no failures, and the after-state is 5734 with none. ONE FURTHER BEHAVIOURAL CHANGE, DELIBERATE: a PRESENT token with `call_id is None` previously skipped validation entirely; it now refuses for secure tools, because a token that cannot be validated is not a validated token. No existing test depended on the old behaviour. NOT CHANGED, FLAGGED INSTEAD: tests/unit/.../test_web_mixin.py:1718 test_valid_function_name_passes registers a dict with no `secure` key (so `.get("secure", True)` makes it secure) and posts with no token. Its behaviour genuinely changed — it now receives the refusal — but it still passes, because its assertion is `if hasattr(result, 'status_code'): assert result.status_code != 400` and a refusal dict has no status_code. That vacuity is pre-existing and the test's real subject is function-name validation, so rewriting it would be scope creep. SEPARATE DEFECT FOUND, NOT FIXED HERE — WORTH ITS OWN LANE: the SERVERLESS SWAIG path has NO token validation at all. core/mixins/serverless_mixin.py:224 _execute_swaig_function checks the function name and registry membership, then dispatches; no token is ever read. So `secure=True` is unenforced under lambda / cloud-function / azure. It is a different code path from _swaig_pre_dispatch and was outside this change's scope. CONSEQUENCES FOR THE COORDINATED PASS: this is REFERENCE SOURCE, so both oracles need regenerating and all nine ports re-drifting. porting-sdk 04e24f2's `token_absent` corpus golden is derived LIVE from the reference and will move with this change automatically — it must never be hand-edited. Note cpp already fails CLOSED (403) and is therefore now closer to the contract than the reference was; six ports (go, java, php, ruby, perl, dotnet) still never validate at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
…valid, not merely ignored
OWNER-RULED 2026-07-29: "if the server doesn't read them, remove them."
THREE INDEPENDENT SOURCES AGREE THAT `body` ON A WEBHOOK IS WRONG:
* THE SPEC FORBIDS IT. porting-sdk/schema.json $defs/Webhook declares exactly ten properties —
error_keys, expressions, foreach, headers, input_args_as_params, method, output, params,
require_args, url — under `unevaluatedProperties: {"not": {}}`. `body` is not among them, so
emitting it is a SCHEMA VIOLATION, not a harmless extra.
* THE ENGINE NEVER READS IT. mod_openai/actions.c:735-739 and bedrock.c:4920-4926 each read
url, method, form_param, `params`, `headers` and nothing else. Stronger than that:
`grep -n '"body"'` across BOTH files returns ZERO matches, so `body` appears nowhere in
mod_openai at all.
* THE HELPER SILENTLY DISCARDED THE CALLER'S DATA. create_simple_api_tool accepted `body=` and
forwarded it to DataMap.body(), which wrote a key nothing consumes.
REPRODUCED BEFORE FIXING, not inferred:
create_simple_api_tool(..., method='POST', body={'query':'Q'})
-> webhook KEYS: ['body', 'method', 'output', 'url']
An invalid key on the wire carrying the caller's payload into a void.
AFTER: KEYS: ['method', 'output', 'url']; `body` is absent from the signature; passing `body=`
raises TypeError.
ALSO CORRECTED — A FALSE DOCSTRING THAT HAS ALREADY PROPAGATED. params() described itself as an
"alias for body". It is not: the two write DIFFERENT KEYS and only `params` is ever read. That
sentence has been copied verbatim into signalwire-cpp/include/signalwire/datamap/datamap.hpp:90,
which is plausibly how cpp's datasphere skill picked the wrong method (fixed earlier today in
cpp 07054db). The docstring now states the distinction and cites the schema and the engine readers.
The module docstring example at the top of data_map.py taught `.body(...)`; it now teaches
`.params(...)`. docs/api_reference.md drops `body` from the signature line and the parameter list.
SCOPE HELD DELIBERATELY: `DataMap.body()` — the public BUILDER METHOD — is NOT removed here. The
ruling was about "that call", i.e. the helper's parameter. Removing the builder is a larger,
separately-breaking change and belongs to its own decision. Measured cost if it goes: it is genuine
port surface recorded in python_signatures.json and implemented by ALL NINE PORTS (`Body` in
go/dotnet, `body` elsewhere) — roughly 24 test call sites, 7 example call sites, 18 doc references
and 6 internal factory forwards across 10 repos, plus an oracle regen and 9 port PRs. After this
commit, NOTHING in the reference's production code calls DataMap.body(): the discard site deleted
here was its only internal caller. Remaining reference references are 3 tests and 3 doc examples.
NOTE FOR THE BATCHED PORT PASS: each port's own create_simple_api_tool equivalent carries the SAME
body-forwarding bug, so the ports must be touched regardless of how the builder-method question is
ruled.
Verification:
BEFORE 5734 passed / 3 skipped / 0 failures (verified independently on the clean tree)
AFTER 5736 passed / 3 skipped / 0 failures — the +2 is exactly the two new tests.
ruff format --check signalwire -> exit 0, 215 files already formatted
ruff check signalwire -> exit 0, All checks passed
mypy --config-file pyproject.toml -> Success: no issues found in 359 source files
No oracle regenerated and no port touched — both are deliberately batched into the single
coordinated pass alongside 7c2f253 (secure=True requires a token).
A SECOND RULED ITEM TURNED OUT TO BE ALREADY DONE: task #140 (strip_control_chars becomes a real
1-param function with a structlog adapter) was landed by the owner on 2026-07-28 as b1ab620.
logging_config.py:33 is already `def strip_control_chars(event_dict)`, the private `_as_processor`
adapter supplies the protocol at both registration sites (:205, :233), `_drop_internal_keys` was
correctly left alone, and tests exist at tests/unit/core/test_logging_config.py:416-510. No change
was made for it here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
…counting private symbols porting-sdk 481b435 corrected DOC-SURFACE's python decl regex, which matched a LEADING UNDERSCORE and so counted `_helper`, `__init__` and every dunder as public surface — 511 of 1539 symbols, a third of the denominator. Excluding them the way go/typescript/java/cpp already do, coverage is 70.6% (726/1028), not 64.5% (993/1539). This RAISES the bar. The gate had been RED since before today (64.5% vs a 64.7% floor) and report-only, so nobody saw it; the drop traced to cdd0b17 (ai_chat) and b1ab620 (strip_control_chars), and 9 of the 12 symbols that caused it were private. The floor now describes the corrected measurement — leaving 64.7 would have pinned a number the gate no longer produces. Gate exit 0 at the new floor. Full rationale and the per-port before/after in 481b435. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
Every typed RELAY event wrapper's `from_payload` classmethod now documents which wire
event it parses and which fields it lifts out of `params`. These are what nine porting
teams read to know the RELAY event contract, and the non-obvious mappings are exactly what
was undocumented:
CallReceiveEvent `context` falls back to the payload's `protocol` field — older RELAY
servers name the same value `protocol`
RecordEvent url/duration/size read from the nested `record` object when present
and from the top of `params` otherwise, because RELAY reports them in
either position depending on event stage
CollectEvent `final` stays None when the payload omits it — absent is not False
DialEvent correlated by `tag`, not `control_id` like the other operations
ConferenceEvent conference-scoped, so the inherited `call_id` may be empty
MessageStateEvent carries `reason`, which is what explains a failed `message_state`
TranscribeEvent reads its artifact fields only from the top level — unlike RecordEvent
there is no nested object
DOCS ONLY — no code, signature or behaviour changed.
Verified: 490 relay tests pass; ruff format --check and ruff check clean; file parses.
DOC-SURFACE for this file goes from 24 undocumented to 0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
Adds real docstrings to the 12 undocumented public symbols in the REST
client's shared base, the layer every generated resource inherits and the
nine porting teams read as the contract.
HttpClient raw HTTP verbs (get/post/put/patch/delete): each documents that
`path` is an absolute API path appended to the client's scheme://host base,
what is serialised where (params -> query string, body -> JSON), the {}
return for 204/empty bodies, and the SignalWireRestError /
SignalWireRestTransportError split. Records the idempotency asymmetry read
from status_is_retryable: GET/PUT/DELETE retry on the full retry_on_status
set, while POST/PATCH retry only on a transport error or a 429/503 throttle.
Notes which verbs accept no query params (put/patch/delete).
ReadResource/CrudResource/CrudWithAddresses resource CRUD (list/get/create/
update/delete/list_addresses): documents path composition from the
resource's own base_path (vs the caller-built absolute path the HttpClient
verbs take), so the two `get`s and the two `delete`s are no longer
conflatable. Records that `list` returns ONE raw page and does not follow
pagination links (paginate() does); that create/update send kwargs as the
JSON BODY where list sends them as the QUERY STRING; that update dispatches
on _update_method (PATCH default, PUT under FabricResourcePUT); that
resource delete is typed TItem but SignalWire delete endpoints answer 204,
arriving as {}; and that list_addresses hits the nested sibling collection
<base_path>/<id>/addresses and is untyped Any, unlike list's TList.
_AbortSignal.is_set: documents that cancellation is cooperative and polled
only BETWEEN attempts, so a request already on the wire is not interrupted.
Measured with the AST gap checker (porting-sdk 62d0e64): the two files in
scope went 12 gaps -> 0. No code, signature or behaviour changed; docs only.
No defects found while reading.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
Closes every DOC-SURFACE gap in the `search` lane. Measured with the AST
gap tool (public = no leading underscore, documented = ast.get_docstring):
18 gaps before (search_service.py 11, __init__.py 6, document_processor.py 1),
0 after.
The bulk of this lane is the OPTIONAL-DEPENDENCY structure, which was
entirely undocumented and is easy to misread:
- search/__init__.py 117-146: preprocess_query, preprocess_document_content,
DocumentProcessor, IndexBuilder, SearchEngine and SearchService in the
`else` branch are FALLBACK STUBS, bound only when numpy / scikit-learn /
sentence-transformers / nltk are missing. Importing them always succeeds;
calling or constructing one always raises ImportError naming the missing
packages. Each stub now says so and points at the real symbol it shadows.
- search_service.py: SearchRequest / SearchResult / SearchResponse are
defined TWICE — pydantic BaseModel subclasses when pydantic imported, and
plain __init__-assignment classes when it did not. Both sets are now
documented per-field, and each fallback says it performs no validation and
is only reachable via search_direct (no FastAPI means no HTTP route).
- add_security_headers / validate_host: the two `@app.middleware("http")`
handlers — header stamping (https-conditional) and Host-header allowlisting
returning a bare 400.
- get_authenticated / search / health: the auth-scheme provider and the two
route handlers, including that /health is registered WITHOUT the security
dependency and masks the pgvector connection string to "***".
- document_processor.py flush(): the closure inside _chunk_markdown_ast that
emits an accumulated chunk with hierarchy/section/line-range metadata and
resets the accumulator, dropping whitespace-only accumulations.
Docs only — no code, signature or behaviour change.
Verified: pytest 5785 passed / 9 failed (the pre-existing test_examples
JSON failures) / 9 skipped; ruff format --check 0; ruff check 0;
mypy Success (359 source files).
Closes the entire `cli` DOC-SURFACE lane. Docstrings written from reading each body; no code, signature or behaviour changed. cli/core/agent_loader.py (2) mock_serve, mock_run — the monkeypatches installed over SWMLService.serve()/ run() (and AgentBase's) while a module's main() is called, so loading an agent file for inspection configures its service without starting a web server; the receiver is captured for the caller and all arguments are ignored. cli/output/swml_dump.py (1) suppressed_print — installed as builtins.print so loaded agent code cannot contaminate stdout during a SWML dump; calls naming an explicit non-stdout file are forwarded to the saved original, the rest are discarded. cli/simulation/mock_env.py (8) MockQueryParams.get/items/keys/values and MockHeaders.get/items/keys/values — two distinct stand-ins for FastAPI request objects in serverless simulation. Documented the behavioural split: MockQueryParams matches keys exactly and case-sensitively, while MockHeaders lowercases on both construction and lookup, so its items()/keys() yield lowercased names and the caller's original casing is not recoverable. All four view methods return live dict views. cli/dokku.py (10) Colors, print_step/success/warning/error/header, prompt, prompt_yes_no, generate_password, main. Noted that print_error writes to stdout and neither raises nor exits; that prompt_yes_no treats any unrecognized input (a typo) as NO rather than re-prompting; that generate_password draws from secrets.token_urlsafe (OS CSPRNG) over the base64url alphabet and TRUNCATES to `length`, retaining ~6*length bits rather than 8*length. main documents the five subcommands (init/deploy/logs/config/scale) and app-name inference. cli/init_project.py (5) Colors, print_step/success/warning/error — sw-agent-init's own copies. Each Colors docstring states which CLI it belongs to and that the dokku.py copy is a separate class (dokku's additionally has MAGENTA), so neither is mistaken for a shared module. Measured (porting-sdk AST gap counter, run from this worktree): before 114 real gaps / 30 files -> after 88 / 25 files (-26, all in scope). Verification: pytest 5785 passed, 9 skipped, 9 failed (the pre-existing tests/test_examples.py "Invalid JSON" failures, unrelated to this change); ruff format --check and ruff check exit 0; mypy Success (359 source files). Nothing in scope left undone. No defect found in the code read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
Adds real docstrings to every remaining DOC-SURFACE gap in signalwire/livewire/__init__.py, the LiveKit-compatible API layer. Each explains the LiveKit-side concept AND what it maps to on SignalWire, which is what a migrating user and the porting teams need. Documented (repeated names sit on different classes): _NoopTracker.was_logged / .reset (:116, :120) ChatContext.append (:164) RunContext.userdata getter (:271) Agent.session getter/setter (:346, :350) AgentSession.userdata getter/setter (:505, :509) AgentSession.history getter (:513) handler closure in _register_function_tool (:626) Measured with the AST checker (porting-sdk 62d0e64): fleet gaps 114 -> 104; livewire/__init__.py 10 -> 0. Behaviour worth recording, found while reading: - AgentSession.history is initialized empty and never appended to anywhere in the SDK; the platform owns the transcript. Documented as such rather than implying it fills. - RunContext.userdata reads through to the session, but the handler built by _register_function_tool always constructs RunContext(session=None), so tool handlers get the empty-dict fallback. - ChatContext.append stores its `text` kwarg under the `content` key. - The tool handler calls fn synchronously, so an async tool function would be stringified as an un-awaited coroutine. Docs only -- no code, signature or behaviour changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
…ecurity middleware) Docs-only. No code, signature or behaviour changed. ai_chat/client.py (4): the three public response models ConversationInfo / ChatResponse / ChatLog now document what each field carries and where it comes from (notably that `id`/`conversation_id` are echoed from the request argument, not read from the response), and `close` says it only closes a client-owned session and that the client stays usable — the next request lazily rebuilds one. relay/call.py (6): is_done/stop/pause/resume/volume are on the Action handle hierarchy (Action -> StoppableAction -> PausableAction -> VolumeAction), NOT on Call; each now names the `calling.<prefix>.<cmd>` command it posts and the state it requires. Two facts taken from the engine (mod_infrastructure/relay_apis.c): `pause(behavior=)` is a closed enum "skip,silence" accepted only on record.pause (play.pause has no behavior field), and `volume` is gain in DECIBELS validated to [-40, +40] and required — not a 0-to-1 multiplier. `rank` is the lifecycle-order helper inside _wait_for_state; documented incl. the -1 unknown-state sentinel. pom/pom.py (2): build_section and recurse are nested closures, not rendering internals — build_section is the from_json/from_yaml validator that builds the Section tree (documented incl. every ValueError it raises), recurse is find_section's depth-first exact-title search. web/web_service.py (3) + mcp_gateway/gateway_service.py (3): the security middleware now states the exact headers set. web_service uses SecurityConfig.get_security_headers (nosniff / DENY / XSS / Referrer-Policy, HSTS only on https AND when enabled); mcp_gateway hardcodes its own set which adds a `default-src 'none'` CSP, omits Referrer-Policy, and has a non-configurable HSTS. validate_host rejects with 400 unless the host is in allowed_hosts — with two documented pass-throughs: a missing Host header is not checked at all, and "*" in allowed_hosts permits everything. `decorated` is _check_auth's wrapper (Bearer then Basic, both hmac.compare_digest, 401 + WWW-Authenticate on failure); `handle_error` is the catch-all that returns a fixed opaque 500 and never leaks the exception. Measured coverage (AST: public = no leading underscore, documented = ast.get_docstring): 114 -> 96 real gaps fleet-wide; all 18 in scope closed, zero left in these 6 files. Verified: pytest 5785 passed / 9 failed (only the pre-existing tests/test_examples.py "Invalid JSON" set) / 9 skipped; ruff format --check, ruff check, and mypy all exit 0. Brief corrections: pom's build_section/recurse were briefed as "rendering internals" — they parse/validate and search respectively, nothing renders. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
Closes every DOC-SURFACE gap under signalwire/signalwire/core/. All 17 are
nested/inner functions (route handlers, middleware, decorator inner layers,
closures) that the AST measure counts as public surface. Measured with the AST
gap script: 114 real gaps fleet-wide -> 97; core/ 17 -> 0.
Documented, with the wire/security behaviour read out of each body:
- auth_handler.get_fastapi_dependency.auth_dependency — bearer-then-basic,
first match wins, secrets.compare_digest; the api_key parameter is accepted
but NEVER consulted on this path; failure = HTTPException(401,
"Invalid authentication credentials") + WWW-Authenticate: Basic even when
bearer is the configured scheme; optional=True returns
authenticated=False instead of raising.
- auth_handler.flask_decorator.decorated — bearer -> API-key header
(X-API-Key or security_config.api_key_header) -> Basic; failure RETURNS a
401 Response with body "Authentication required" and
WWW-Authenticate: Basic realm="SignalWire Service" (returns, not raises).
- security/webhook_middleware.make_webhook_validation_dependency.dependency —
raw body captured before any parser and stashed on request.state.raw_body;
every rejection path (non-UTF-8 body, missing header, bad signature) raises
the same bare HTTPException(403) with no body detail; the injected response
arg is unused because a returned Response does not short-circuit a
dependencies=[] entry.
- swml_service.as_router.handle_swaig — the /swaig endpoint available on ANY
SWMLService; documents the handler's status codes (401/415/413/400).
- swml_service.serve.handle_all_routes and mixins/web_mixin's two
handle_all_routes — the with/without-trailing-slash recovery and the
serve()-vs-get_app() split (get_app's variant only classifies and returns
204; serve's variant is what actually dispatches /swaig, /post_prompt,
/check_for_input, /debug_events and the routing callbacks).
- swml_service.make_verb_method / swml_builder.make_verb_method — per-verb
closure, None-valued kwargs dropped before the wire, sleep excluded
(bare integer, not an object); service returns add_verb's bool (False on
schema-validation failure), builder returns self for chaining.
- agent/tools/decorator's inner_decorator and decorator — how a python
signature becomes a SWAIG tool: kwarg pop-list, name/description fallback
order, type-inference handoff and the typed-handler wrapper; the class
decorator only stamps _is_tool/_tool_name/_tool_params for deferred
registration, so inference does not run at class-definition time.
- agent/tools/type_inference.create_typed_handler_wrapper.wrapper — the
(args, raw_data) calling convention adapter; no validation or coercion.
- agent_base.enable_sip_routing.sip_routing_callback — always returns None on
every branch, so it never emits the 307 the routing contract allows; the
username match is observational logging only.
- config_loader.substitute_vars.replacer — ${VAR} / ${VAR|default} expansion;
missing var with no default yields empty string, never an error.
- mixins/web_mixin's two add_security_headers — nosniff / DENY /
strict-origin-when-cross-origin always; HSTS only when SSL is on.
- mixins/web_mixin.setup_graceful_shutdown.signal_handler — SIGTERM+SIGINT,
cleanup body is a no-op placeholder, always sys.exit(0), no request drain.
Notable behaviour found while reading (documented, NOT changed — docs-only
lane): the three "path not found" / "invalid route" catch-all responses in
swml_service.serve and web_mixin return FastAPI's default 200 status with an
error JSON body rather than a 404, so a caller must inspect the body to
detect a miss. Ports mirroring these handlers should be aware this is the
current reference behaviour.
Verification: pytest 5785 passed / 9 failed (only the pre-existing
test_examples "Invalid JSON" cases) / 9 skipped; ruff format --check exit 0;
ruff check exit 0; mypy Success (359 source files).
The shared mock_relay fixture probed the HTTP health endpoint and, on a 200, returned
immediately — treating that as proof the server was usable. But HTTP and WS are two
DIFFERENT ports (HTTP defaults to WS+1000, 9773 vs 8773). A foreign process owning the
HTTP port makes the probe green, so the fixture skips spawning its own server, and every
test then dies with ConnectionRefusedError on a WS port nothing is serving.
OBSERVED LIVE, not theorised: during the parallel docs burn a sibling lane's mock_relay
held the fixed ports and exited between one lane's probe and its WS connect. That lane saw
2 failures + 6 errors in tests/unit/relay/test_connect_mock.py, all ConnectionRefusedError
on 8773, from a diff that touched only livewire/__init__.py. It correctly refused to call
that a flake and traced it to conftest.py:349.
THE FIX: _probe_health now takes the ws_port and additionally opens a TCP connection to it.
A mock is only 'already up' when BOTH the endpoint the health check uses AND the socket the
tests actually connect over are alive.
PROVEN WITH A NEGATIVE CONTROL rather than asserted — an HTTP-only squatter answering 200
on 9773 with nothing on 8773:
probe(http only) -> True <- the old logic: 'reuse it', then every test refuses
probe(http + ws 8773) -> False <- the new logic: spawn our own
and tests/unit/relay/test_connect_mock.py -> 11 passed.
This is the fixed-mock-port hazard CLAUDE.md warns about. The deeper fix — bind an ephemeral
port instead of hardcoding 8773 — is a larger change to a shared fixture and is NOT done
here; this stops the silent misdetection, which is the part that turns a port collision into
a mystery test failure in an unrelated lane.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
Documents every AST-measured DOC-SURFACE gap under signalwire/signalwire/skills/,
written from reading each body:
claude_skills/skill.py (5)
replace_command - shell-injection substitution; returns stdout, or a
bracketed [command timed out/error: ...] placeholder
replace_indexed - expands $ARGUMENTS[N]; out-of-range erases to ""
replace_shorthand - expands $N; same lookup, applied after the bracketed
pass so an expanded $ARGUMENTS[N] tail can't re-match
make_handler - per-skill closure factory (why a factory: else every
handler in the loop sees the last skill)
handler - section-vs-body selection then shell/variable/argument
substitution and prefix/postfix wrapping
info_gatherer/skill.py (5)
get_parameter_schema - the three params added on top of SkillBase's
get_instance_key - overrides the base tool_name keying to key on `prefix`,
which is what actually differentiates two instances
setup - what it validates; returning False = skill not loaded
get_global_data - initial namespaced questionnaire state
register_tools - the two prefixed tools and the toggle_functions on
completion
google_maps/skill.py (1) GoogleMapsClient - Places/Routes wrapper; both methods
log-and-return-None rather than raising
mcp_gateway/skill.py (1) handler - per-(service, tool) closure forwarding to
_call_mcp_tool
registry.py (1) add_skill_to_schema - attributes read, AttributeError
treated as empty schema, other exceptions swallowed so
one bad skill can't abort the scan
MEASURED (AST gap script, from worktree root):
before 114 gaps / 13 under skills/
after 101 gaps / 0 under skills/
Docs only - no code, signature or behaviour changed. Nothing left undone in
scope; no defects found in the code read.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
f601032 added `import socket` at module scope while _ws_port_accepting already imports it locally, matching the file's existing style (_probe_health imports requests the same way). Ruff flagged it F401 unused. Verified the file is back to its exact pre-change lint baseline: 24 errors both at 83a9833 and now — these are pre-existing findings in this conftest, none introduced by the probe fix. tests/unit/relay/test_connect_mock.py -> 11 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
…l is documented
BURN COMPLETE. 70.6% -> 100.0% (1102/1102), zero remaining gaps.
The move splits into two halves, and only the second half is documentation:
MEASUREMENT (70.6 -> 89.7, no docstrings written). Three independent defects in the
gate, all found by trying to burn against its output:
porting-sdk 481b435 counted _private + dunders as public surface (511 of 1539)
porting-sdk f8f58a2 could not see a docstring behind a WRAPPED SIGNATURE (161 false
negatives — every one already carrying full Args:/Returns:)
porting-sdk 62d0e64 matched def/class INSIDE STRING LITERALS — scaffolding templates
and docstring examples (24 phantoms). Superseded the other two by
measuring python with the AST instead of regex.
The first file opened to start writing (core/function_result.py, nominally 16 missing)
turned out to be fully documented already. That is what stopped the burn and sent the
work at the gate instead.
DOCUMENTATION (89.7 -> 100.0, 114 real symbols). One commit here + seven lanes run in
parallel git worktrees:
relay/event.py 24 83a9833 every from_payload event parser
cli 26 3e69e8d
search 18 508d75e
misc 18 d460ff4
core 17 aed6e78
skills 13 c28213e
rest/_base 12 efbf3cf
livewire 10 4f43f76
WHAT THE LANES FOUND WHILE READING — the reason to write these by hand rather than
generate them. Each was read out of the source, not inferred:
* REST retry is ASYMMETRIC: GET/PUT/DELETE retry on the full retry_on_status set, but
POST/PATCH retry ONLY on transport errors and 429/503, never 500/502/504. The code
calls it "part of the pinned contract"; it was documented on none of the verbs.
* Resource delete is typed TItem but the endpoints answer 204, so _request returns an
empty dict — the type signature actively misleads.
* relay volume is GAIN IN DECIBELS, engine-validated to [-40,+40], not a 0..1
multiplier (from mod_infrastructure/relay_apis.c). A port modelling it as 0..1 is
wrong.
* The pause behavior argument is a closed enum accepted ONLY on record.pause;
call_play_pause declares no such field, yet it is shared via PausableAction.
* Catch-all "not found" responses return HTTP 200, not 404 — a client cannot detect a
routing miss from the status line.
* get_app() and serve() build DIFFERENT catch-alls; the serverless entry point
dispatches nothing and answers 204.
* AuthHandler.get_fastapi_dependency accepts an api_key parameter and never reads it,
while the Flask decorator does honour it.
* sip_routing_callback returns None on every branch, so the 307 redirect it appears to
offer can never fire.
* ai_chat ConversationInfo.id and ChatResponse.conversation_id are echoed from the
request argument, never read off the wire.
* The two security-header implementations diverge: mcp_gateway adds a CSP and omits
Referrer-Policy relative to web_service.
* validate_host never checks a MISSING Host header.
These are documented, not fixed — several deserve an owner ruling before nine ports
reproduce them.
TWO BRIEFS OF MINE WERE CORRECTED BY THE LANES, both verified at source afterwards:
* I said search's duplicate class sets key on the four search dependencies. They key on
BaseModel is not None (fastapi+pydantic) — an independent axis. There is also a third
path I did not know about: _SEARCH_AVAILABLE true but a submodule import failing
partially populates __all__, so names are ABSENT rather than stubbed.
* I called pom's build_section/recurse "rendering internals". Neither renders — one is
the from_json/from_yaml validator/constructor, the other a depth-first title search.
Verification of the merged tree, run here rather than delegated:
doc_surface --port python -> 100.0% (1102/1102), gate exit 0
ast gap count -> 0 across 0 files
pytest tests/ -q -> 5785 passed, 9 skipped, 9 failed
(the 9 are the known test_examples "Invalid JSON"
local-checkout defect, task #66 — unchanged)
ruff format --check signalwire -> 215 files already formatted
ruff check signalwire -> All checks passed
mypy --config-file pyproject.toml -> Success, 359 source files
A 100% floor means the next undocumented public symbol reds the gate. That is the point.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
1291 -> 0. The whole repo is now held to the SDK's own ruleset
(E4,E7,E9,F,B,S,C4,PERF,SIM,PTH,RET,RUF,UP), with tests/** carve-outs for
S101/E402/S104/S105/S106 only — the same four idiom exemptions examples/** already had,
plus assert.
Three lanes, each burned to zero on disjoint rules, merged here:
A F841 69 · RUF059 112 · S108 20 · B017 6 · S107 5 · S110 4 · S603 4 · E712 4
B SIM117 88 · RET504 12 · SIM105 12 · SIM102 6 · SIM115 4 · SIM103 1
C RUF015 44 · RUF012 30 · PTH108 10 · PTH110 8 · RUF003 7 · E402 5 · PTH107 4 + 14 more
FIVE FILES CONFLICTED, and none was resolved by picking a side wholesale:
pyproject.toml
Two lanes added DIFFERENT per-file-ignores at the same anchor. Both are justified, so
both survive (union): C's UP045 for test_type_inference.py — its Optional[X]
annotations are DATA proving the legacy path still resolves, and rewriting them would
delete the only coverage of it — and A's S603 scoped to the three files that re-invoke
sys.executable. A's is per-file, never tests/**, so a NEW subprocess anywhere in the
suite still reds the gate.
tests/unit/core/test_skill_manager.py
Kept BOTH sides: A's added assertion on the returned error string, and C's
next(iter(...)) rewrite of the list-index.
tests/unit/relay/conftest.py
Took A. A narrowed except that catches only WebSocketException/OSError and LOGS beats
a blanket suppress of Exception: a teardown error must not mask the test's real
result, but a genuine bug in disconnect() must still propagate.
tests/unit/rest/test_resource_request_options.py
Took A, three sites. These were CONTROLS that used a bare suppress — they would have
passed if the call had unexpectedly SUCCEEDED, which is the opposite of what a control
is for. Now they assert the REST error is raised.
tests/unit/skills/test_spider_skill.py
Took B's collapsed with-statement.
THE RESOLUTION ITSELF RE-EXPOSED 7 FINDINGS, because taking one lane's side necessarily
discards the other's fix in the same file. Fixed each rather than re-suppressing:
2x F401 contextlib no longer used once A's non-suppress versions won
2x B018 two bare attribute expressions as statements — evaluating an attribute and
discarding it, doing literally nothing. The same dead-assertion shape lane A was
chartered to kill, so they are deleted, not silenced.
1x F841 spider's result was assigned and never asserted — added the assertion rather
than deleting the binding, which is the whole point of the rule.
2x PTH118 os.path.join -> Path, consistent with the PTH rules already enforced elsewhere.
VERIFIED ON THE MERGED TREE, not on any lane's branch:
ruff check tests scripts eng -> All checks passed (from 1291)
ruff format --check -> 147 files already formatted
ruff check signalwire -> All checks passed
mypy -> 85 errors, EXACTLY the pre-existing baseline
pytest tests/ -q --no-cov -> 15 failed / 5976 passed / 7 skipped
THE 15 IS THE CORRECTED BASELINE, AND ALL THREE LANES CAUGHT MY ERROR INDEPENDENTLY. I
briefed 9. The 6 extra are tests/unit/mcp_gateway/**, hidden behind a module-level
pytest.importorskip on flask; flask was not importable in the shared venv when I measured,
so 195 tests silently skipped. A lane installed flask+flask_limiter to verify a defect
properly, the module started collecting, and its 6 genuine pre-existing failures surfaced.
Each lane proved them pre-existing by running the untouched base and diffing the FAILED name
sets. Tracked as #183; the fix is queued as #184.
THE GATE IS STILL NOT WIRED. Burn to zero before wire — zero is now true, so wiring is the
next commit, kept deliberately separate so a wiring mistake is not tangled with 1291 fixes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
…gated by NOTHING The burn landed in 4dd2897 (1291 findings -> 0) but the gate was left unwired, so nothing stopped the count climbing straight back. Wiring it now that zero is true: burn to zero BEFORE wire, so the gate never lands red — but "deliberately separate" stops being a reason the moment the burn is done. WHAT WAS UNCOVERED. FMT/LINT target only `signalwire/`; EXAMPLES-FMT/EXAMPLES-LINT cover the three shipped example dirs. tests/ (144 files), scripts/ (2) and eng/ (1) were covered by neither — not linted, not format-checked, ever. That is not theoretical: an unused `import socket` I added to tests/unit/relay/conftest.py earlier the same day passed every gate, and I only found it by running ruff by hand. REPO-LINT / REPO-FMT mirror the EXAMPLES-* pair exactly — same REPO_DIRS array shape, same local-applies / CI---check split in repo_fmt_gate, same cheap per-PR wave (pure static checks, no build and no mock). ruff reads pyproject's tests/** per-file-ignores (S101/E402/S104/S105/S106 — the four examples/** already had, plus assert), so the SDK ruleset otherwise applies in full: F401, F841, SIM117, RUF015/012/059, UP035/045, PTH and every bandit rule still red the gate. Verified by RUNNING run-ci.sh, not by reading it: [REPO-LINT] ruff check zero findings over tests/ scripts/ eng/ ... PASS [REPO-FMT] ruff format over tests/ scripts/ eng/ ... PASS ruff check tests scripts eng signalwire -> All checks passed ruff format --check -> 362 files already formatted The run's other reds (TYPECHECK/DRIFT/SEMVER-DIFF/GEN-FRESH/TEST) are PRE-EXISTING and not touched by this commit — confirmed by stashing this change and re-running: mypy still reports exactly 85 errors in 9 files, its known baseline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
Both `contextlib.suppress(TypeError)` blocks in test_swml_service.py were
documented as working around a structlog "'message' used as both positional
and keyword" conflict in `_detect_proxy_from_primitives`. No such conflict
exists: structlog's bound-logger signature is `warning(event, **kw)`, and the
source passes `message=` — a plain kwarg, not a collision with the positional.
The collision only occurs for `event=`, which the source never passes.
Proven, not assumed: reproduced both tests' exact setup with the suppress
removed and log.warning/log.info instrumented. Neither raised, and the
instrumentation confirms the guarded call sites DID execute
(`proxy_detected_but_url_unknown` with client_ip+message,
`proxy_detection_failed` with message) — so this is a dead guard, not an
untaken branch. Control: `log.warning('evt', event='dup')` does raise
TypeError, so the detection mechanism was live and simply never fired.
Drops the now-unused `contextlib` import.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
…183) Six tests in tests/unit/mcp_gateway/** patched functions the source under test does not call, so each asserted on a mock nothing ever touched. The source uses `Path.mkdir` / `Path.exists`; the tests patched `os.makedirs` / `os.path.exists`. Corrects the brief's diagnosis on one point: `test_stop_cleans_up_sandbox_dir` patched `shutil.rmtree` CORRECTLY — the source does call it (mcp_manager.py:291). The broken target there was the *guard*: the test patched `os.path.exists` while the source checks `Path(self.sandbox_dir).exists()`, so the guard was False and rmtree was never reached. Per-test fix: - test_sandbox_enabled_restricted_env os.makedirs -> Path.mkdir - test_sandboxing_disabled_returns_full_env os.makedirs -> Path.mkdir (its assert_not_called() was passing vacuously) - test_stop_cleans_up_sandbox_dir os.path.exists -> Path.exists - test_init_with_empty_config os.makedirs -> Path.mkdir - test_init_with_custom_sandbox_dir os.makedirs -> Path.mkdir - test_init_loads_services os.makedirs -> Path.mkdir - test_load_config_copies_sample_when_available os.path.exists -> Path.exists - test_run_enables_ssl_when_cert_exists os.path.exists -> Path.exists Patches use autospec=True where the created/checked path is load-bearing, so the receiver is captured and the assertion covers WHICH directory is created, not merely that mkdir was called. test_load_config_copies_sample_when_available also drops its builtins.open mock (the source opens via `Path.open`) in favour of a shutil.copy side_effect that writes a real file, so the source's own read path executes and the loaded config is asserted. Pre-existing, not lint-burn fallout: the identical 6 failures reproduce at 8066297, the commit before the lint work began. Every fixed test was verified capable of going RED by breaking the source behaviour it claims to check (call-removal AND wrong-argument variants), then restoring; the source is unchanged by this commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
…that ABORT the call
validate_config checked `prompt` four ways and `post_prompt` ZERO times, so a
hand-assembled config the engine kills the call over was reported valid:
validate_config({"prompt": {"text": "hi"}, "post_prompt": "Summarize."})
-> (True, []) # engine: calling.error, fatal:true, call aborted
THE ENGINE TREATS THEM IDENTICALLY. mod_openai/app_config.c checks
!cJSON_IsObject(assistant_prompt) at :3193 and !cJSON_IsObject(post_prompt) at :3219 —
same structure, same fatal:true calling.error, 26 lines apart. Both error payloads read
"must be an object with 'text' or 'pom' field"; post_prompt's names the array case
explicitly ("not an array"). Under the spine — the server is the spec — validating one and
not the other is an inconsistency in the CHECK, not a contract difference.
SCOPE, STATED PRECISELY, because the first framing of this was wrong. build_config has
ALWAYS emitted the correct shape (`{"text": ...}`), so no reference code path ever produced
bad wire and nothing about what this SDK emits changes. The hole was only reachable through
validate_config, which is public surface taking a caller-supplied dict.
THAT BLIND SPOT IS NOT THEORETICAL — IT IS HOW go SHIPPED THE BUG. go's Service.AI
hand-assembled cfg["post_prompt"] as a bare string (fixed in go 51934ec), and go's
validator faithfully mirrors this one, so nothing flagged it. Surveyed at source: go, ruby,
php and java ALL check prompt 4-5 ways and post_prompt zero times. Every port reproduced the
reference's blind spot; none invented it, none had closed it.
This is a deliberate TIGHTENING of a public validator: a config that returned (True, []) may
now return (False, [...]). That is the point — those configs abort the call — and it follows
the precedent of the secure=True token fix (#56), reference-first then rippled.
TDD-bidirectional, RED landing ON the assertion rather than upstream:
before test_bare_string_post_prompt_is_rejected assert True is False
test_array_post_prompt_is_rejected assert True is False
(3 positive controls green throughout — object accepted, absent accepted,
build_config round-trip valid — so the reds are the new rule, not a broken
fixture)
after 5 passed
Verified on the full tree, not just the new file:
pytest tests/ -q -> 9 failed, 5987 passed, 7 skipped
5982 -> 5987 is exactly the 5 new tests; the 9 are the
pre-existing test_examples JSONDecodeError from --raw writing
structlog debug to stdout ahead of the JSON (task #66),
unchanged and unrelated.
ruff check signalwire tests scripts eng -> All checks passed
ruff format --check -> 362 files already formatted
mypy -> 85 errors (baseline, unmoved)
NO EXISTING TEST RELIED ON THE LOOSE BEHAVIOUR — nothing needed updating to accommodate this.
Next in the coordinated pass: regen both oracles, then ripple the check to go/ruby/php/java,
each with its own RED-before/GREEN-after.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
…ng reads BREAKING. Owner-ruled 2026-07-29, extending the f171ce3 ruling ("if the server doesn't read them, remove them") from create_simple_api_tool's PARAMETER to the public BUILDER METHOD. f171ce3 deliberately held this back as "a larger, separately-breaking change [that] belongs to its own decision"; this is that decision, taken as a hard delete rather than a deprecation shim. THE SAME THREE SOURCES CONDEMN THE BUILDER AND THE PARAMETER IDENTICALLY: * THE SPEC FORBIDS IT. porting-sdk/schema.json $defs/Webhook declares exactly ten properties — error_keys, expressions, foreach, headers, input_args_as_params, method, output, params, require_args, url — under `unevaluatedProperties: {"not": {}}`. `body` is not among them, so emitting it is a SCHEMA VIOLATION. * THE ENGINE NEVER READS IT. mod_openai/actions.c:735-739 and bedrock.c:4920-4926 read url, method, form_param, `params` and `headers` and nothing else. `grep -n '"body"'` across BOTH files returns ZERO matches. * SO ITS ONLY POSSIBLE EFFECT was producing an invalid document while silently discarding the caller's payload. A caller reaching for the obviously-named body() for POST data got data loss with no error — the exact trap that produced this bug and, per f171ce3, plausibly cpp's datasphere bug too. `params()` is the correct method: it writes the `params` key, which IS in the contract and IS read. Its docstring already carries the corrective note f171ce3 added. TDD-bidirectional, RED landing ON the assertion: before test_body_method_is_gone -> AssertionError: assert not True (where True = hasattr(DataMap, 'body')) test_params_still_writes_the_contract_key -> PASS throughout after both PASS The positive control was green the whole time, so the red was the new rule and not a broken fixture. Proven at the wire afterwards: hasattr(DataMap, "body") -> False .body({...}) -> AttributeError params() webhook KEYS -> ['method', 'params', 'url'] MY FIRST PASS WAS INCOMPLETE AND THE SUITE CAUGHT IT. Removing the method + its 3 test call sites took the suite to 10 failures, one ABOVE the known baseline of 9. The extra was examples/data_map_demo.py (2 sites) plus a third doc example — the "7 example call sites" f171ce3 had measured and I under-read as already handled. All migrated to params(); the POST-search examples they appear in are exactly what params() is for. test_webhook_body_and_params was WORTH REPLACING RATHER THAN EDITING: it called body() AND params() and then asserted only `len(_webhooks) == 1` — it never checked either key's value, so it would have passed whatever the methods wrote. It is now test_webhook_params and asserts the emitted key. Verified on the full tree: pytest tests/ -q -> 9 failed, 5989 passed, 7 skipped BASELINE RESTORED. 5988 -> 5989 is the one net-new test; the 9 are the pre-existing test_examples JSONDecodeError from --raw writing structlog debug to stdout (task #66), unrelated and unchanged. ruff check signalwire tests scripts eng examples -> All checks passed ruff format --check -> 425 files already formatted SCOPE: reference only. The nine ports each carry the same builder (`Body` in go/dotnet, `body` elsewhere) and must be rippled next, then the oracle regenerated. SEMVER-DIFF will report this as breaking; that is correct and is held report-only in-wave by the D5 hold (porting-sdk 3358839 — python was the one port missing it). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
…swml --raw `swaig-test --dump-swml --raw` wrote structlog debug output to STDOUT ahead of the JSON document, so every caller that does the documented thing (`| jq '.'`, `json.loads(stdout)`) got a parse error. Nine tests in tests/test_examples.py failed on exactly that. The reported cause — that `configure_logging()` picks `default` mode and `_configure_default_mode()` sends the handler to stdout — is real but is NOT what produced this output. `configure_logging()` is never called on the CLI path at all: it runs only from the `serve()` entry points (swml_service.py, web_mixin.py, agent_server.py). The CLI sets `SIGNALWIRE_LOG_MODE=off` before any import, and that env var was then read by nobody. What actually printed to stdout was structlog's OWN default. `get_logger()` deliberately does not auto-configure (a library must not hijack the host app's logging, and `TestImportIsLibrarySafe` pins that). But structlog's out-of-the-box `logger_factory` is `PrintLoggerFactory()`, which writes directly to `sys.stdout` and never reaches stdlib — so the `NullHandler` we attach to the `signalwire` namespace never saw a single record. "We never configured anything" did not mean silent; it meant print to stdout. Two fixes, because either defect alone re-corrupts the payload: 1. `_install_library_defaults()` binds `structlog.stdlib.LoggerFactory()` at import, so unconfigured SDK records travel through the `signalwire` stdlib logger where the NullHandler is already waiting. Silent by default, as documented. It no-ops when the host has configured structlog itself, and touches no host-owned logger — the library-safe invariants are unchanged. `reset_logging_configuration()` re-installs it, because `structlog.reset_defaults()` restores the stdout PrintLogger. 2. `configure_logging()` now consults the machine-readable flags when it INFERS a mode, so an app that does call it under `--raw` gets stderr instead of stdout. The root defect behind (2) was flag-list DRIFT: `--raw` / `--dump-swml` were consulted in exactly one place, `_detect_colors()`, which decides ANSI colour only. So the flags reliably suppressed colour while still writing the logs into the JSON. Both decisions now read one helper, `_machine_readable_stdout()`, over one frozenset, so they cannot disagree again. That set is `--raw`, `--dump-swml` (swaig-test) and `--json` (sw-search `search` / `remote`); `--output` is a file path, not a stdout mode, and is excluded. PRECEDENCE, chosen deliberately: an explicit `SIGNALWIRE_LOG_MODE` always wins — an operator asking for `default`, `stderr`, or `off` gets it even under `--raw`. The flag inference only replaces the mode we would have GUESSED. Within the inference, machine-readable stdout outranks the server default but not CGI's `off`, where stdout is the HTTP response body and stderr the server error log. Both directions are pinned by tests. Logging level and content are untouched — debug output is still wanted, just not on the channel the caller parses. pytest: 9 failed / 5792 passed -> 0 failed / 5806 passed (+5 new tests). Public surface unchanged (both helpers are private); porting-sdk python_surface.json and python_signatures.json regenerate byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
…e error to — 85 errors -> 11
RUFF FORMAT AND MYPY DISAGREED ABOUT WHERE A SUPPRESSION LIVES, AND THE FORMATTER WON.
mypy anchors `method-assign` to the line where the assignment STARTS, and `arg-type` /
`typeddict-unknown-key` to the line of the offending ARGUMENT. `ruff format` splits a
long call across lines and parks the trailing comment on whatever line it ends up
beside — the second line of the call, or the closing paren. Once the comment moved off
the anchor line, each site cost TWO errors instead of zero:
engine._vector_search = Mock( <- mypy reports method-assign HERE
return_value=[ # type: ignore[method-assign] # mock <- comment landed HERE
)
-> error: Cannot assign to a method [method-assign] (unsuppressed)
-> error: Unused "type: ignore" comment [unused-ignore] (misplaced)
That doubling is the whole of the backlog: 31 method-assign + 48 unused-ignore + 6
arg-type/typeddict/attr-defined = 85, from 36 misplaced comments across 8 test files.
Not one is a real type defect, and not one of these suppressions was new — they were
correct when written and were displaced by the mechanical `ruff format` pass in 8ff231f
that first brought tests/ under the formatter.
FIX: move each comment back onto its anchor line, and where ruff would just split the
call again, shorten the CODE (bind the callee or the literal to a local) so the
statement fits on one line and the formatter has nothing to move. No ignore was added,
broadened, or made bare; every one keeps its error code and its reason text. The mypy
configuration is untouched.
The formatter is now a fixed point rather than a fight — verified by running it to
convergence, not by a single pass:
ruff format tests scripts eng -> 147 files left unchanged
ruff format --check tests scripts eng -> 147 files already formatted
ruff check tests scripts eng -> All checks passed
mypy --config-file pyproject.toml -> Found 11 errors in 1 file (was 85 in 9)
REMAINING 11 ARE ONE UNRELATED SHAPE, LEFT FOR ITS OWN COMMIT: every one is an
`untyped-decorator` ignore in mcp_gateway/gateway_service.py reading as unused because
flask ships `py.typed` in this checkout. flask is an optional extra (`mcp-gateway`) and
is NOT in requirements-dev.txt, so CI type-checks it as `Any` and the ignores ARE used
there. That is an environment-dependent suppression, not a misplacement, and deleting
it locally would red CI.
pytest tests/unit/search tests/unit/core/test_swml_service.py
tests/unit/core/test_agent_base.py tests/unit/skills/test_spider_skill.py
tests/unit/skills/test_native_vector_search_skill.py
-> 1111 passed
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
A tool registered with `secure=True` was enforced over HTTP and COMPLETELY
UNENFORCED under every serverless mode (lambda, cgi, google_cloud_function,
azure_function). A forged token, and an entirely absent one, both ran the
handler. `secure=True` silently meant "enforced only if you happen to deploy
on a server".
Two independent causes, both fixed:
1. No serverless mode read a query string at all. The SDK itself mints the
credential into the webhook URL's query string (`_build_webhook_url`'s
serverless branch strips call_id and keeps only `__token`, since the token
self-carries the call_id), so the query string is the SOLE identity channel
-- and nothing read it. `queryStringParameters` / `QUERY_STRING` /
`rawQueryString` appeared zero times in the repo.
2. The dispatch path never reached the check. `_swaig_pre_dispatch`'s first act
is `request.query_params.get("__token")`, and there is no request object in
the serverless path.
The fix extracts the security decision into a transport-agnostic core and
plumbs the credential per mode:
- New `_swaig_validate_token(function_name, token, call_id) -> refusal | None`
on SWMLService (no-op extension point) and AgentBase (the real check). It
takes only plain scalars -- no request/transport type -- so every transport
reaches the identical decision. `_swaig_pre_dispatch` keeps its signature
and its `Request` (dynamic-config genuinely needs one) and now delegates the
security half to this core; the HTTP path is behaviourally unchanged.
- `_execute_swaig_function` gains a trailing optional `token` parameter and
calls the core before dispatch. Applied to BOTH definitions (ToolMixin and
ServerlessMixin) so an MRO change cannot silently reopen the hole.
- Per-mode query-string extraction, four different payload shapes:
lambda -> `queryStringParameters` mapping (REST API v1 and HTTP API v2),
falling back to the raw `rawQueryString` (HTTP API v2)
cgi -> the `QUERY_STRING` environment variable
gcf -> Flask `request.args`, falling back to `request.query_string`
azure -> `req.params`, falling back to the query component of `req.url`
Semantics match HTTP exactly: read `__token` then fall back to `token`; valid
only if a token AND a non-None call_id AND `validate_tool_token` agree; a
MISSING call_id counts as unvalidated rather than as a bypass; only `secure`
tools are refused, and an insecure tool proceeds ungated. The refusal is a
200 + FunctionResult body, never an HTTP error status -- the engine
(mod_openai) has no handling for a SWAIG refusal status.
Also fixes a real routing defect found while writing the azure tests: the
azure handler split the RAW `req.url` on "/api/", so a query string was baked
into the function name ("say_hello?__token=abc" -> "Function not found"). It
now splits the PARSED path. Any azure deployment passing query params was
affected, independent of the token work.
Tests: 56 new, covering all four modes (plus both lambda payload shapes) x
{valid, forged, absent, no-call-id, token-for-another-function,
token-for-another-call} x {secure=True, secure=False}, and the
`__token`/`token` spelling precedence. The valid-token cases are the
load-bearing half: they prove the fix refuses selectively rather than
refusing everything.
Note for the port matrix: the BEHAVIORAL/SWAIG-HTTP golden derives live from
this reference and previously recorded a secure tool running with NO token --
it encoded the fail-open. It now records the refusal. That divergence is
correct and expected; the golden needs regenerating.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
…ne gate
Python was the LAST port whose SEMVER-DIFF could BLOCK on an intentional in-wave
breaking change. GATE_ENFORCEMENT_PLAN.md D5a defers the version-line decision to
the real release cut ("no bump churn now; perl/rust 4.0.0 declarations stay as-is;
unified-vs-per-port decided at cut time"), so the gate must REPORT, not block.
The hold is already fleet-wide, applied at porting-sdk
scripts/suites/_surface_commands.py:274-275 via `semver_report_only=True`. Eight
ports (typescript/java/php/ruby/perl/dotnet/go/cpp) route SEMVER-DIFF through the
SURFACE suite and inherit it; rust got the same standalone fix today in 8560313.
Python schedules SEMVER-DIFF as its own sched_gate, so it never passed through
`gate()` — the only place the flag is applied — and silently opted out. Structural
gap, not a deliberate exemption.
Note the trap this closes: porting-sdk 3358839 claimed to fix python, but only
touched the _surface_commands.py layer, which python's run-ci does NOT route this
gate through. The fleet table is not evidence a port has the hold — the port's own
run-ci.sh is.
The gate description is reconciled in the same change. It previously read "(the
reference is not exempt)", which predates D5a and would read as contradicting the
flag; it now states what is true — the gate still runs and still reports, it just
does not block in-wave.
Per D5a: version stays 3.3.0 (owner: "wait.. no keep python 3.3"), no
SEMVER_DIFF_ALLOW entry, no baseline touched.
Negative-controlled both directions — the flag changes only the exit code and the
blocking header, never the finding. Both runs emit the identical MISMATCH line, the
same 3 removed members, the same 10 retyped members and the same 44 additions:
WITH --report-only (exit 0):
[semver-diff] python: 3.2.0 (3.2.0) -> 3.3.0 (pyproject.toml) actual bump =
'minor', required = 'major' [MISMATCH]
BREAKING - 3 member(s) removed since last release:
- signalwire.agent_server.AgentServer.app
- signalwire.core.data_map.DataMap.body
- signalwire.web.web_service.WebService.app
BREAKING - 10 member(s) retyped since last release: ...
ADDITIONS - 44 new member(s) since last release (minor): ...
[semver-diff] REPORT-ONLY - not failing (burn the backlog / set the right bump,
then drop --report-only).
WITHOUT (exit 1): byte-identical finding, preceded by the blocking header
[semver-diff] a major change shipped as a 'minor' bump.
Bump the version to at least a major release, or - for an approved
intentional exception - add the breaking symbol(s) to SEMVER_DIFF_ALLOW.md
with a reason.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
Installing the SDK without the `mcp-gateway` extra and importing the gateway gave
the user a raw traceback naming a third-party package, with nothing pointing at
the fix:
>>> from signalwire.mcp_gateway.gateway_service import GatewayService
File ".../signalwire/mcp_gateway/gateway_service.py", line 30, in <module>
from flask import Flask, request, jsonify, Response
ModuleNotFoundError: No module named 'flask'
The extra has been declared all along (pyproject.toml:112-115), so the fix was one
pip command the user was never told about. Now:
ImportError: flask and flask-limiter are required for the MCP gateway.
Install them with: pip install signalwire-sdk[mcp-gateway]
This follows the guard idiom already established in this repo (agent_server.py:23,
core/swml_service.py:52, core/web.py:20, search/__init__.py:60) rather than a new
one, and uses the real distribution name from pyproject.toml `name` —
`signalwire-sdk`, not `signalwire-agents`.
Swept every optional extra's entry module for the same shape, via an AST scan for
module-level imports of an extra-only package that are not nested in a try/if. A
SECOND instance was found and fixed the same way:
- search/query_processor.py — bare top-level `import nltk` (+ two nltk.* froms).
search/__init__.py gates it behind _SEARCH_AVAILABLE, but index_builder.py:36
and search_service.py:40 import it unconditionally, and both of those files
carefully guard numpy/sentence_transformers/fastapi — so nltk was the one
unguarded hole on that path. Same user impact as flask.
The scan is negative-controlled: 7 unguarded imports on the pre-fix tree (4 in
gateway_service.py, 3 in query_processor.py), 0 after. All seven extras were
covered — search, search-queryonly, search-full, search-nlp, search-all, pgvector,
mcp-gateway; the remaining ones were already guarded (pgvector_backend.py raises
its own PGVECTOR_AVAILABLE message; document_processor / search_engine / migration
use try/except shims).
The top-level `mcp_gateway/` directory has a second copy of gateway_service.py with
the same unguarded imports, but it is deliberately NOT packaged (pyproject sets
packages where=["signalwire"]) and bootstraps via sys.path.insert as a standalone
Docker deployment, so it is not reachable as `signalwire.mcp_gateway` and is left
alone.
Test asserts the raised message names the extra, and simulates the package being
absent (blocking sys.modules + __import__) rather than requiring it to be
uninstalled — so it is meaningful on a dev box that has every extra installed. It
failed on exactly that assertion before the fix:
AssertionError: signalwire.mcp_gateway.gateway_service raised
"No module named 'flask'", which does not name the 'mcp-gateway' extra --
the user is left with a bare ModuleNotFoundError and no way to know the fix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
`_MAX_CONNECTIONS` was evaluated once at module scope, so the limit was frozen
the moment `signalwire.relay.client` was imported. `connect()`'s refusal message
tells the operator to "set RELAY_MAX_CONNECTIONS env var to allow more" — advice
that could never work, because by the time the message is seen the module is
already imported and the value is fixed. The variable is documented as
user-facing configuration in `relay/README.md` and `relay/docs/client-reference.md`,
so a user who set it after import was silently ignored.
Replace the module global with `_max_connections()`, called from `connect()`.
Semantics are unchanged otherwise: `max(1, int(...))`, default "1", a
non-integer falling back to 1.
Tests: `test_env_var_set_after_import_takes_effect` sets the var long after
import and proves the raised limit takes effect (it fails against the frozen
value). `test_invalid_env_var_fallback` now actually exercises the ValueError
path instead of asserting `_MAX_CONNECTIONS >= 1`, and the limit-of-one test
sets the env var instead of patching the module global.
The relay conftest's `os.environ.setdefault("RELAY_MAX_CONNECTIONS", "16")`
stays — it is still what raises the ambient limit for the whole test process —
but its comment no longer describes it as an import-time workaround.
`SkillBase.get_prompt_sections()` returns an empty list when the skill is
configured with `skip_prompt: True`, then delegates to the protected
`_get_prompt_sections()` hook. But 11 of the 13 shipped skill files overrode the
PUBLIC method directly, so the guard never ran for them and `skip_prompt` was
silently ignored.
Measured before this change (probe over every shipped skill, constructed +
setup(), with and without skip_prompt): 10 of 17 constructible skills returned
their sections anyway — JokeSkill returned 1 section with skip_prompt=True where
it should return 0, matching the runtime measurement from the perl lane. After:
0 of 17.
Rename the override in each of the 13 files (datasphere, datasphere_serverless,
datetime, google_maps, joke, math, mcp_gateway, native_vector_search,
swml_transfer, web_search {skill, skill_original, skill_improved},
wikipedia_search) to `_get_prompt_sections`, matching what claude_skills and
info_gatherer already did. No skill called `super().get_prompt_sections()`, and
the only caller of the public method is `SkillManager`, so the guard now runs on
every path. Section content is unchanged: the counts returned WITHOUT
skip_prompt are byte-identical before and after.
`tests/unit/skills/test_skip_prompt_guard.py` locks this in two ways: a
structural sweep over all 19 shipped skill classes asserting none overrides the
public method (so a new skill cannot reintroduce the bypass), and behavioural
checks that JokeSkill/MathSkill return their section by default and nothing with
skip_prompt. The sweep does not swallow import errors — a skipped module would
make it vacuous.
TypeScript already had this shape and rust re-checks skip_prompt in every skill;
both were more correct than the reference until now.
`SWML_SSL_ENABLED=true` with a missing certificate produced a working PLAINTEXT listener: `AgentServer._run_server()` and `SWMLService.serve()` both logged a warning, set `ssl_enabled = False`, and called `uvicorn.run()` with no ssl args. The operator asked for encryption, got cleartext — carrying the Basic-auth credentials — and was never told. Measured before this change by binding a real listener on each path and speaking to the socket: `SWML_SSL_ENABLED=true` + a nonexistent cert answered `GET / HTTP/1.0` with an HTTP status line on BOTH paths. After: both refuse with a RuntimeError and never bind. Both sites now raise instead of downgrading. `SWMLService.serve()` additionally validates the paths that ACTUALLY reach uvicorn — they may come from the `serve(ssl_cert=/ssl_key=)` arguments, which `security.validate_ssl_config()` never sees, so a caller could previously pass nonexistent paths past a passing validation and land in the plaintext branch. Scope control, verified the same way: SSL disabled still serves plain HTTP, and SSL enabled with a real self-signed cert/key still completes a TLS handshake, on both paths. Reverting the guards makes exactly the refusal cases red and leaves the plain-HTTP and TLS cases green. Two existing tests pinned the downgrade (`test_run_server_ssl_disabled_bad_cert`, `test_serve_ssl_invalid_config_disables_ssl`) and are flipped to assert the refusal. Three others (`test_serve_with_ssl`, `test_serve_ssl_without_domain_warns`, `test_serve_overrides_domain`) mocked `validate_ssl_config` to pass while handing uvicorn cert paths like `/path/cert.pem` that do not exist; they now use real temp files. Added positive scope-control tests on both paths. rust already had this shape (`resolve_tls_material()` returns `Err`, not `Ok(None)`); go, php, and cpp shipped the same fix after proving it at the wire. java pinned the current Python behavior instead of fixing it — this unblocks it.
…ra is installed
TYPECHECK's verdict on gateway_service.py depended on which interpreter ran it.
flask and flask-limiter are the OPTIONAL `mcp-gateway` extra and they ship
`py.typed`, so their decorators' types change with their mere presence — and it
flips BOTH ways, which is why no arrangement of `# type: ignore` could be green
everywhere:
* extra absent -> `@self.app.route` is Any, so every decorated handler is
[untyped-decorator] unless it carries an ignore;
* extra present -> the decorators are typed and those same 11 ignores become
[unused-ignore] under warn_unused_ignores (via strict).
CI installs the base package with no extras, so it saw the first world. A dev
box with the extra installed — or merely another venv earlier on PATH — saw the
second and went red on 11 findings. Nothing about the code differed.
Fix, entirely in the source files:
* `self.app` / `self.limiter` are annotated `Any`, so the route/limit
decorators resolve to Any either way and the 11 ignores are required and
used in both worlds.
* `set_security_headers` is registered by CALL rather than with
`@self.app.after_request`. That one is generically typed, so as a decorator
it rewrote the function's type and produced mirror-image failures
([untyped-decorator] without the extra, [unused-ignore] with it) — the one
construct that could not be reconciled. Calling it registers the identical
hook at runtime and leaves the function's own annotations alone.
* its `response: Response` and the test helper's `-> "TestResponse"` become
`Any` for the same reason (both are real types only when the extra is
installed); the now-unused werkzeug import goes with them.
Deliberately NOT fixed with a `[[tool.mypy.overrides]]` follow_imports=skip for
flask. That lives in OUR mypy config, so an SDK consumer running mypy over their
own code gets no override, flask resolves as typed, and our ignores turn into
[unused-ignore] errors in THEIR build. The pyproject change here is only a
comment correction: the old text claimed flask has "no stubs installed", which
has been false since flask 2.x and is what made this look environmental.
Verified on the same tree with independent cache dirs:
flask installed -> Success: no issues found in 362 source files
flask not installed -> Success: no issues found in 362 source files
Runtime unchanged: 197 passed in tests/unit/mcp_gateway/. Removing the
after_request registration reds the four TestSecurityHeaders wire assertions
(assert None == 'nosniff'), so a hook that failed to register would be caught.
`_get_prompt_sections()` returned `[]` with a comment deferring the work to
"register_tools after the agent is set". The real content lived in
`_add_prompt_section(self, agent)` — a push-style helper
(`agent.prompt_add_section(...)` in a try/except) that NOTHING in the repo ever
called. The hook is pull-style and returns a list, so the section was stranded:
loading this skill added zero prompt guidance about the search tool.
Fold the stranded section into the hook verbatim (same title, body and four
bullets) and delete the dead push-style method. The `AgentBase` TYPE_CHECKING
import went with it — that method was its only user.
Before: `NativeVectorSearchSkill(...).get_prompt_sections()` -> `[]`
After: one section, `title="Local Document Search"`, four bullets — the same
shape sibling skills (datetime, math) return, so the consumer handles it
identically. `skip_prompt=True` still yields `[]` via the SkillBase guard.
Tests: `TestAddPromptSection` (which exercised the dead method) becomes
`TestGetPromptSections` against the hook, and `test_get_prompt_sections_returns_empty`
— which asserted the defect — is removed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
The existing guard tested the structural rule (no skill overrides the public `get_prompt_sections`) plus two representative skills. Neither catches a skill whose HOOK returns `[]` for the wrong reason — that is how native_vector_search shipped contributing no prompt section while every skip_prompt assertion about it passed. Add `TestEverySkillHonoursSkipPrompt`, parametrized over the DISCOVERED skill classes (reusing this file's `_iter_skill_classes()`), not a hand-written list — a hand list silently omits a newly added skill, which is the blind spot being closed. Both halves of the contract are asserted: * `skip_prompt=True` suppresses all sections, and * (the load-bearing inverse) `skip_prompt` unset returns a NON-EMPTY list. The inverse half is what catches the vacuous pass; the docstring says so. All 19 discovered skill classes are covered — none is skipped. `MINIMAL_PARAMS` records the smallest params each needs for `setup()` to succeed, and `test_every_skill_class_has_minimal_params` fails if a new skill lacks an entry, so a skill cannot fall out of the sweep unnoticed. Five skills legitimately emit no section and are exempted by name WITH a reason in `NO_SECTION_BY_DESIGN`: api_ninjas_trivia, play_background_file, spider and weather_api define no `_get_prompt_sections` override at all; mcp_gateway defines the hook but emits a section only when `services` are configured. The exemption list is itself pinned, and an exempt skill that starts emitting fails too — so the list cannot silently absorb a regression. Only mcp_gateway needs stubbing to run offline (its `setup()` GETs `<gateway_url>/health` and SSRF-validates the URL, which does DNS); the `_offline` helper patches exactly those two and nothing else. Red-before/green-after: with native_vector_search reverted to its pre-fix state, `test_default_returns_a_section[native_vector_search.skill]` FAILS (1 failed, 43 passed) while every suppression assertion still passes — demonstrating the inverse assertion is the half with teeth. With the fix in place: 44 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
porting-sdk be7a34f vendored calling.conference.{params,result}.json but the
reference's generated relay types were never regenerated, leaving GEN-FRESH
red: 128 schema files vs 126 emitted declarations.
Regenerated with the generator named in the file's own header
(porting-sdk/scripts/generate_python_rest_types.py). The whole delta is two
TypeAlias lines — both schemas are permissive (type: object,
additionalProperties: true, x-permissive: true, no properties), so
declaration() takes the alias branch rather than emitting a TypedDict:
CallingConferenceParams: TypeAlias = "dict[str, Any]"
CallingConferenceResult: TypeAlias = "dict[str, Any]"
Surface-neutral: enumerate_python records only `class` declarations from this
module (123 classes in file, 123 in python_surface.json; the 5 pre-existing
TypeAliases are absent from the oracle). Verified by regenerating
python_surface.json — byte-identical ignoring generated_from. So this does NOT
ripple to the nine ports; it closes the GEN-FRESH gate only.
GEN-FRESH: exit 1 -> exit 0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
… types
verify_basic_auth and verify_bearer_token annotated their sole parameter with
FastAPI's HTTPBasicCredentials / HTTPAuthorizationCredentials. Neither body ever
touched anything framework-specific: they read .username/.password and
.credentials and compare with secrets.compare_digest. The FIELDS are the
contract; which web framework's class carries them is idiom.
FastAPI is already an OPTIONAL dependency in this module — the try/except sets
these names to None in a non-web install, so the annotation degraded to None
exactly when FastAPI was absent. The framework type was doing no real work.
Both params are now typed by a runtime_checkable Protocol:
BasicCredentials (username, password)
BearerCredentials (scheme, credentials)
A Protocol is strictly WIDER than the concrete class, so this is backward
compatible: a real FastAPI HTTPBasicCredentials still satisfies it at every
existing call site, at runtime and under mypy. Tests pin both a genuine FastAPI
object and a duck-typed one, plus the FastAPI-absent import path.
The names and field sets are the fleet's existing convention rather than a new
invention: 8 of the 9 ports (rust, java, typescript, dotnet, php, ruby, perl,
cpp) already ship exactly these two carriers with these field names, and
dotnet's comment already claims it "mirrors the reference's
signalwire.core.auth_handler.BasicCredentials" — a symbol that did not exist
until now. go is the sole exception, passing *http.Request or a scalar pair,
which folds at the emitter.
scheme is REQUIRED, matching all 8 ports. verify_bearer_token compares only
credentials, but scheme is half of what the Authorization header conveys and
ports dispatch on it (rust auth_handler.rs:276,280).
No concrete value class is added: any object with the fields satisfies these,
so shipping one would be surface the ports would have to mirror for nothing.
No behaviour change — secrets.compare_digest and both method bodies are
untouched. BOTH oracles regenerate byte-identical (python_signatures.json is
unchanged; python_surface.json differs only in its provenance hash), because
the reference now genuinely declares the shape the enumerator's
STRUCTURAL_TYPE_FILLERS (dcff742) previously had to synthesize.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSHZoWoxoPVK6FuNaNKWFh
…ored specs The generator now resolves `<file>.yaml#/components/schemas/<Name>` into the sibling spec instead of keeping only the last path segment, so the post-prompt swaig_log entry's post_response / delayed_post_response are the real SwaigResponse rather than `dict[str, Any]`. swaig_actions_generated gains the two envelope types the resolution targets (SwaigAction, SwaigResponse); their field types are the same expressions the _SwaigActions builder methods take, so envelope and builders cannot drift. The rest of the diff is the three swaig files catching up to the re-vendored specs (post-prompt / swaig-request / swaig-response): tighter item types (list[Any] -> list[dict[str, Any]]), the 27-value system-log `action` enum, present-and-true fields as Literal[True] (native, mcp_error), mcp_response as str, and three system-log keys (context/step/step_index) the spec no longer declares. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016i9TibArqEmDszG7VYEGYg
… pins hold
requirements-dev.txt:36 was `mypy>=1.8` — an open floor with no ceiling — two lines
below a ruff pin whose own comment states the rule it violates ("PINNED exact — an
unpinned `>=` lets a newer ruff reformat files that passed under the version a dev
has installed (local≠CI drift)"). mypy adds checks and narrows inference between
releases, so an open floor makes TYPECHECK's verdict depend on WHEN the environment
was provisioned rather than on the source: CI installs the newest allowed, a
contributor runs what they installed months ago, and the CI red does not reproduce
locally because the difference is not in the code.
requirements-dev.txt:36 mypy>=1.8 -> mypy==2.3.0
This was not hypothetical here: the interpreter running these gates had mypy 1.18.2
while a fresh CI install of `>=1.8` resolves 2.3.0. TYPECHECK's local and CI
verdicts were being produced by type checkers a major version apart.
A manifest pin alone cannot make local == CI, so scripts/assert_tool_pins.py is
wired as a new TOOL-PINS gate. pip does not re-resolve an already-satisfied
requirement, so an environment provisioned before a pin was tightened keeps its old
version indefinitely — the pin is then right in the file and violated in the
interpreter that actually runs the gates, invisibly. TOOL-PINS checks two things and
reads the expected versions out of requirements-dev.txt so there is one source of
truth:
1. every version-sensitive tool (ruff, mypy) is pinned EXACT — an open
constraint is itself the finding, whatever is installed today;
2. that pinned version is the one importable by this interpreter.
SW_ALLOW_TOOL_VERSION_DRIFT=1 downgrades a mismatch to a warning, for a deliberate
bump-and-fix run only.
Two findings from actually running the upgrade rather than assuming:
* mypy 2.3.0 on this tree is CLEAN — `Success: no issues found in 362 source files`.
So the bump surfaced zero real violations to fix.
* The 3 `redundant-cast` errors 2.3.0 initially reported (pom.py:519,
swml_renderer.py:146/197 — all `cast(str, yaml.dump(...))`) were NOT a code defect
and NOT caused by the version bump. They came from `types-PyYAML 6.0.12` being
present in the local environment while declared in NO manifest: with the stubs,
yaml.dump() is typed `str` and the casts are redundant; without them (which is
what CI has, since nothing installs them) the casts are load-bearing and mypy is
silent. Removing the undeclared stray package took the count 3 -> 0. Worth knowing
because it cuts both ways: an UNDECLARED package can make a gate red locally and
green in CI, which is the same class of defect as an unpinned one, just inverted.
Leaving the casts alone is correct — deleting them would break CI, where yaml is
genuinely untyped. (The stale comments above them, "yaml has no type stubs", are
accurate for the declared environment.)
Verified via run-ci:
[TOOL-PINS] ruff/mypy pinned exact in requirements-dev.txt AND installed at that version ... PASS
[TYPECHECK] mypy zero findings ... PASS
[LINT] ruff check zero findings ... PASS
[FMT] ruff format ... PASS
[REPO-LINT] ruff check zero findings over tests/ scripts/ eng/ ... PASS
[TEST] 6061 passed, 1 skipped
Negative control, both paths: restoring `mypy>=1.8` makes TOOL-PINS exit 1 ("NOT
pinned exact ... an open constraint lets CI run a different version than local"),
and pinning `mypy==9.9.9` against the installed 2.3.0 exits 1 on the mismatch; the
real pin exits 0. The version can no longer float undetected.
The gate itself is subprocess-free: it reads installed-distribution metadata rather
than shelling out to `python3 -m <tool> --version`, because the subprocess form
tripped the repo's own ruff S603. Removing the rule's premise beat suppressing it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016i9TibArqEmDszG7VYEGYg
…I verdict split)
A type stub's PRESENCE changes TYPECHECK's verdict on unchanged source, and
nothing declared or checked it. Every repo on a dev box resolves ONE shared
venv, so types-PyYAML — declared in api-reference-specs' requirements-dev.txt
and in NO signalwire manifest — silently landed on this repo's path.
Measured with mypy 2.3.0 in a venv built from this repo's own manifest, the
only variable being the stub:
without types-PyYAML: Success: no issues found in 362 source files
with types-PyYAML: Found 3 errors in 2 files (redundant-cast)
pom.py:519, swml_renderer.py:146, swml_renderer.py:197
CI installs `pip install -e .` + requirements-dev.txt, so CI had the FIRST
result: those three casts were LOAD-BEARING there, where yaml.dump() is
untyped Any. "Fixing" the local error by deleting them would have broken CI —
the inverse of the usual floating-version defect, and invisible to a version
check because the package is simply absent on one side.
Resolved in the direction that types MORE. types-PyYAML is now declared and
pinned exact, so local and CI read the same type information; with the stub
yaml.dump(stream=None) is genuinely `str` (reveal_type -> "str"), which makes
the three casts truly redundant, so they are removed in this same commit
rather than left to red the gate.
TOOL-PINS is extended to catch the CLASS, not just this instance: any PEP 561
stub (`types-*` / `*-stubs`) importable by the gate interpreter must be
declared, and any declared stub must be installed and pinned exact. Both
directions fail, because both are a local≠CI split — a neighbour's stub
leaking in makes local see types CI lacks, and a missing declared stub makes
local type-check less than CI does.
Negative-controlled both ways: declared-but-absent fails ("types-pyyaml is
declared ... but is not importable"), and an undeclared stub installed on the
path fails ("types-requests==2.33.0.20260712 is importable ... but is NOT
declared"). Green only with the declaration and the stub in agreement.
Measured, not assumed: no other port shares this gap — signalwire-python is
the only port whose run-ci.sh runs mypy at all (php/perl/dotnet ship a
requirements-dev.txt for ruff only, and no port declares or needs a stub).
==> CI PASS (43/43 gates, 0 FAIL)
…ten off C truthiness, one new post-prompt action
GEN-FRESH was stale on this branch: porting-sdk d8e5787 re-vendored swaig-specs
from mod_openai @ 8d6ed5e and nothing regenerated the ports. Named by psdk#125's
SPEC-FANOUT, which listed exactly these two files.
Regenerated with
python3 <porting-sdk>/scripts/generate_python_rest_types.py \
--signalwire-python signalwire
against wave6/ctor-dunder-fold. Not hand-edited.
THIS IS A WIRE-SURFACE CHANGE, not a reformat. Read it as one.
swaig_actions_generated.py -- four actions were typed dict[str, Any] because the
spec had no type for them; the re-vendor gave each one an x-truthiness fact
naming the C that reads it, so they narrow to what the engine actually accepts:
clear_dynamic_hints dict[str, Any] -> bool | str
hangup dict[str, Any] -> bool | str
stop dict[str, Any] -> bool | str
stop_playback_bg dict[str, Any] -> bool | str | int | dict | list | None
The first three share one mechanism, quoted from the spec: read with
cJSON_FSTrue (step_management.c:990) -- a JSON `true`, or a string switch_true()
accepts; a JSON `false` does NOT trigger it. stop_playback_bg stays wide because
the engine reads it as presence, so any JSON value triggers it.
The builder-method parameter types move with the TypedDict fields, so a caller
passing `{}` to .hangup() -- previously the only type-legal call -- now fails
type-check, correctly: an empty object was never what the engine reads.
post_prompt_generated.py -- PostPromptSystemLogEntry.action gains
"attention_wait", 27 -> 28 values. The closed set is derived from the producer
call sites (ai_conversation_system_log 7 -> 8), not hand-listed.
The remaining hunks are x-source line-number drift (actions.c shifted ~3 lines);
they carry no surface.
Verified: run-ci.sh GEN-FRESH PASS; SPEC-FANOUT reports python fresh.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MjEro9sSs5TLq66rTzSq6Z
…w request pair, two types widen
porting-sdk wave6/ctor-dunder-fold @ dea604b re-vendored swaig-specs/ from
mod_openai ff4f544. Of the six files that re-vendor touched, three are python
generator inputs (swaig-request, swaig-response, post-prompt); the corrections
the commit message leads with -- background_file's wrong writes_field
"bgfh.vol", item_key_reserved_values, indirect_via, the validator_fn ->
accessor swap -- all live in ai-verb-params.yaml and ai-sidecar-config.yaml,
which no port generator reads. So none of them appear here.
What the three consumed specs actually changed, and what each produced:
swaig-response.yaml
+ SWML: a new response action, oneOf string|object. Executes a SWML
document inline, or with sibling transfer:true transfers the call into
it; gated by swaig_allow_swml, and transfer additionally requires
from_relay (actions.c:142-145). Emits the SwaigAction field
`str | dict[str, Any]` plus its builder.
~ extensive_data and functions_on_speaker_timeout go boolean ->
[boolean, string]. Both are read with cJSON_FSTrue (actions.c:376 and
:372), so a switch_true string has always fired them on the wire and the
committed `bool` was under-typed. Now `bool | str`, matching the four
that already carried that shape.
~ transfer's x-source corrected 136 -> 343 (the dispatch site, not the
swaig_allow_swml gate above it). Docstring trailer only.
swaig-request.yaml
+ SWMLCall and SWMLVars, both open string-keyed objects. Posted only when
swaig_post_swml_vars is set AND the swml_serialized_state channel var is
present (actions.c:2097-2100).
post-prompt.yaml: header pin only, no schema change -- and correspondingly
post_prompt_generated.py is byte-identical after the regen.
back_to_back_functions, clear_dynamic_hints, hangup and wait_for_user picked
up x-truthiness/x-truthiness-source prose in this re-vendor but no type
change; they were already bool|str and are untouched here. The nested
x-coerced-by annotations (context_switch.consolidate/full_reset,
hold.timeout, playback_bg.wait, transfer.summarize) are provenance the
generator does not consume -- no emitted bytes move.
Regenerated with scripts/generate_python_rest_types.py --signalwire-python
signalwire; the two files below are the generator's complete output delta, and
all 15 other specs re-emitted byte-identical.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MjEro9sSs5TLq66rTzSq6Z
anthmFS
added a commit
that referenced
this pull request
Aug 10, 2026
* fix(ci): green the gates the ai_chat/ChatGateway commits reddened The three ChatGateway commits (8b790ea, cac3118, 20663a6) landed on main without a full run-ci, reddening LINT, FMT, TYPECHECK and NO-CHEAT. This fixes each at its source. LINT (ruff 0.15.21, the pinned version) - ai_chat/__init__.py: RUF022 __all__ sorted. - search/document_processor.py: SIM102 nested if collapsed. FMT - ruff format over ai_chat/gateway.py + core/function_result.py. TYPECHECK (mypy --strict; the config puts tests in scope on purpose: "a new untyped test fails the gate") - tests/unit/ai_chat/test_gateway.py shipped fully unannotated: 44 no-untyped-def + 19 no-untyped-call. Annotated throughout. - FunctionResult.response widened to `str | dict[str, Any]`, so 37 call sites doing `.response.lower()` stopped type-checking. Added `assert isinstance(<r>.response, str)` next to the existing `assert isinstance(<r>, FunctionResult)` — a real assertion that narrows the union, not a cast. - FunctionResult.hold: `bool` subclasses `int`, so excluding bools from the back-compat int-swap left `str | bool`. Handle bool explicitly; the remaining type is `str | None`. hold(120) still means hold(timeout=120). - ChatGateway.visible_messages / last_activity accept None and non-dict items by design (`for msg in messages or []`, `if not isinstance(msg, dict): continue`) and are tested for it, but were typed `list[dict[str, Any]]`. Widened to match the real, documented contract. Free to change: ChatGateway is new surface no port has implemented yet. NO-CHEAT - Three origin tests asserted nothing ("does not raise"), so they passed regardless of the code. Each now pairs the allowed case with the refusal that proves it is an exemption and not open-by-default: localhost vs an unlisted origin, a listed origin vs a lookalike domain, absent vs present-but-unlisted. Verified: LINT clean, FMT clean, NO-CHEAT clean, mypy clean over every file CI reports, 5940 unit tests pass. The 6 remaining mcp_gateway failures are pre-existing (they fail identically on unmodified main) and env-dependent — CI passes them. Not addressed here (deliberately): GEN-FRESH and DRIFT/SEMVER-DIFF are coordinated-pin artifacts. PORTING_SDK_REF is set to wave6/ctor-dunder-fold, so CI builds against that branch; the matching regen is PR #78's half of the wave, not this branch's. * fix(gen-fresh): regenerate generated types against the pinned wave6 specs CI resolves porting-sdk via PORTING_SDK_REF, currently wave6/ctor-dunder-fold, so GEN-FRESH regenerates from THAT branch's specs and compares. The committed files were generated from main's specs, so six reproduced differently and the gate failed. Regenerated with the pinned ref's specs; `--check` is now clean. Note these are NEWER than the same files on the wave6 branch itself: the swaig specs gained `| str` on several action fields after that branch last regenerated (e.g. `consolidate: bool` -> `bool | str`, `wait: bool` -> `bool | str`). So this is the output current wave6 specs actually produce, which is what CI checks against. Full unit suite still 5940 passed; the 6 mcp_gateway failures are pre-existing and env-dependent (they fail identically on unmodified main).
… engaged
_verb_top_level_property_names tested `body.get("type") != "object"` on a verb's
config node and returned None otherwise. A union node — `{"anyOf": [...]}` —
carries no `type` of its own, so that test failed and the resolver bailed.
_validate_verb_top_level_keys reads None as "no key-set to enforce" and answers
valid. The check therefore did not report a problem; it stopped checking and
reported success, which is strictly worse than failing.
Five verbs are union-shaped in the shipped schema.json:
connect oneOf of 4 $refs (ConnectDeviceSingle/Serial/Parallel/SerialParallel)
play oneOf of 2 $refs (PlayWithURL / PlayWithURLS)
send_sms anyOf of 2 $refs (SMSWithBody / SMSWithMedia)
sleep anyOf of object-with-duration / integer / SWMLVar
unset anyOf of string / array-of-string
Four of the five have object branches with perfectly enumerable keys, and the
shallow resolver was returning "not enumerable" for all four.
REACHABILITY IN THIS PORT — the defect is LATENT, not live. add_verb dispatches:
a verb with a registered HANDLER goes to this shallow resolver, everything else
goes to the deep full-JSON-Schema validator. `ai` is the only registered handler
(SWMLVerbRegistry.__init__ registers AIVerbHandler and nothing else), and `ai` is
a plain closed object, not a union — so no union-shaped verb reaches the shallow
resolver today. Measured end-to-end: add_verb('sleep', {duration:5000,
zzz_forbidden:1}) is REJECTED, by the deep path. No live acceptance is reachable
through a public API in this port. The resolver defect is nonetheless real and
measured at the resolver boundary, and it goes live the moment any union-shaped
verb gets a handler; the go and dotnet ports carry the identical bail.
The resolver is now a shared recursive _closed_key_set handling three node
shapes: a $ref (followed into $defs), a union (resolved branch by branch and
UNIONED), and a plain closed object. The union semantic is the correct one for
this schema shape: a config satisfying an anyOf/oneOf satisfies SOME branch, so a
key belonging to no branch belongs to no valid document. Non-object branches
(sleep's bare integer, SWMLVar) contribute no keys — they constrain the config to
not be an object at all, a different question from which keys an object config
may carry. A depth bound keeps a self-referential $ref from spinning.
Shapes with genuinely no closed key-set stay disengaged, and are pinned by test
so this is not read as "always enforce something": `set` is an OPEN object
(unevaluatedProperties:{} with no `not`), `unset` is a union with no object
branch, and cond/label/return are array/string/untyped.
Engaged verbs go 30 -> 34 (sleep, play, send_sms, connect). The disengaged set is
asserted to be exactly {set, unset, cond, label, return}, so a resolver that
engaged the WRONG four cannot pass.
Negative control, both directions, on this branch:
pre-fix 11 failed / 19 passed
test_union_shaped_verbs_resolve_a_key_set FAIL x4 ("DISENGAGED")
test_union_shaped_verbs_reject_unknown_keys FAIL x4 ("was ACCEPTED")
test_engaged_count_and_membership FAIL ("got 30", want 34)
post-fix 30/30 pass, including the legitimate-config direction and connect's
four branch discriminators (to/serial/parallel/serial_parallel), which
an INTERSECTION computed in place of a union would reject.
No oracle artifact moves: python_signatures.json, python_surface.json and
port_surface_native.json all regenerate byte-identically with and without this
change (_closed_key_set is private, so it is not surface).
Ported from go 2a34105 and dotnet 8aac44c, which fixed the identical bail.
…allback claim `_validate_verb_lightweight`'s docstring said "This is the fallback when jsonschema-rs is not available." That path does not exist: `jsonschema_rs` is imported unconditionally at module scope (schema_utils.py:20) and `_init_full_validator` (:87-89) constructs the validator with no try/except, so an absent or failing dependency propagates — the module simply fails to import. The lightweight path is real, but its actual trigger is a PARTIAL SCHEMA (no `sections` key in the schema's top-level `properties`, :360-364), or a mocked instance whose `__init__` never ran (:334). Reaching it also does not imply `full_validation_available` is False — with a partial schema the validator is still constructed, so that property returns True (:96-99) while every verb is validated lightweight. This docstring is cited as the contract by four ports (rust, dotnet, cpp, php), each of which built a fail-open validator to honour a fallback Python does not have. Correcting the source sentence so the next copy is not made from it. Prose only — no executable line changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DXK6fU1ASoT14yniFHhWRh
…E-FRESH gate SURFACE-NATIVE regenerates the COMMITTED port_surface_native.json in place and nothing verified or restored it, so any change to the native-only surface silently rewrote a committed artifact in the working tree. Every gate still passed; only `git status` showed it, which means a lane reading console output alone would have committed a polluted oracle. Proven live: appending a member to signalwire/livewire/plugins.py made the gate rewrite the sidecar from 42 to 43 members and leave the tree dirty, with the planted name inside the committed file. The fix is the missing half of the pair this repo already uses for its other regenerated artifact — SIGNATURES regenerates python_signatures.json and DRIFT `git diff --quiet`s it. SURFACE-NATIVE now has the same check. Checking rather than restoring is deliberate: the sidecar is a DOC-AUDIT INPUT consumed via --native-names at its real path, so it cannot be redirected into a scratch copy. The gate is ordered deps=DOC-AUDIT so the consumer still reads the freshly-regenerated bytes; diffing any earlier would either starve the consumer or diff a file nothing had written yet. When it fails, the remedy is to commit the regenerated sidecar. The same regenerate-without-check gap existed in .github/workflows/doc-audit.yml. CI's checkout is ephemeral so it cannot leak into a commit there, but a STALE committed sidecar would have passed unnoticed; the added step makes it fail loudly. Verified both directions: the gate exits 0 on a clean tree (no false positive) and exits 1 with the plant present, through the real gate_scheduler.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The reference SDK's half of the wave-6 coordinated pass. 49 commits on top of
main.This supersedes #77 — that PR's head (
fix/win-test-portability-tail) is a directancestor of this branch, so all seven of its commits (Windows portability, the multi-OS
interpreter fix, and the
builddeclaration PACKAGE-SMOKE needed) are already includedhere. #77 can be closed in favour of this, or left open and closed on merge; it has
nothing this branch lacks.
Why the oracle had to move
Two of these commits were the root cause of a fleet-wide CI red today. Every port had been
brought to the enforced SWAIG token contract, so each correctly REFUSES an untokened
serverless call — but CI built its oracle from a reference that still executed it, and
seven ports failed BEHAVIORAL-HTTP for being right:
e4f66b6— enforcesecure=Trueon all four serverless transports. The negative-halfcorpus fixture
http_serverless_lambda_swaigdeliberately carries no__token; thereference now refuses it, matching the ports.
c0183cd— keep SDK logs off stdout. structlog debug lines were interleaving with theJSON that Layer-D dumps parse.
Both sat unpushed on a local branch, which is exactly the staleness the coordinated-pass
mechanism exists to prevent. Confirmed fixed: ruby's re-run went from
CI FAIL (gates: COORDINATED-PASS BEHAVIORAL)to a clean pass.Also in here
71eed0c— removeDataMap.body()(breaking): the builder wrote a key nothing reads.The fleet-wide ripple is done; all ten ports have dropped it.
a23c85b+ the lint burn — REPO-LINT/REPO-FMT wired overtests/,scripts/andeng/,which were gated by nothing. Burned 1291 -> 0 before the gate landed, so it never lands red.
4371610— validatepost_promptshape; the validator was passing configs that abort the call.ee1911c— mypy 85 errors -> 11.2850adf— DOC-SURFACE floor pinned at 100%.Verification
Local
run-ci.shis green on this branch. The gates that matter for the coordinated set —SIGNATURES, DRIFT, SURFACE, EMISSION — all pass, and the two oracle artifacts regenerate
byte-identically.
Coordinated-With: porting-sdk@wave6/ctor-dunder-fold