Codex caught this on the first commit: clearing the destination with
recursive=false made a file renamed onto a non-empty folder answer
ENOTEMPTY, where real rename answers EISDIR, and a file onto an empty
folder still destroyed it.
rename(2) refuses either mismatch regardless of emptiness, verified
against os.replace: file onto a directory is EISDIR whether or not the
directory has children, and a directory onto a file is ENOTDIR. So the
type check now runs before the emptiness check, and the folder is never
deleted to find out. This is what the copy in the same commit already
did.
The three sibling renames (gdrive, dropbox, msgraph) share the gap and
are untouched here: two of them learn the conflict optimistically and
hold only the destination's metadata, so each needs an extra call to
compare types, which is its own change.
box was the one cloud drive that cleared a destination its siblings
refuse. A rename recursively deleted a non-empty destination folder
where gdrive, dropbox and the two msgraph drives all report "Directory
not empty", and a copy recursively deleted a folder standing where a
file was being written.
rename now deletes with recursive=false and reads Box's 409 as
ENOTEMPTY, which is what box's own rmdir already does, so an empty
folder still gives way and a non-empty one does not. copy refuses both
type mismatches with rename(2)'s errnos, EISDIR and ENOTDIR, matching
gdrive and the msgraph copy_tree; only a file still replaces a file.
Neither was reachable from a shell line, since generic mv and cp both
guard first, but rename is reachable through os.rename, os.replace and
os.renames, which the op dispatcher passes straight to the backend.
Adds ow_mv_T_onto_existing_empty_dir, pinned against GNU mv on
debian:stable-slim. It covers the empty-folder replace that rename now
routes through recursive=false, on every target the sibling cases test.
Three findings from the second review round, each reproduced first.
A python host keeps its one argument. Threading the kill channel into
`AskHandler` broke every existing `async def on_ask(record)` with
`TypeError: takes 1 positional argument but 2 were given`, swallowed
into a failed command with the raw python error as its diagnostic.
Typescript never noticed because javascript drops extra arguments. The
asymmetry is the runtimes', not a divergence worth mirroring: a pending
promise cannot be interrupted, so a javascript host has to be handed the
signal, while cancelling the wait here raises CancelledError inside the
handler at the await it is parked on. Python's own idiom is the stronger
one and asks nothing of embedders, so the handler goes back to
`(record)` and the whole test battery is written against that shape.
A compound line is killable too. `prejudgeLine` judges every command of
a multi-command line before any of it runs, and it puts real questions
to a host, but it took no signal and neither did its `admit` call. So
`rm f; echo done` under a silent approver ignored its own deadline
exactly as the single-command case did before it was bounded. Pinned in
both languages, and both pins hang without the fix.
The approval prompt quotes each word. It is the whole of what the human
authorizes and the words are already-expanded values, so joined raw
`rm -- 'quarterly report'` read as two operands and a newline forged a
line break in the prompt. Rendered through `shellQuote`, which leaves an
ordinary name bare.
The fourth finding does not reproduce as described: a compound line is
held above its own redirections, so `rm f 2>&1; echo done` never reaches
the duplication and answers 126 on stderr — all eight compound shapes
probed report denied. The residue is a line whose command name comes
from an expansion (`C=rm; $C f 2>&1; echo done`), which that pass reads
literally and cannot name. That and the operand-scoped deny are the same
missing fact and want the ruling carried on `ExecuteResult`; the
function documents both rather than reaching for a wording test that
cannot tell mirage's refusal from a log quoting one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both from the review on #895.
- a multi-operand gzip failure keeps the error's code in either order,
so `zcat nope dir` is 1 like `zcat dir nope`: gzip's WARN records a
code only when nothing has failed yet, while an error records outright
- the cross-mount stream classifies a rendered line by its terminal
strerror instead of scanning it, so a directory named `No such file or
directory` no longer reads as ENOENT and sed answers 4
Two independent improvements landed on the same literal. This branch
lowered MIRROR_BASELINE from 184 to 183 when `tests/utils/test_naming.py`
gave `utils/naming.py` its first twin; upstream lowered it to 183 as well,
for a gap the hierarchy-kit migration closed. Both sides wrote `183`, so
git auto-merged the line without a conflict -- but the two improvements are
additive, and the real count after the merge is 182.
Worth noting for the next merge that touches this file: a ratchet whose two
sides move the same number in the same direction merges clean and lands
wrong, so the value has to be re-measured after merging rather than trusted
because the diff was quiet.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four conflicts, all where the hierarchy-kit migration (e4e332abb) and this
branch touched the same slack lines from opposite directions: the kit
narrowed the formatters' parameter from `SlackScope` to `SearchTarget`
while this branch rewrote their bodies to route through `channel_dirname`.
Both sides kept -- upstream's type in every signature, the budgeted joiner
in every body -- and `sanitize_name` dropped from the TypeScript imports
now that nothing there uses it.
The new pins moved onto `SearchTarget`, which no longer carries
`use_native`, `target` or `resource_path`.
Verified after merging that the premise the fix rests on still holds: both
readdirs still name channels with `channel_dirname` / `channelDirname`
(python readdir.py:85, typescript readdir.ts:91), so search and listing
still agree. Re-measured -- grep result and readdir dirname both 253 B for
a 100-character CJK channel, every budgeted builder under NAME_MAX.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both findings from the codex review of #890, each reproduced first.
P1 — a run waiting on an approval ignored its own deadline. A silent
approver and a 700ms timeout hung past 4s; the request got no signal, so
`Decisions.raise` sat in the host's promise, which the executor's
cooperative abort checks cannot reach. The run's signal now goes to the
handler (dsh forwards it into `ctx.approval.request`, so a prompt can
take itself down) and bounds the ledger's own wait: the same run now
returns at 706ms as `timedOut: true, exitCode: null, SIGTERM`.
A yes that lands after the kill is dropped rather than recorded, which
is why the wait is bounded inside `raise` and not around it. Banking a
spent-once ALLOW against a dead run would leave a grant in the ledger
for the next identical line to take with nobody asked; the record stays
pending instead. Nothing is put to a host for a run already over.
The ledger reports this as a new `Abandoned`, and the door raises the
abort every other killed wait raises — policy states the fact in its own
vocabulary rather than importing execution (in python that import is a
hard cycle: `mirage.policy.decisions` -> `mirage.workspace.__init__`).
P2 — `rm secrets 2>&1` left stderr empty and put the refusal on stdout,
so `sandbox.denied` read false and dsh offered no escalation for a
refusal it never saw. Both streams are read now; on stdout the exit code
has to agree (126, what both policy renders carry) so a command printing
the phrase as data is not read as a ruling. The operand-scope gap stays
open and stays documented: `ExecuteResult` carries no ruling, and
carrying one there is what would retire the sniffing entirely.
py+ts, 27 new assertions across the ledger, the door and the seam.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The read slots forwarded the backend's own error, so a directory on a
keyed backend read as ENOENT in 23 commands. Refuse it in the slot
instead, so no builder has to remember.
- with_dir_guard / withDirGuard on read_bytes/read_stream/read_range,
applied where the factory already stamps the namespace fact, so the
slot inherits the namespace-only mount parent #887 taught the stat
chokepoint about
- READ_FAIL_EXIT beside USAGE_EXIT: GNU's code for a failed read is the
command's, not the errno's, and it is gated on filesystem errors so a
bad sed script keeps exiting 1
- sed aborts on a directory and continues past a missing file, per GNU
- a directory no longer routes to a filetype handler chosen by name
Codex review on #885 found that `format_grep_results` still composed
`<channel>__<id>` itself, so readdir emitted a 253-byte dirname while a
pushed-down grep reported a 613-byte one for the same CJK channel: the hit
named a path that could not be opened.
Sweeping for the same shape found five more sites #885 missed. They never
call `make_id_name`, so the caller grep that drove that PR did not see them:
slack format_grep_results, format_file_grep_results, file_blob_name
discord file_blob_name
notion page_dirname, database_dirname, data_source_dirname, and the
child-page rows in readdir
All of them now go through `fit_id_name` / `fitIdName`. Notion's four
compositions collapse into one `format_segment`, mirroring the TypeScript
`formatSegment`. Measured before -> after: slack file 617 -> 254 B, discord
attachment 625 -> 253 B, notion dirnames 338 -> 254 B.
The TypeScript grep formatters carried a second divergence: they sanitized
the channel name where readdir keeps the original spelling, so the two
disagreed on any name with a space, apostrophe or emoji regardless of
length. Both now call `channelDirname`.
Existing integ goldens are unaffected -- the longest pinned `__id` name in
the tree is 53 ASCII bytes, so only over-budget names change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both du callers opened with the same try/except around stat, which
TypeScript has factored as du/walk.ts since the module landed. Layout
parity read it as typescript-only ['walk']; baseline drops to 254.
Every postgres lister answered from the path alone, so tables/ and views/
existed under any first segment and the entity files under any third. An
absent schema therefore read as a real directory: test -d succeeded, find
printed two paths nothing could stat, and the readdir channel in
resolve_path_stat synthesized a DIRECTORY row for it.
The kit already had the mechanism. stat wired guards and readdir did not,
so the two disagreed about the same path. Move the two guards to readdir,
where mongodb keeps its own, and pass them to make_readdir.
The guard checked members, but the contract a mount enforces is nominal:
check_resource refuses anything failing isinstance(resource,
BaseResource), so a structurally complete class passed here and was
rejected two doors later, which is the opposite of what an early guard
is for. It now makes the same subclass check, at the door the author
called. BaseResource supplies every member, so a subclass cannot be
missing one and the only thing left to check is that the name it is
keyed by is not empty.
TypeScript keeps checking members, and that is not drift: Resource is an
interface there, erased at runtime, so structural is the whole contract
and nothing downstream can ask for more.
The boundaries arrived as their own keyword, and sixteen of the
generic's nineteen call sites did not pass it: every bespoke backend
wrapper (box, discord, dropbox, email, github, gmail, history, slack)
left it at None, which turns the mount-parent wrappers off, so a
namespace-only ancestor still read as missing there while the fan-out
printed hits from the mount below it. The two shared builders and the
search factory were the only callers that passed it.
Both generics now take the whole CommandOpts and read opts.flags and
opts.ns.mounts themselves, which is the shape TypeScript has always
had and the reason it never grew this gap. The two wrappers that
override a flag before delegating pass dataclasses.replace(opts, ...),
the idiom the repo already prescribes for that.
The guard checked open and close, so a class missing getState reached
installMounts and then crashed Workspace.save(), and one with an empty
kind registered no command or op and failed nowhere. It now checks every
member the interface declares non-optionally and reports which ones are
missing, since naming a frame the author never wrote is what the guard
exists to prevent. Python had the same hole and no guard at all:
BaseResource is a plain class supplying defaults rather than an ABC, so
only a class that subclasses it is safe, and the colon rung is the one
that takes a class subclassing nothing.
The mount-parent wrappers and tree's fallback caught the walk's whole
error set, which carries a permission and a not-supported error on
purpose. A directory the backend refused is there and holds data the run
cannot read, so answering it as empty let grep -r print the descendant
mount's hits and exit 0 while silently omitting the parent, and tree drew
the children without even the unopened row. All four now key on the miss
set, which exists for exactly this split.
The read chokepoint refined an ENOENT into EISDIR when a dispatched stat
called the path a directory, and a dispatched stat answers from a
backend's own listing as well as from the namespace. postgres reads any
first segment as a schema and lists tables and views under it without
checking the schema exists, so cat /pg/nope.txt refused a directory that
is not there. It now reads the child names the namespace owes the path,
which derives a segment only from a mount prefix or a link it holds, is
the authority namespace_listing already gates on, and is session
filtered, so a hidden mount's parent still reads as absence.
The object-store stat override called the generic with hand-written
keywords, so it named whatever argument list the generic had when it was
written. stat_path and mounts were added to the generic and to the one
builder that calls it, and this wrapper kept naming the older set: on s3
and gridfs a mount root's row reported the backend's name for itself
rather than the operand, and a namespace-only mount parent still read as
absent. It now delegates to stat_generic, which reads every fact off
CommandOpts, so the next one added reaches a keyed store for free.
TypeScript already delegated this way.
Three conflicts, all of them a rename this branch and main made
independently. main split FileType into FileType (the POSIX node kind)
and ContentType (a regular file's rendering hint), so a fixture naming
FileType.TEXT becomes FileType.FILE plus ContentType.TEXT; the new
test_wrap.py was not conflicted because it is new here, and named the
member anyway. In config.py this branch generalized _absolutize_cli_ref
into _absolutize_code_ref(entry, key, base) and added the mounts loop
for a resource colon reference, while main added the profiles loop for
a permissions script; both halves are kept.
- hydration copies the stored script onto the default session like the other durable restrictions, both hosts
- excuse the scriptjs cases' ram-commands drop (quickjs is a service only ram-scripted carries)
- format the crossmount route test yapf missed while untracked
A read command already decides what a directory is at one chokepoint,
and it knew two kinds: the explicit stat row, and the implicit prefix a
keyed backend answers a readdir for. A directory that exists only
because mounts sit under it is the third and belongs to no backend at
all, so cat, wc, head, tail, cut, nl, rev, tac, strings, zcat, fmt,
fold, expand, unexpand, md5 and the four sha sums all called the path
missing while stat, file, ls, du, find, tree and test -d called it a
directory.
Adding the third probe to the chokepoint fixes the whole family at once,
and it is reached only where the code already re-raised, so nothing else
can move. The probe asks the dispatcher rather than the mount table,
because a read command wants the operand's own row and nothing here
walks; that also means a hide outranks the synthesis for free, since the
dispatcher filters what the session may be told about.
The two helpers now take the whole opts rather than its index, because
this is where the decision lives and the facts that answer it arrive on
the bag. Threading them one at a time would mean editing all two dozen
builders again for the next one. The typescript stat and stream halves
were two copies of the ENOENT logic and are now one, as python's already
were.
A mount prefix and a link path each owe every directory above them a
name, and the visibility test was applied to the segment rather than to
the thing that synthesized it. Those differ exactly when the hide sits
deeper than the segment, so a session that hid /ghost/deep still learned
/ghost from a root listing while every verb applied to /ghost answered
ENOENT.
Both enumerations now share one predicate, because both had the same bug
for the same reason: child_mount_names and the link half were two copies
of the same loop. It tests the full path, which is strictly stronger
than testing the segment, since a hide is subtree closed on all three of
its planes. A segment can therefore only be withheld by this, never
granted, and a parent that keeps one visible mount keeps its name.
The mount parent wrappers keyed on every descendant, where MountView
splits the two questions apart: avoiding a boundary needs all of them,
naming one needs only the mounts the session may be told about. Answering
at all is naming, so a session that hid the only mount under a namespace
only parent got an ordinary no match from grep and rg while stat, ls, du
and find all reported the path absent, which told it the directory was
there.
Both wrappers now read the visible descendants, so a parent whose mounts
are all hidden goes back to reading as absence, and a parent with one
visible mount left still lists as the empty directory it is.
read_range is the one CommandIO field whose shape differs from the name
it publishes under. A builtin's range_read takes (accessor, path, start,
end) with the end exclusive; the table slot takes (accessor, path, index,
offset, size), and __getattr__ binds only the accessor. Forwarded as it
stood, the table read start as its index and end as its offset, so an
object store answered from end to EOF and any table that touched the
index at all crashed on an int.
The index is read per call rather than captured, because set_index can
replace the store after construction. A meta test states the rule by
arity instead of naming the one case, so the next reshaped field fails
loudly rather than silently reading the wrong window.
- a profile script is a policy hook judging every command (deny/ask/silence), runtime required, failures fail closed
- the commands document is optional beside a script; a script-only profile installs everything and the script alone judges
- the allow list now governs shell builtins too; only agent functions and executed paths are exempt
- rename workspace/route to workspace/lookup
resource: ./wiki.py:WikiResource now resolves the way a clis entry's cli
value already did, in both languages, with a relative path rebased on the
config file's directory. A registry name still wins, so a name cannot be
reread as code. TypeScript honors a static create ahead of the
constructor, which is how a backend needing IO at setup is spelled there.
A path that exists only because mounts sit under it had no backend to
answer for it, so stat and file reported it absent, tree called it
unopenable, and grep -r named it missing while printing hits from the
mount below it. stat and file ask the dispatcher on a miss, tree reads
the mount table before the backend rather than merging after it, and
grep and rg wrap their walk's stat and readdir so the primary backend
answers empty instead of absent.
Also renames a mount root's stat row from the operand: a backend names
its own root, so ram answered / and disk answered the host directory's
basename.
BaseResource.__getattr__ read _ops off the class, which a builtin sets
and a GenericResource has no class of its own to set, so every op a kit
backend's table carried reported as a missing attribute. Read through
the instance and build the map from the table, using the names the
builtins already publish.
Keep only the two tests that guard real logic: the validator rejecting
content on a non-FILE kind, and type being required. Drop the
change-detector tests that restated the enum definition and the ones
that only exercised pydantic field storage.
The follow-up #873 deferred. `make_id_name` capped the label at 100
characters through `sanitize_name`, or at nothing at all through
`path_safe_name`, so a CJK display name rendered 313 and 621 bytes against
a 255-byte NAME_MAX and ext4/APFS refused the name outright.
Measuring it found eleven more builders with the same defect that do not go
through `make_id_name` at all: `make_id_name`'s docstring claims Linear and
Trello as callers, but they only use `parse_id_name` to read names back and
compose them with their own f-strings -- twelve builders across
`core/{linear,trello}/pathing.py`. All 14 measured forms overflowed. Fixing
only `make_id_name` would have fixed 7 of 19 sites.
`fit_id_name` / `fitIdName` is the one budgeted joiner every site routes
through, applying the rule gcal's event filenames already set: the id and
the suffix are spent first and never trimmed, and the label takes what is
left, because a shortened id stops addressing the resource while a
shortened label still round-trips through `parse_id_name`.
Two details the sites needed:
- `make_id_name` takes a `suffix`. Discord and Slack appended `.json` after
the call, spending eleven bytes the budget never saw.
- `fit_id_name` takes an already-transformed label, which is what Linear's
`team_dirname` needs: its label is two sanitized parts joined by the
separator, and re-sanitizing would collapse `__` to `_` and rename every
team directory.
The trim only runs when the budget is exceeded, so a `path_safe` name that
legitimately ends in `_` keeps it. gcal's calendar directory is fixed for
free. All 19 builders now land at 253-255 bytes and python and TypeScript
agree byte-for-byte on every one.
`naming` had no test on either side despite holding real logic; both now
exist with 14 mirrored cases, plus per-builder pins over all twelve
Linear/Trello builders.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Zecheng Zhang