feat(read-only): two explicit levels, so asking for read-only means something (re-land of #367) - #416
Merged
Conversation
…omething #303 asked whether the write gate should refuse the tools that PUT a scratch class on their main path. The answer is not one gate but two, because "can it change my code?" and "can it touch my instance at all?" are different questions and only the asker knows which they mean. IRIS_SOFT_READ_ONLY=1 every declared mutator refused; the scratch-class readers still answer, and still write a class to IrisDevTmp to do it IRIS_STRICT_READ_ONLY=1 also refuses those, and drops them from the advertised list ## Two precedence decisions, because getting either wrong makes the switch worthless An explicit request BEATS `IRIS_ALLOW_PROD`. That variable forces writes on, and if it kept winning, an operator with it exported — precisely the environment where asking for read-only matters — would silently get writes while believing they had asked for none. Order: IRIS_STRICT_READ_ONLY > IRIS_SOFT_READ_ONLY > IRIS_ALLOW_PROD > the inferred gate The last two links are untouched, so a deployment setting neither new variable behaves exactly as it did before. `the_inferred_gate_is_untouched` re-asserts the whole pre-#303 truth table so the new precedence cannot quietly alter the old behaviour underneath it. Strict both FILTERS and REFUSES. The tools leave the advertised list and are refused at call time. The filter is the UX — a model does not reach for a tool it cannot see — and the refusal is the boundary, because a client can call a tool that was never advertised. This deliberately differs from the inferred gate, which prunes nothing: that gate follows the CURRENT connection and can flip under a reconnect, so hiding would be worse friction than refusing. A mode read from the environment at startup cannot flip. ## The count in this issue's title is wrong It says "the ten tools". GENERATOR_WRITE_TOOLS holds ELEVEN — #343 added iris_gateway_manage and #282 removed two once their reads stopped needing a generator. The tests derive and print the number rather than asserting a literal, with a vacuity floor instead of a count, because every count stated in prose in this repo has gone stale. ## Testability drove the shape `read_only_mode()` caches for the life of the process on purpose: a gate that can change its answer mid-run cannot be reasoned about. That also means no in-process test can vary it, so both decisions are pure functions taking the mode as an argument: * `write_allowed_with(requested, system_mode, namespace, allow_prod)` — the whole precedence chain in one testable expression; `is_write_allowed` is now a thin env-reading wrapper * `strict_refuses_scratch_write(mode, tool)` — used by BOTH the filter and the refusal, so the two cannot drift into disagreeing about which tools exist * `advertised_tools_with(mode)` — because otherwise the one line that removes the tools is untested, and a deleted `retain` looks exactly like a working filter when the level is Off Taking the mode as an argument is also what makes the CONTROL possible: "strict refuses these eleven" is equally satisfied by a build where nothing works, and only "soft still allows them" separates the two. If `soft_allows_every_one_of_them` ever reddens, soft has become strict and the useful level is gone. ## Two things the existing code got wrong once these levels exist `write_gated_error` advised "set IRIS_ALLOW_PROD=1 and restart". Under an explicit request that is FALSE — the variable no longer wins — and it hands the caller a bypass for a decision made deliberately. An explicit request now reports itself as one, names no way around it, and the envelope carries `requested_read_only` so a caller can tell a deliberate choice from a heuristic about the instance. Strict needed its OWN code, not WRITE_GATED. The caller did not ask to mutate anything; these are reads. "Your write was refused" would misdescribe what happened and give no next step, so SCRATCH_WRITE_BLOCKED says the answer requires writing at all and that soft permits exactly that. `check_config` reports `read_only_requested` through the same call that enforces the gate. `write_tools_enabled: false` cannot distinguish a Live instance from an explicit request, and only one of those can be lifted — upstream's #110 was a flag reading false while every write landed. ## A third emission route in the remedy gate, found by this change Adding SCRATCH_WRITE_BLOCKED made `no_remedy_entry_is_stale` call the brand-new row stale. `McpError::invalid_params(msg, json!({"error_code": "X"}))` is a complete emission route that goes nowhere near `err_json`, so the gate never read it — and it was hiding three pre-existing codes with no reviewed remedy, WRITE_GATED among them: the write gate's own refusal. The scanner now reads that route and all four have rows. ## Mutations, predicted before running, all six red drop the explicit-request short-circuit -> an_explicit_request_beats_allow_prod invert precedence (allow_prod first) -> an_explicit_request_beats_allow_prod soft behaves as strict -> soft_allows_every_one_of_them + the wiring test any non-empty value counts as a request -> only_affirmative_values_turn_a_level_on delete the advertised-list retain -> the wiring test soft checked before strict -> strict_beats_soft_when_both_are_asked_for Restored: 1080 passed, both touched files byte-identical. Worth recording: the FIRST run of that harness reported all six as red with zero tests executed — `cargo test` takes one TESTNAME and it was passed three, so every run including the baseline exited non-zero having run nothing. The baseline is the only thing that distinguished a working mutation from an instrument that never ran. The harness now refuses to report a verdict when no `test result:` line appears, and prints cargo's own words instead. Refs #303 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
One file needed a hand resolution, recorded because a line-wise resolve would have been wrong.
Resolved by taking each module whole from the branch that owns it ( Checked before committing: no markers remain; |
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.
Recovery PR — no new work. This re-applies the content of a PR that GitHub marked MERGED but whose
changes never reached
master.What happened
#366 and #367 were opened with
baseset tofix/remedy-gate-reads-wrapped-calls(#364's headbranch) rather than
master. When I squash-merged them, GitHub merged them into that stale featurebranch, which had itself already been squashed into master a moment earlier. Both PRs are now
closed as merged and neither one's changes are on master.
The cause is narrow and worth recording: GitHub only auto-retargets an open PR to the default branch
when its base branch is deleted. I merged with
--squashand no--delete-branch, so the stalebase survived and the retarget never happened. The same defect was present on three still-open PRs
(#376 → #375's branch, #377 → #376's branch, #381 → #372's branch); those have been retargeted to
masterbefore merging, and merges from here on delete the branch.Nothing was lost — both head branches survived intact, each with exactly one commit of its own.
What this PR contains
The original commit, re-applied onto current
master. CI ran green on the original PR; it isre-running here because master has moved (#363, #364, #365 landed in between).
🤖 Generated with Claude Code
https://claude.ai/code/session_01N7fLbq3ftb82ub45QCpsPB