Repository navigation
docs: make the prose match the shipped behaviour, and pin the version copies - #375
Merged
Merged
Conversation
… copies An audit of the docs found prose describing behaviour that does not exist. Two of these would have led a user to write something that silently does nothing. **docs/plugins.md was still a design document.** It said 'Status: design -- no implementation here; #49 implements against this doc', and the feature has shipped. Worse, its example manifest was wrong in a way that fails silently: params: # a YAML mapping city: {type: string, required: true} The parser only handles params when the value is a string, so a mapping leaves the parameter set empty, {city} is never substituted, and the tool still lists in the schema with no required arguments. Nothing warns. Rewritten as shipped, with the working inline form shown and a 'Deliberate differences' table recording each divergence from the original design and why. **The shell fallback does not exist.** The doc said a command outside ALLOWED_BINARIES 'falls back to the shell tool, which keeps its approval gate + hard-refusal patterns'. There is no fallback and no escalation -- tool_exec returns its own 'Blocked: ...' string and the command does not run. Implicit escalation to a more powerful tool is a worse default than refusing, so the code is right and the doc was wrong. **docs/mcp-client.md omitted the key that refuses your tools.** Default trust is 'untrusted', a tool needs a literal readOnlyHint: true, and the refusal tells you to set trust = "full" -- a key that appeared nowhere in the file. inherit_env was absent entirely. Both documented, with the three gates (trust / readOnlyHint / approval) spelled out, including that an unrecognised trust value silently normalises to untrusted. **Two web_search claims were false.** Both README and docs/egress.md said the allowlist covers 'every hit it follows'. tool_web_search requests only html.duckduckgo.com and returns result URLs as text; it never fetches a hit. Following one is read_url, which is governed on its own. **SIDEKICK_TRUST_REPO was undocumented** -- a security gate, since a repo's .sidekick/commands/*.md expands !`cmd` into sh -c with no approval and is silently ignored without it. A user had no way to find out why their repo's commands did nothing. **Version copies had all drifted.** AUR pkgver was 0.26.0 with a hardcoded sdist hash for that version (now 0.29.0, URL and sha256 verified against the real artifact) and documented a manual tag step the release workflow does instead -- while on.push filters on branches, so a tag push runs nothing. ROADMAP 'Next' pointed at v0.28.0, already shipped. The website roadmap was eight releases stale. CONTRIBUTING's release checklist still demanded a test-count badge gate that #319 removed, so a release PR was being judged against gates that no longer exist. 14 new tests assert the mechanical agreements -- version copies agree, documented keys exist, the params form shown is the form the parser accepts, and no removed gate is still referenced. Deliberately not asserting that prose is true: a doc can be confidently wrong in a way no test can see, which is why the high-severity ones were verified against the code first. Sabotages: reverting plugins.md to design-doc framing fails; removing the trust key docs fails. 1382 passed (+14), ruff clean, mypy clean, coverage 79.92%.
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.
Two of these would have led a user to write something that silently does nothing
docs/plugins.mdwas still a design documentIt said "Status: design — no implementation here; #49 implements against this doc." The feature shipped long ago. Worse, its example manifest was wrong in a way that fails silently:
plugins.pyonly handlesparamswhen the value is a string, so a mapping leaves the parameter set empty,{city}is never substituted, and the tool still appears in the schema with no required arguments. Nothing warns.Rewritten as shipped, showing the working inline form (
params: city: string required) and adding a Deliberate differences table recording each divergence from the original design and why.The documented shell fallback does not exist
There is no fallback and no escalation —
tool_execreturns its ownBlocked: …string and the command does not run. The code is right and the doc was wrong: implicit escalation to a more powerful tool is a worse default than refusing.docs/mcp-client.mdomitted the key that refuses your toolsDefault trust is
untrusted, a tool needs a literalreadOnlyHint: true, and the refusal message tells you to settrust = "full"— a key that appeared nowhere in the file.inherit_envwas absent entirely. Both are now documented, with the three gates (trust/readOnlyHint/ approval) spelled out, including that an unrecognised trust value silently normalises tountrusted.Two
web_searchclaims were falseREADME and
docs/egress.mdboth said the allowlist covers "every hit it follows".tool_web_searchrequests onlyhtml.duckduckgo.comand returns result URLs as text — it never fetches a hit.SIDEKICK_TRUST_REPOwas undocumentedA security gate: a repo's
.sidekick/commands/*.mdexpands!`cmd`intosh -cwith no approval prompt, and is silently ignored without it. A user had no way to discover why their repo's commands did nothing.Version copies — all five had drifted
pkgveron.pushfilters on branches, so a tag push runs nothingROADMAP.mdNextROADMAP.mdCONTRIBUTING.mdchecklistTests (+14)
Mechanical agreements only: version copies agree, documented keys exist, the params form shown is the form the parser accepts, the
docs/examples/manifests don't use the broken mapping form, and no removed gate is still referenced.Deliberately not asserting that prose is true — a doc can be confidently wrong in a way no test can see, which is why the high-severity ones were verified against the code first.
plugins.mdto design-doc framingtrustkey documentation1381 passed (+14), ruff clean, mypy clean, coverage 79.92%.