Merge upstream/main: ask command + flush-race reconciliation - #28
Merged
Conversation
…lready ship (NateBJones-Projects#83) * Fold the useful half of the token-saver PR into the skill we already ship PR NateBJones-Projects#82 proposed a second "token-saver" skill plus a pre-call HTTP gateway (~7,200 lines). Most of it was either already enforced by ringer.py or didn't work: the gateway requires your own API key, so it converts a flat-rate OAuth plan into metered API billing, and its own docs say it is incompatible with Claude Code. Ringer already gets the same saving in a better shape — every task spawns a clean worker that sees only the spec, opt-in and verified by an executed check. What's kept: - `ringer.py ask` — one bounded, read-only question answered by one clean worker over a selected context packet. This fills a real gap: such a question needs a model but not a manifest, and answering it inline pulls whole files into an already-expensive context. The packet selector is INLINED rather than added as a sibling module, so ringer.py stays single-file per CONTRIBUTING. - `max_attempts` and `redact_spec` as general manifest fields. What changed from NateBJones-Projects#82's version: - Redaction is opt-in (`--redact`), not hardcoded on. NateBJones-Projects#82 redacted every `ask` call, which made those runs unreviewable. - `ask` shows on Ringside and registers an artifact. NateBJones-Projects#82 suppressed both, contradicting the skill's own "runs are watched, not hidden" rule. - A matching passage that overflows the packet budget now says so and names the byte figure to raise the cap to. It previously reported "no passage matched the request", sending the reader after a selection bug that did not exist. The skill gains an `ask` section placed BELOW the four rules and labelled as their one sanctioned exception, stating plainly that its check proves an answer was produced, never that it is right — there is nothing to execute against free-form prose. Plus a section on the orchestrator's own token discipline, which the playbook never covered. Supported Python floor moves 3.11 -> 3.12: CI has only ever run 3.12, so 3.11 was a promise nothing enforced. Not carried over: the gateway, the pre-call router, the separate skill and its duplicate `.agents/` tree, the installer, and the codex-thin engine. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Close a symlink escape in ask's directory scan, and three review findings Pre-merge review (GPT-5.6 Sol, high effort) returned DO NOT MERGE. Findings confirmed by reproduction, and fixed here. BLOCKER — directory scans could read outside the tree you named. Every filename check in source_files ran on the DIRECTORY ENTRY, and only afterwards was the path resolved and read. A symlink with a benign name therefore pulled its target's contents into the packet, bypassing the sensitive-filename filter entirely. Reproduced: inside/safe-notes.md -> ../outside/secret-private.txt put the secret straight into the packet that would have been sent to a provider. Scans now require the resolved path to stay inside the named tree and re-run the name checks on the real target. An explicitly supplied file is exempt: naming it is consent. Also fixed: - The runtime guard still admitted Python 3.11 while the docs had moved to 3.12. It now matches what we say and what CI runs. - `ask` did not start Ringside, so a run could begin against a dark watch page — contradicting the prose added in the previous commit. It now calls ensure_hud_running like `run` does. - `max_attempts` and `redact_spec` accepted truthy stand-ins: `1.5` became 1 and silently removed the retry, and the string "false" became True. Both are now type-checked. Note `timeout_s` and `full_access` coerce the same way on main; that is pre-existing and left alone deliberately rather than changed under cover of this PR. Two prose overclaims corrected. The README said `ask` refuses when nothing matches; it does not — a source small enough to fit is included whole regardless of relevance, and the docs now say so, because choosing the sources is the real work. And redaction was described as keeping the request "out of the log", when it covers Ringer's own state and eval records only — raw worker output is captured verbatim by invariant, so a worker that echoes its request still writes that text to worker.log. Declined: the reviewer wanted relevance gating so `ask` would refuse unrelated small sources. Including a small file wholesale is cheap and more likely to answer; the defect was the documentation promising otherwise, so the prose moved rather than the behaviour. 9 new tests, including the reproduced escape and its in-tree variant. 250 tests green across three consecutive runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…es-Projects#84) `test_sigterm_cleans_up_active_worker_and_finishes_state` and `test_second_signal_during_shutdown_does_not_cancel_cleanup` failed about 30% of runs (measured 4/12 and 3/12 on main). Not timing sensitivity in the tests — a real race in StateWriter. StateWriter.flush wrote `<run>.json.tmp`: one fixed temp name shared by every caller. The background writer thread flushes on a timer while explicit flushes come from set_port, close, and the signal path, so two flushes overlap routinely. Both wrote the same temp file; the first os.replace consumed it and the second raised FileNotFoundError on a path that no longer existed. That propagated to the top-level handler, which exited 2 — so the test asserting 130 saw 2 and failed. The same fixed name appeared again in the end-of-run re-flush. Both sites now use atomic_write_json, already in the file and already correct: tempfile.mkstemp gives each writer its own temp file, and the final os.replace is atomic, so overlapping flushes are last-write-wins instead of one of them crashing. After the fix, 20/20 passes on each of the two tests, and 4 consecutive clean full-suite runs. Before, the suite failed roughly every other run. Verified the new tests actually catch it: run against the pre-fix commit, test_parallel_flushes_never_lose_their_temp_file fails with exactly the FileNotFoundError described above. The other two guard adjacent invariants (no torn reads, no leaked temp files) and pass either way. Left alone deliberately: _write_active_runs and write_settings use `.{name}.{pid}.tmp`. They are called from the main thread at run start and end, so the pid suffix is sufficient there, and there is no observed defect to justify touching them in this change. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…2026-07-29 # Conflicts: # docs/MODEL-NOTES.md # ringer.py
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
2 participants
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.
Brings the fork current with
upstream/main(2 commits) and reconciles the headline conflict: two independent fixes to the same flush race.The reconciliation — both layers kept, deliberately
Upstream
a1a91b8fixed the StateWriter temp-file race by switching both flush sites toatomic_write_json(per-writermkstemp, last-replace-wins). Our fork fixed the same race on 2026-07-18 with_flush_lockserialization. These are complementary, not redundant:.json.tmpFileNotFoundErrorcrash (upstream measured it as a ~30% shutdown-test flake).stop()'sjoin(timeout=2)can return with the writer thread mid-flush, and a stale in-flight snapshot can then overwrite the finalreport_ready=Truestate — Ringside polls that file.Resolution:
with self._flush_lock: atomic_write_json(...)at both sites, with the layered rationale in the comment. Upstream's newtest_state_writer_concurrency.pypasses unchanged under the combined form.Also in this sync (
deda7c1)ringer.py ask— one bounded read-only question, one clean worker, an inlined context-packet selector — plusmax_attempts/redact_specmanifest fields,tests/TESTING.md, and Python floor 3.11→3.12. Upstream's SKILL.md changes auto-merged cleanly around ourexpect_filesbullet (verified present). MODEL-NOTES union-merged, both evidence sets kept, per house style.Verified
ask --helpsmokeexpect_filesskill bullet🤖 Generated with Claude Code