Conversation
Two problems in chmod, both found while answering "what does chmod do
with the owner/group/world flags".
`chmod -rw` revoked only the first share and then failed. Deleting a
permission answers 204 No Content, but the DELETE went through call(),
which ends in .json() and threw "invalid json response body: Unexpected
end of JSON input" on the empty body. The first share was already gone
server-side, so the item was left partially unshared with a confusing
parse error and a nonzero exit. Use the existing del() helper, written
for exactly this ("DELETE returns 204 No Content on success, so don't go
through call()").
That also fixes the `ok` tracking it would otherwise have broken: del()
resolves to undefined, so assigning its result would report "Nothing was
changed" after a successful revoke. Set the flag explicitly instead, and
initialize it to false now that it is a plain boolean.
Second, chmod1 computed 'OK' / 'Nothing was changed' and chmod() threw
it away, so the command printed nothing whichever happened. Log it per
file, which also disambiguates a no-op (e.g. -w against shares that are
already read-only) from real work.
Verified against a mock that answers 204 for DELETE the way the API
does: -rw now revokes every share; -w patches every write share to read;
read-only-under--w, an unshared item, and multiple files on one command
line all report correctly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
defangdevs
approved these changes
Aug 5, 2026
lionello
commented
Aug 5, 2026
| // -rw write DELETE | ||
| // -rw read DELETE | ||
| let ok // FIXME track result from all shares | ||
| let ok = false // FIXME track result from all shares |
Addresses the "FIXME track result from all shares" that the previous commit left in place. That commit only kept `ok` correct after the switch to del() -- it went from "result of the last mutation" to "did any mutation happen", which is still the aggregate the FIXME complains about. Count the shares instead, and say what happened: :/a.txt: 2 shares made read-only :/a.txt: 1 share made read-only, 2 shares already read-only :/a.txt: 2 shares revoked :/a.txt: Nothing was changed The mixed case is the one that was previously unreportable: `-w` against an item shared both ways patched the write shares and silently ignored the read-only ones, and a single boolean could not distinguish that from having changed everything. A share that errors still aborts the remainder, so the counts describe the shares reached before that point; noted in a comment where the vague FIXME used to be. Deciding whether chmod should instead push past a failing share and report per-share errors is a behavioral change, left alone here. Verified against the 204-answering mock across write-only, read-only and mixed permission sets for both modes, plus the unshared item: counts, singular/plural, and the emitted requests all match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Two bugs in
chmod, found while answering a question about what the owner/group/world flags actually do. Independent of #55 — this branch is offmaster.chmod -rwrevoked only the first share, then failedDeleting a permission answers
204 No Content, but the DELETE went throughcall(), which ends in.json():The first share was already gone server-side by then, so an item with several shares was left partially unshared, with a confusing parse error and a nonzero exit. Anyone who ran
chmod -rwon a file with two or more shares and saw that error was likely left believing nothing happened.Fixed by using the existing
del()helper, which was written for exactly this — its comment already says "DELETE returns 204 No Content on success, so don't go throughcall()". It was the only remainingcall(…, 'DELETE')in the file.That also avoids a trap the fix would otherwise have introduced:
del()resolves toundefined, sook = await del(…)would report "Nothing was changed" after a successful revoke. The flag is now set explicitly and initialized tofalse.chmodprinted nothing at allchmod1computed'OK'/'Nothing was changed'andchmod()discarded the return value, so the command was silent whether it changed something or not. Now logged per file, which also distinguishes a genuine no-op (e.g.-wagainst shares that are already read-only) from real work:Testing
No live API — the token here is expired — so I mocked the classic API with a
/permissionscollection that answers a real empty204for DELETE, and reproduced the original failure first (first share deleted, then the JSON parse error, second share untouched). After the fix, verified:-rwOK-wread,OK-wNothing was changed-rwOKNothing was changedNotes for the reviewer
npm run lintcannot run onmasterat all right now (ESLint 10 vs.eslintrc.js) — that is what fix: list and traverse folders shared with you (closes #7) #55 fixes. I borrowed that PR'seslint.config.jsto lint this branch: no new errors, only the four pre-existing unused catch bindings that fix: list and traverse folders shared with you (closes #7) #55 cleans up. Prettier is clean.g-w,o-w,go-wand bare-wall collapse to the same action, and600/700are the same as each other, so there is no way to drop a public link while keeping a named-user share. Inlsoutput those triads do carry meaning (triad 2 = people it is explicitly shared with, triad 3 = an anonymous link exists), which makes the flags misleading rather than merely redundant. Happy to file that separately.🤖 Generated with Claude Code