Repository navigation
fix: honour --dry-run everywhere, lock the config, keep skill installs in the repo, bound --query - #174
Merged
Conversation
A clone brings its links along. Under --project, a dangling SKILL.md link made the install create the file it pointed at, and a linked skill folder or .claude was written into wherever it led. --project now refuses a skill folder or SKILL.md that is a symbolic link, and a folder that resolves outside the repository. Home installs still follow links, as npx skills makes them there.
The search shares what an expression repeats, so [@,@] | [@,@] | ... ran in no time there, while encoding the result wrote every copy out: 22 steps reached 400 MB in a second. The encoding now has a budget of 32 times the response plus 1 MiB; a result past it fails as a --query error, like any other query that fails on the data.
Each command that changes ~/.plivo/config.toml loaded it, changed its copy and saved it whole, so two run at once (config set and auth use in parallel) lost one change every time. config.Update holds an exclusive lock on ~/.plivo/config.lock (flock; LockFileEx on Windows) from the load to the save, and every change goes through it: auth use and remove, login, logout, config set and telemetry, and the skills offer. A login picks its profile name under the lock, from the config it saves. auth remove and logout delete the keychain token once the profile is saved without it.
auth use, auth remove, logout, config set and config telemetry ignored --dry-run and acted for real: auth remove --dry-run deleted the profile and its keychain token. Each now runs its checks, so an unknown profile or value is still refused, prints what it would change on stderr, and changes neither the config nor the keychain.
cobra checks only that a required flag is there, so `messaging sms send --src ' ' --dst '' --text ''` built a request. The root pre-run hook now refuses a flag marked required that was given an empty or all-space value, before any request is built. An MMS with --media-url may still leave --text empty. The commands that checked a blank value themselves (sip create --name, verify sessions validate --otp, voice streams test --to) now give this error instead, with the same exit code.
config.Update waited on ~/.plivo/config.lock for as long as another command held it, so a login stuck on a keychain prompt, or any hung command, hung every later command that changes the config. It now tries the lock every 50 ms and, after 10 seconds, fails with RESOURCE_CONFLICT: another plivo command is updating the config.
skill update rewrites the copies in the repository's .claude/skills and .agents/skills too, and followed any link there, so with --force a cloned link could aim the write at any file the user can write. A copy in the repository that resolves outside it now fails the update with BAD_INPUT before anything is written through it. Links inside the repository, as npx skills makes them, and home copies are unchanged.
…docs These acted for real under --dry-run: login opened the browser and saved a profile, upgrade downloaded and replaced the binary, voice streams test connected to the endpoint, and docs fetched the export and wrote its cache. Each now prints what it would do on stderr and does none of it; upgrade still looks up the release, to name the version it would install. The update check after a command is skipped under --dry-run too, as it writes its cache. ask -i has nothing to preview, and now refuses --dry-run as a bad flag rather than bad input.
Building a feedback event saved a new ~/.plivo/machine-id, so feedback --dry-run, which builds one to print it, wrote a file. A new id is now kept by the send that carries it, and a dry run writes nothing.
The result budget bounds only writing the result out. to_string(@) over [@,@] | [@,@] | ... writes every shared copy out during the search itself, so it could still run for minutes and fill memory. The search now runs under a watchdog: once it has run for 10 seconds, or grown the heap by 256 MiB, the query fails as a --query error. go-jmespath can't be cancelled, so the abandoned search ends with the process.
…tory --project refused any skill folder or SKILL.md that was a link, so the layout npx skills makes, .claude/skills/<skill> linked to its copy in .agents/skills, could no longer be installed into, even with --force. It now refuses only a skills folder, skill folder or SKILL.md that resolves outside the repository, or a link that leads nowhere, and follows links that stay inside it.
skill update stopped with an error at the first copy in the repository reached through a link out of it, after it had already updated the home copies, and a --dry-run failed the same way. Such a copy is now a refused row, never written, and the update goes on with the rest. It is refused only when it would be written: an up-to-date or kept copy is reported as before.
On NFS mounted without its lock service, or another file system that takes no locks, flock fails with ENOLCK, ENOTSUP or ENOSYS, and every command that changes the config failed with it, login included, after the browser step. Such a file system now leaves the config unlocked and the change goes on, as on the platforms without locks.
…oint A collection during the search can free what was on the heap before it, so growth measured from the start could stay under the limit while the search itself grew past it. Growth is now measured from the lowest point seen, and a test covers the memory limit as well as the time limit.
An MMS could leave --text empty whenever --media-url was given, even as --media-url ' ', so a message with neither went out. The exemption now needs at least one --media-url that is not blank.
auth remove --dry-run and logout --dry-run said they would delete the profile's keychain token even when the token was kept in the config, or there was none. They now name it only when the keychain holds one.
--dry-run now previews local changes as well as requests, but its help still read "print the HTTP request without sending". It now reads "preview requests and local changes without sending or writing anything". Help goldens and docs/COMMANDS.md are regenerated.
# Conflicts: # cmd/root.go
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
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.
Local-safety fixes from the latest QA round.
--dry-runpreviews and changes nothing on every command that acts outside the API:auth use/auth remove(which used to remove the profile and its keychain token),logout,config set/config telemetry,login,upgrade,voice streams testanddocs.ask -irefuses it. The update check is skipped under--dry-run, andfeedback --dry-runno longer creates a machine id.skill install --projectandskill updaterefuse a skill path that leads outside the repository. Links inside it, asnpx skillsmakes them, still work; an outside link inskill updateis reported asrefusedand never written.--queryresult or search that grows too big or runs too long stops with a query error instead of eating memory.mms sendmay leave--textempty when--media-urlhas a URL).Testing: each fix has a test that fails without it. The config race lost an update in 30 of 30 parallel rounds before and 0 after. Binary checks ran in a scratch home and repository.
Size
Added lines by type, blank lines excluded. Comments counts comment lines in non-test code.
Removed: 379 lines. Tests include 30 comment lines.
Tests include 224 lines of regenerated help snapshots (the reworded global
--dry-runline in each); hand-written tests are 314 lines.