fix(cli,provider): six defects from the edge-case round, and the scan that could not see one - #55
Merged
Merged
Conversation
…lf-confining it `task_sandbox.policy` builds TWO controls over the same paths, and they disagree. The sandbox's `filesystem` block has `denyRead` **and** `allowRead`, and there the allow really does win — a workspace under a denied `$HOME` stays readable, which is what `tests/e2e_agent_sandbox.py` measures against the real binary. The `permissions` block has only `deny`, because Claude Code's permission layer has no allow that beats one, so the same workspace is covered by `Read(//$HOME/**)` with nothing to re-allow it. MEASURED 2026-08-20 on Claude Code 2.1.234, two arms, same prompt and same project shape: with `GRID_TASK_ROOT` inside `$HOME` the `Write` tool returns `is_error: true` and the agent says so in its own words — *"blocked by a Read deny rule in your permission settings"* — then works around it with a shell redirect: 5 turns, 24s. With the root outside, `Write` succeeds first time: 3 turns, 15s. **The task reports `completed` either way**, so the whole cost is invisible; a less capable agent fails a task that should have run and nothing anywhere says why.⚠️ **Refused rather than repaired, because the repair is not available.** The obvious fix — drop the ancestor from `denyRead` to make room for the workspace — is only sound for the sandbox layer. In the `permissions` layer it would hand the in-process `Read` tool the operator's whole home directory, which is the hole that layer exists to close (measured: without it, a run with the entire sandbox above still read a file outside the workspace). Checked in two places, and the second is not redundant. `policy` is where the two lists are built, which is where the invariant belongs — the argument `task_agent._safe_segment` makes for validating a path at the point it is constructed. But `policy` runs from `agent_argv`, which is built AFTER the task's checkout, so that guard alone reports a configuration error on a task that has already fetched a repository. `preflight` therefore takes the task root too, and runs the check BEFORE the socket probe: an operator whose root is in the wrong place should be told that, not made to wait on an unrelated probe that is about to succeed. The root is passed IN rather than read there, for `_task_root_of`'s reason — `task_agent` imports this module, so asking it would close a cycle.⚠️ `test_the_workspace_stays_readable_even_when_it_sits_inside_the_denied_home` is OVERTURNED, and rewritten in place rather than deleted. Its assertion was not wrong — `allowRead` does beat `denyRead` in the sandbox layer, and `test_build_caches_survive_the_denied_home` still pins that, since every build cache it checks sits inside the same denied `$HOME`. What it did not cover is the second layer. The replacement lives at the same address so nobody re-derives the old rule from a gap in the file. Measured live after the fix: with the root inside `$HOME` the task now fails in 12s with a message naming the variable, the offending path, why it cannot be repaired, and where to point it — before any model call, so it costs no agent tokens. With the root outside, the same task completes in 15s.
…filesystem does `_safe_segment` refused the object store's own directory name with `==`, and the machines a provider actually runs on do not compare that way. APFS and NTFS are case-insensitive by default, so `STORE.GIT` and `store.git` are ONE directory there — measured on this Mac: write `store.git/marker`, read `STORE.GIT/marker`, and the store's own content comes back. So the guard admitted, in all three positions, the exact collision it exists to refuse: a workspace built on that name sits on top of the member's whole git history. `Path.resolve()` does not normalise case either, so comparing resolved paths would have missed it too — the comparison has to be on the NAME. Not reachable today (a conversation id is a relay-minted UUID, so nobody chooses the spelling), but the guard's own docstring says it is applied at all three levels "rather than only the one where the collision is reachable" — it is written as defence in depth, and a case-sensitive compare is what breaks that intent, on the platform the fleet runs. `STORE.GIT` and `Store.Git` join `_HOSTILE_SEGMENTS`, so the existing parametrized test covers both spellings in all three positions: 6 red rows before, 27 other rows green throughout as the positive control. A second test measures the FILESYSTEM rather than assuming it — it reports which world it is in instead of skipping, because the box that runs this suite is not the box that runs a provider, and it pins that `resolve()` still does not normalise case so the note above cannot rot silently. Mutation: restoring `==` kills the new rows. Found in the 2026-08-20 edge-case round (ND-11).
Every eviction sweep on every macOS provider announced that "this provider's workspace bound is not being fully enforced", once per sweep, forever. The cause was a `.DS_Store` — written by Finder the moment anybody opens the folder, chosen by nobody. `_conversations` handed each child of `projects/` to `_subdirectories`, which listed it, got `NotADirectoryError` for a file, and said the one true thing it can say when a real listing fails. But a file there is not a failure: it is not a project and can never hold a candidate, and the sweep went on enforcing the cap correctly over every genuine project in the same call. So the warning was true of the entry it named and false of everything a reader takes it to mean — impact nil, sentence alarming. That pair is what teaches an operator to ignore a warning, which costs the next one that matters. Fixed by not asking: `projects.iterdir()` is filtered with `_subdirectories`' own predicate, so a non-directory never reaches it and the warning keeps its meaning for the case it was written for (the sibling test still holds it to that). A failure to list `projects/` ITSELF is still reported, unchanged. Symlinks are excluded with it, deliberately and for a second reason: a symlinked project directory would let this sweep walk — and `rmtree` — through a link out of the tree entirely. The test asserts BOTH halves, and the eviction half is the positive control: a test that only read stderr would pass just as well against a sweep that had silently stopped working. Mutation: restoring the bare `iterdir()` kills it.⚠️ The first write-up of this called it "a `.DS_Store` disables eviction". That was wrong and the same round's own evidence refuted it — E4 evicted 8→1 with the file sitting there. `_conversations` uses `or []`, so a child that cannot be listed costs only its own candidates. What was left was the wording, which is all this changes. Found in the 2026-08-20 edge-case round (ND-18, LOW).
… held in a local
`grid task create` told a brand-new user "Project <id> has no main yet, so there
is nothing to cut a task from" — git vocabulary at the FIRST wall anybody meets,
`project create` then `task create` — while `tests/test_application_surface.py`
reported 18 passed.
Both halves are fixed here, because either alone leaves the trap armed.
The scan. `_printed_strings` walks the SINKS and collected only the `Constant`
pieces of an f-string, so a sentence reached through a `Name` was invisible:
`_no_trunk_message` assigns its first sentence to a local `head` and returns
`f"{head} …"`, from which the walker yielded the punctuation and nothing else.
It now resolves the function's own `name = <literal>` assignments, through both
`FormattedValue` and a bare `Name`.
Deliberately shallow — one pass, no flow analysis, and a local built out of
another is not chased. Depth is not what this scan lacks, and the bound is what
keeps it usable: a local that never reaches a sink stays invisible, which is
what keeps a wire value like `kind = "merge"` out of the report. Widening to
every module-level string was rejected before for that exact reason, and that
reasoning is unchanged.
This is the THIRD instance of one structural hole, and the first that
`_SENTENCE_TABLES` could not have caught — `_DEFAULT_WARNINGS` and
`_MERGE_TURN_LABEL` were both module-level constants, and an explicit list was
the answer to them. A list cannot reach a function's local, so the walker had to.
The sentence. "has no files yet, so there is nothing to start a task from" —
paired with `_project_ready`'s own "is ready to work in", which is the state this
one is the absence of, so the two sides read as one thing.
Order, and it is the evidence: the walker test went red first; fixing the walker
turned the git-vocabulary scan itself red on `cli/remote_task.py:543 says 'main'`;
fixing the sentence turned it green. Blast radius was measured BEFORE the fix
landed — exactly one finding, zero false positives across all eleven handler
modules, and `counted` rises to 321.
The new test carries five rows: a direct constant (the positive control, without
which a walker returning nothing would pass the rows that matter), the f-string
local, `print(local)`, `return local`, and a wire value that must stay invisible.
The two bare-`Name` rows exist because a mutation sweep found the f-string row
does not reach that arm — disabling it left the test green. Both arms are killed
now.
Found in the 2026-08-20 edge-case round (ND-06, MEDIUM).
`grid task cancel` told the user "Whatever it had already done is kept: grid task fetch <id>", and at the moment it says so nobody knows that. Cancel returns as soon as the relay records it while the agent runs on until the next lease beat, so what will finally be recorded is unknown at this end. Measured on a live grid 2026-08-20: cancel a running turn, fetch it, and the tree that comes back is the task's INPUT, with none of the agent's edits in it. `_task_fetch` was already honest about this — it says "recorded no result, so this is the branch as the grid last saw it — it may hold only the task's input". The two were read one after the other by the same person in the same minute, and contradicted each other. Both now say the same thing: nothing is undone, and whether the agent published anything is a separate question the fetch will answer.⚠️ TWO copies of the promise, not one. `--help` carried it too, and the original 08-18 report was against the help rather than the printed line — issue 46 reworded the parser's one-line description and left the epilogue standing. They were written apart and fixed apart once already, so one test now pins both, where a future edit to either is measured against the same rule. The `grid task fetch` pointer stays. It is the only way to find out which of the two you got, and removing it to fix the honesty would take the answer away with the wrong promise.⚠️ The command goes LAST on its own line, and that is a house rule with a test behind it: `test_task_lease.test_every_command_this_cli_tells_you_to_run_actually_ parses` reads a printed hint as `grid (task|project) …` to the end of the LINE and retypes it. The first wording here put prose after the command, and the parser refused what this CLI had just recommended — caught by that test, on the full suite, not by any test written for this change. Positive control in the test: `grid task fetch <id>` must still be in the output, without which every assertion passes against a command that printed nothing. Mutation: restoring the old sentence kills it. Found in the 2026-08-18 round, re-measured 2026-08-20 (ND-02).
… and says so `grid task create` handed the user FastAPI's two words. In the old-relay drill, thirteen of fourteen commands translated a bare framework 404 into a sentence naming the relay and a way forward; this one printed `Not Found`.⚠️ The reason it had no hint was sound, and this does not undo it. `POST /relay/v1/tasks` exists on relays predating the whole project plane, so a 404 here cannot mean "your relay is old" — an old relay answers 201 and quietly files the task in the caller's own `default`, which is the failure the echoed- `project_id` guard above already catches. Every other missing-route hint in this module says "your relay predates this feature" and would, here, send somebody to upgrade a server that is answering correctly. So this is not a rollout-order gap but a misdiagnosis: a 404 from a route every relay has means whatever is answering at that address is not the relay — a proxy in front of it that does not pass the path on, or a base address pointing somewhere else. `_NOT_THE_RELAY` says that, says no task was created, and points at `grid status` for the address in use. The sibling test passes for a reason that never covered this: without `--project`, the command resolves a project first and THAT call carries `_OLD_RELAY`. Name the project and the resolution is skipped, so the task route is the first request. The new test names it, and asserts `seen[-1]` is `/relay/v1/tasks` as the positive control — otherwise a command that refused before ever calling the relay satisfies every other assertion. Both directions are pinned. The new sentence must appear for the bare framework 404, and must NOT appear for a real refusal the relay explained itself — keyed on the detail being exactly `"Not Found"`, the same test `_task_oneshot` applies for the other thirteen. A guard this specific and this alarming is exactly the kind that becomes the next bug by overreaching. Mutation: dropping the hint kills the first test. Found in the 2026-08-20 edge-case round (ND-13, LOW).
…instead of it
The refusals paragraph said `--json` "writes one document to stderr", which reads
as "stderr is that document" — and an application that parses the whole stream
breaks on the second line.
Measured, not reasoned: `grid task get <id> --json` puts the envelope on line 1
and the same message as a plain sentence on line 2. The interpreter prints
`str(exc)` on its way out as it always has, and `cli/json_error.py` says so in
its own module docstring ("the envelope is beside it, not instead of it"). It was
only the user-facing doc that implied otherwise.
Code is correct and unchanged; the doc now shows both lines and says to read the
FIRST line of stderr rather than parsing the stream.
Found in the 2026-08-20 edge-case round (C7.1).
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.
Six independent defects found in the 2026-08-20 edge-case / multi-member round, plus the one before it. One commit each, so the history stays bisectable.
What is here
3f9f612_safe_segmentrefused the object store's name with==. APFS and NTFS are case-insensitive, soSTORE.GITandstore.gitare one directory on the machines a provider runs on — measured.casefoldnow.63e2bb8.DS_Storemade every sweep on every macOS provider announce that the workspace bound "is not being fully enforced", while enforcing it correctly in the same call. Non-directories are filtered where the children are listed.0114a27grid task createsaid "has no main yet" at the first wall a new user meets — and the git-vocabulary scan reported 18 passed, because the literal sits in a local. Walker and wording both fixed.5871eaagrid task cancelpromised "whatever it had already done is kept". It cannot know that when it prints. Two copies — the printed line and--help.e42f703grid task createleaked FastAPI's bareNot Found; the other thirteen commands in the old-relay drill translate it. Its own sentence, because a 404 here means not a relay, never an old relay.e666ecadocs/cli.mdimplied--jsonmakes stderr one document. Measured: envelope on line 1, human sentence on line 2.The one worth reading twice
ND-06 is the third instance of one structural hole in
tests/test_application_surface.py._printed_stringswalks the sinks and collected only theConstantpieces of an f-string, so a sentence reached through aNamewas invisible. The two earlier instances (_DEFAULT_WARNINGS,_MERGE_TURN_LABEL) were module-level constants and were answered with_SENTENCE_TABLES— an explicit list. A list cannot reach a function's local, so the walker had to.The fix is deliberately shallow (one pass, no flow analysis, a local built from another is not chased). The bound is the point: a local that never reaches a sink stays invisible, which is what keeps a wire value like
kind = \"merge\"out of the report. Widening to every module-level string was rejected before for that reason and still is.Order was the evidence: walker test red -> fix walker -> the scan itself turned red on
cli/remote_task.py:543 says 'main'-> fix sentence -> green. Blast radius measured before the fix landed: exactly one finding, zero false positives across all eleven handler modules.Test plan
mainsync-down)ruff check .clean —main's ownb5ae178fixes the six pre-existing errors, picked up by the sync-downNamearm (print(local)/return local), so the test was widened rather than the fix weakened/relay/v1/tasks; ND-18 asserts the cap still evicted 2->1; ND-02 asserts the fetch pointer survives)Notes
grid …hint must be last on its line, becausetest_every_command_this_cli_tells_you_to_run_actually_parsesretypes it to end-of-line. Commented at the site.🤖 Generated with Claude Code