fix(runtime): eight defects in spend accounting, approval visibility, memory and platform support by mkthoma · Pull Request #19 · theschoolofai/S16Code

mkthoma · 2026-08-14T15:30:29Z

Eight defects across seven commits, found by running the agent end to end on
Windows 11 with Python 3.14 against live channels, rather than by reading the
code.

They are in one PR because they were found in a single session. Each commit is
self-contained and cut from main, so this splits cleanly if you would prefer
separate reviews.

The first three are the ones worth your attention. The rest are smaller, and two
are platform support.

1. Both dollar ceilings can never fire

s16code/gateway.py, s16code/events/engine.py, s16code/runtime.py

What breaks. budget and daily_budget are configured, displayed, and
never enforced. A run cannot exceed a ceiling because the ceiling never sees a
non-zero number to compare against.

Root cause. chat() returns a whitelisted subset of the gateway response,
and cost was not in the whitelist. Spend therefore arrived as zero at every
layer above it. _spend_of read zero, the governor compared zero against the
ceiling, and the comparison could never trigger.

This is the most consequential of the eight, because a governor that cannot
enforce its own ceilings is worse than having none. The operator reads the
configuration, believes there is a limit, and there is not.

The fix. Thread cost through the whitelist and record it against the
subscription so both the per-run and per-day ceilings compare against real
spend.

Test. tests/test_autonomy_spend_accounting.py asserts cost survives the
gateway boundary, that it accumulates against the daily budget, and that a run
exceeding its ceiling is refused.

2. A parked question is announced nowhere

s16code/events/report.py, s16code/events/routes.py

What breaks. When a run parks on request_approval it waits durably, which
is correct. But nothing surfaces that it is waiting. Not the morning report, not
any endpoint. The run waits indefinitely for an answer the operator has no way
of knowing is wanted.

Root cause. The parked state is recorded in the graph but no projection
reads it, so there is no path from a waiting node to anything a human looks at.

The fix. Surface parked approvals in the report and on the events route, so
a waiting run is visible in the places an operator already checks.

Test. tests/test_parked_approvals_are_visible.py asserts a parked run
appears in the report and on the route, and disappears once answered.

3. The agent's own answers come back as evidence, so a wrong answer entrenches

s16code/core/memory/store.py

What breaks. A wrong answer becomes its own best-ranked support for
repeating itself, and confidence grows with each repetition rather than
decaying.

Root cause. recall() returned the agent's own prior answers alongside
genuine evidence, with no distinction between something the agent was told and
something the agent said. Semantic similarity then ranks a previous answer very
highly for the question that produced it, which is the worst possible ranking
behaviour here.

The fix. Add include_own_answers: bool = False and exclude answer records
in SQL by default. Deliberately narrow: older episodes remain usable and callers
that genuinely want the agent's own output can still ask for it.

Test. tests/test_recall_excludes_own_answers.py asserts an answer written
by the agent is not returned as evidence for a later related question, and that
ordinary episodes still are.

4. Replies disclose the host filesystem to third-party channels

s16code/runtime.py, s16code/capabilities.py, s16code/tools.py,
s16code/core/redaction.py, s16code/core/live_graph/core.py

What breaks. An answer citing its source carried an absolute host path,
which then went out over a third-party channel, disclosing the operator's
account name.

Root cause. Evidence dictionaries carried the raw path from the capability
that produced them, and nothing removed it before that evidence entered the
model's context or the durable run state.

The fix. Never put a host filesystem path into the model's context. This is
done at the capability boundary rather than at the edge, because redacting only
on the way out leaves the path sitting in durable run state, where it
resurfaces on resume. A companion fix on the gateway side covers the wire.

Test. tests/test_no_host_paths_in_evidence.py and additions to
tests/test_capability_contracts.py assert no host path reaches evidence text,
for both Windows and POSIX forms.

5. `tzdata` is not a declared dependency

pyproject.toml, uv.lock

What breaks. Every timezone lookup raises ZoneInfoNotFoundError on
Windows.

Root cause. zoneinfo has no bundled timezone database on Windows and
relies on the tzdata package, which was never declared. On Linux it works by
accident, because the system database is present.

The fix. Declare tzdata.

6. `file://` URI parsing breaks on Windows

s16code/runtime.py

What breaks. file:///C:/... resolved to a path that does not exist, so
reading a file by URI failed.

Root cause. The URI path component was used directly, leaving the leading
slash in front of the drive letter.

The fix. Use url2pathname, which handles the platform difference.

Test. Added to tests/test_runtime_regressions.py.

7. UI pages are decoded with the locale encoding, not UTF-8

s16code/ui/routes.py

What breaks. On a non-UTF-8 Windows locale, every page containing a
non-ASCII character raises UnicodeDecodeError and the console does not load.

Root cause. The pages are read without an explicit encoding, so Python uses
the locale default, which on Windows is typically cp1252.

The fix. Pass encoding="utf-8" explicitly.

8. Morning report is shown as raw markdown in the console

s16code/ui/client/console.html

What breaks. Cosmetic but user-facing. The report is generated as markdown
and was displayed literally, asterisks and hashes included.

The fix. Render it.

A note on the A2A test, not fixed here

s16code/core/a2a/tests/test_hardening.py::test_official_subscription_resumes_waiting_graph_and_maps_cancel
is flaky, and it is flaky on main independently of this PR. I ran it four
times on a pristine checkout of c166526 with none of these changes present and
saw two failures and two passes, then three times on this branch and saw two
failures and one pass.

I have not tried to fix it, since diagnosing it properly is a separate piece of
work, but it is worth knowing that a red run on this test is not necessarily a
regression.

Deliberately excluded

config/tiers.yaml and sandbox/reminders.txt are specific to my local
deployment and demo fixtures. They are not in this branch.

Testing

Windows 11, Python 3.14.4, run against this branch: 377 passed, with the flaky
A2A test above deselected. With it included the result is 377 passed and 1
failed, for the reason described.

`current_datetime` accepts an IANA timezone and hands it to `ZoneInfo`. Windows ships no system tz database, so `zoneinfo` falls back to the `tzdata` package, which is not declared as a dependency. Every zone except UTC therefore raises `ZoneInfoNotFoundError` on a clean Windows install. That is not a corner case for this project: an assistant that cannot resolve "today" in the user's own timezone answers date questions wrongly, and the shipped test for it fails on a fresh checkout. Declared as a marked dependency rather than an unconditional one, since POSIX systems already have the database and do not need the wheel: "tzdata; platform_system == 'Windows'" A maintainer on Linux or macOS cannot reproduce this, which is worth stating explicitly in the pull request.

`verify_artifact` resolved a file URI with: parsed = httpx.URL(task.input["uri"]) candidate = Path(str(parsed.path)).resolve() `Path.as_uri()` puts a slash before the drive letter, so a Windows artifact URI reads `file:///C:/Users/...` and its path component is `/C:/Users/...`. Handing that to `Path()` produces `\C:\Users\...`, which is not a valid path, so every `verify_artifact` call on Windows fails on an artifact the run had just written itself. `url2pathname` does the platform-correct conversion. It also percent-decodes, which is why the value must not be unquoted first: doing both turns a `%20` in a filename into a space and then decodes the space again. The same mistake is in the shipped test, which used `uri.removeprefix("file://")` and therefore only ever passed on POSIX. Both sites are corrected, so the test now exercises the fix rather than agreeing with the bug. Invisible to a maintainer on Linux or macOS, where the path component happens to be a valid path already. Worth saying so in the pull request. Tests: tests/test_runtime_regressions.py, existing test corrected; 6 pass.

… enforce `daily_budget` and `daily_triage_budget` are enforced by comparing recorded spend against the ceiling. Spend was never recorded as anything but 0.0, so neither comparison could ever be true and neither ceiling could refuse anything. The morning report also showed $0.00 regardless of what was spent. `max_runs_per_day` is unaffected, because it counts rather than sums money. The agent was therefore bounded by one of its three controls rather than three. Four independent causes, all of which had to be fixed: 1. `_spend_of` looked for `cost_usd` or `usd`. The economics controller writes its per-call cost under `cost` (economics.budget.Charge), so every real metered call summed to zero. 2. `GatewayClient.complete()` discarded the gateway's `cost` object and returned only text, provider, model and token counts. The gateway prices every call and returns `cost.total_usd`; the price was known at the boundary and thrown away one layer later. This is the triage path, which is exactly what `daily_triage_budget` bounds. 3. `spend_usd` was read but never written. The engine records run spend from `result.get("spend_usd")`; grepping the package finds four references, all reads. `RunBudget.spent` had always accumulated the real figure and was simply never surfaced. 4. `chat()` dropped `cost` before `complete()` could read it. This one only became visible after 2 was fixed and live triage still recorded zero: `complete()` does not call the gateway, it calls `chat()`, which rebuilds the response as a field whitelist that never named `cost`. Measured with the fix for 2 already in place: gateway POST /v1/chat -> cost.total_usd = 1.5e-06 GatewayClient.complete -> cost_usd = 0.0, metered_calls = [] Cause 4 is worth calling out in review: the first round of tests for this fix were all green while the bug was still live, because each started from a reply dict that already carried a price. They pinned the arithmetic and never asserted that a price arrives at all. Tests: tests/test_autonomy_spend_accounting.py, 9 tests. Two stub the HTTP layer and assert the price survives `chat()` and reaches `_spend_of`, which is the seam that was actually broken. The last drives the governor directly and asserts a spent-out `daily_triage_budget` returns a refusal with that control name, which is the behaviour the ceiling exists for.

`morning_report` returns an `awaiting_a_human` key that was hardcoded to `[]`, and `render_markdown` never emitted a section for it. The module docstring promises this section; nothing produced it. That matters more here than a missing field usually would. A run started by an event has no conversation to reply into, so when it stops to ask something, nothing is sent anywhere: not to a channel, not by email, not on completion. The morning report is the only surface that can announce a parked question, and it announced nothing. An operator reading a clean report could not tell "a quiet night" from "the agent is waiting on you and has been for hours". `_awaiting_a_human` walks the decisions in the window and lists every run whose `run_status` is `waiting`, with the event, the subscription and the question. The graph parameter is optional but load-bearing. A decision record keeps the status it had when it was written, so without live node state the report keeps listing questions that were answered hours ago. When a graph is supplied the node state decides, and a run whose gate has since succeeded drops off. The report still works without one, which keeps it usable in tests and anywhere a runtime is not available. The route passes the runtime's graph via getattr, so a deployment without a runtime attached degrades to the old behaviour rather than failing. Tests: tests/test_parked_approvals_are_visible.py, 3 tests. A parked run is listed, the markdown contains the section, and a run whose gate has succeeded drops off once a graph is available.

Two defects in the operator console, in one pull request because they are the same page and the second is only visible once the first is fixed. 1. Every page is decoded with the host locale. `path.read_text()` uses `locale.getpreferredencoding()`, which on a Windows install is cp1252, not UTF-8. The console, its client and the run page are UTF-8 files containing typographic characters, so each one is decoded with the wrong codec and served mojibake. A refresh control reading "\u21bb Refresh" arrives as the three cp1252 characters for its UTF-8 bytes ("\xe2\x86\xbb"), and any page containing a character outside cp1252 raises UnicodeDecodeError and returns HTTP 500 instead of a page. Three call sites, all fixed by naming the encoding. This is invisible on Linux and macOS, where the preferred encoding is already UTF-8. 2. The report tab shows markdown source rather than a report. The console fetches `/v1/agent/report?fmt=markdown` and inserts the result as text, so an operator reads `## Awaiting a human (1)` and a wall of hyphens and backticks. The morning report is the deliverable of an unattended night and the one artifact meant to be read by a person, and it was the least readable thing on the page. The client now renders headings, lists, bold, code spans and paragraphs. Deliberately a small renderer over the report's own limited vocabulary rather than a markdown library: the console ships no bundler and no dependencies, and the report is generated by code in this repository, so the subset is known rather than arbitrary. Text is escaped before any markup is produced, so a refusal reason containing angle brackets cannot inject HTML. Tests: tests/test_ui_invariants.py and tests/test_control_plane_auth.py, 20 tests, covering that pages are served, that they stay read-only, and that the report tab renders rather than echoes.

Every answer is written to memory as an episode so a conversation has continuity, and `recall()` then returns those episodes as ambient evidence. That closes a loop with no damping: an answer is a near-perfect lexical match for the question that produced it, so it becomes the top hit the next time that question is asked, and the agent ends up citing itself. Observed. Asked three times about a conference invitation held in a sandbox file, recall returned the agent's own three previous "there is no information about it" replies as the top three hits. The file had said otherwise the whole time. Each attempt added another denial, so rephrasing made it worse rather than better, and the agent grew steadily more confident about something it had never checked. The discriminator is exact and was already present: this store writes `run://<run_id>/answer` as the source of every answer it records, while an inbound message carries `channel://<channel>/<id>`. On the database this was found on, that separates 17 self-authored answers from 33 user messages with no ambiguity. This follows the precedent three lines above it in the same function, which excludes `audit` and `policy` records with the comment "control-plane data, never ambient answer evidence". An answer the agent produced belongs in that category for the same reason: it is output, not observation. Scope is deliberately narrow: - what the user said is still evidence; - `kind=fact` records are untouched, which matters because cross-channel recall depends on them; - documents and chunks are untouched; - `include_own_answers=True` is available for a caller that genuinely wants "what did you tell me earlier", mirroring the existing `include_history` flag. Tests: tests/test_recall_excludes_own_answers.py, 6 tests, 3 of which fail with the change reverted. They pin both halves: that a denial cannot come back, and that a fact is not crowded out by an answer contradicting it.

…ntext A Telegram reply delivered to the operator: Based on your reminders list (`file://C:\Users\<user>\...\sandbox\reminders.txt`) and today's date ... That discloses the operator's username, home directory and installation layout to a third party. A sent message cannot be recalled, so the disclosure is permanent at delivery. Nothing failed: the message sent successfully, looked helpful, and was wrong in a way no error surface reports. Root cause. `read_file` returned the resolved absolute path, and its evidence projection interpolates that value into the `[source: ...]` annotation the answer worker reads. The model was shown a host path and told to cite its sources, and did exactly that. The delivered text used backslashes, which identifies the producing site: `Path.as_uri()` emits `file:///C:/...` with forward slashes, so only `str(path)` interpolated into `source_template="file://{path}"` can produce `file://C:\Users\...`. A path the model can read is a path the model can quote, so no capability result may contain one: - `read_file` returns the caller's relative path; its projection emits `sandbox://{path}`. - `index_file` stores `sandbox://` in every chunk's SourceRef, which `memory_recall` would otherwise replay indefinitely. - `write_text_file`, `file_sha256` and `copy_file` return relative paths and no file:// URI. `file_sha256` is the live one: it is not a side effect, so it is plannable on any run without appearing in an allowlist. - `create_calendar_events` keeps absolute `artifacts` as a machine contract for `verify_artifact` and exposes `artifact_names` to the projection. Artifacts live under S16_DATA_DIR, which defaults to the home directory, so this leaked the username even though the sandbox is elsewhere. - `verify_artifact` narrows only its return; its input contract is untouched. - Worker exceptions are scrubbed in `LiveGraphExecutor._execute`, because an OSError carries the filename it failed on into the durable journal, the planner's next prompt and the answer evidence at once. - `generic_evidence` and `resolve_sources` refuse to promote any source naming a host path, and the serialised body is scrubbed. A denial rather than a scheme allowlist: an allowlist silently discards legitimate provenance the day someone adds a scheme. This is the producing half. The gateway also redacts at the outbound envelope, in its own pull request. Both are wanted: the redactor alone leaves the model reasoning over the path before anything is sent, and approval questions never pass through the agent's reply path at all. Tests: tests/test_no_host_paths_in_evidence.py, plus an added case in tests/test_capability_contracts.py asserting a host-path source is refused and falls back to internal provenance rather than vanishing. Note for the reviewer: on Windows, test_calendar_skill_... fails on this branch for an unrelated reason. It uses `uri.removeprefix("file://")`, which yields an invalid path on Windows; that is a separate defect with its own pull request.

theschoolofai · 2026-08-18T07:10:04Z

Session 16 — graded ✅

Score: +100.

Eight defects across spend accounting, approval visibility, memory and platform support, found by running the system rather than reading it.

mkthoma added 7 commits August 14, 2026 16:21

Navigation Menu

fix(runtime): eight defects in spend accounting, approval visibility, memory and platform support - #19

fix(runtime): eight defects in spend accounting, approval visibility, memory and platform support#19
mkthoma wants to merge 7 commits into
theschoolofai:mainfrom
mkthoma:fix/runtime-and-governor-defects

mkthoma commented Aug 14, 2026

Uh oh!

theschoolofai commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mkthoma commented Aug 14, 2026

1. Both dollar ceilings can never fire

2. A parked question is announced nowhere

3. The agent's own answers come back as evidence, so a wrong answer entrenches

4. Replies disclose the host filesystem to third-party channels

5. tzdata is not a declared dependency

6. file:// URI parsing breaks on Windows

7. UI pages are decoded with the locale encoding, not UTF-8

8. Morning report is shown as raw markdown in the console

A note on the A2A test, not fixed here

Deliberately excluded

Testing

Uh oh!

theschoolofai commented Aug 18, 2026

Session 16 — graded ✅

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

5. `tzdata` is not a declared dependency

6. `file://` URI parsing breaks on Windows