chore: stop shipping two claims nothing exercised - #51
Merged
Conversation
Windows was listed as supported in six places while `runCommand` spawns `sh -c`, which Windows has no equivalent of — so `run_terminal` and `run_tests`, the two tools the agent leans on hardest, cannot run there. The approval classifier compounds it: it knows `rm -rf`, `sudo` and `chmod`, not `del /f /s /q`, so every Windows command falls through to the fail-closed default and asks. CI never caught this because the matrix is ubuntu and macOS. Adding windows-latest would produce a permanently red job, so the matrix keeps its two entries and records why the third is missing. The `win32` branch in config/paths.ts stays. It is correct, it costs nothing, and it is what WSL or a later port would want — it just no longer implies a platform that works.
test-reset.ts targeted ./config/providers.json — inside the repository, where user configuration has never lived. Real config is in ~/.config/woopcode/, so every invocation raised ENOENT and reset nothing. It is listed in CLAUDE.md's command table, which is how it stayed broken: the only check on it was somebody running it by hand. It now imports getConfigDir rather than rebuilding the path, so the two cannot drift again, and moves the whole directory aside instead of blanking apiKey fields. Conversation history and the execution log are what separate "no key yet" from "never run before", and a first run has neither. Also names every supported vendor variable in the headless no-provider error. It said GEMINI_API_KEY only, sending a user holding an OpenAI key to buy a credential they already had. Covered by a subprocess integration test: two of its three cases fail against the previous script.
The first screen opened with "an early-stage project" and a table of repository-aware, terminal-first and visible execution — claims that carry no information because every competing agent makes them, on a project with three provider integrations, a benchmark harness and a four-gate pre-commit system. Replaced with the things that are specific and checkable: writes stop on a diff with no path around the review, unrecognised shell commands are treated as destructive, plan mode is gated in two places and why both are needed, provider reasoning is replayed per provider because both Anthropic and OpenAI fail silently without it, and tools are tested against a real filesystem. The provider-maturity note stays — that Gemini is best-tested is useful to someone choosing — but as a fact rather than an apology. Platform support is now stated up front, so a Windows user learns it here rather than after installing. npm description likewise: it was generic enough to describe any agent.
Points homepage at the deployed documentation site rather than the GitHub README, and links it from the top of the README. Both were checked with curl before being committed; the previous value sent anyone arriving from npm to a file rather than the docs.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The ship command told me to end every commit body with a Co-Authored-By trailer naming a model, and every PR body with a "Generated with Claude Code" footer. Neither says anything about why a change exists, and the contribution record on this repository is the maintainer's. The rule now covers PR bodies, issues and review comments too, so it cannot come back through a path the commit rule does not reach. No existing commit carried the trailer. The footer was removed from the body of #51.
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.
Why
Two things were claimed and never exercised, and both had been that way long
enough that only a stranger would have found them.
Windows was advertised as a supported platform.
docs/getting-started/install.mdlisted it in the platform row, and five other places documented
%LOCALAPPDATA%\woopcode\paths. ButrunCommandintools/command.tsspawnssh -c, which Windows has no equivalent of, sorun_terminalandrun_tests—the two tools the agent leans on hardest — cannot run there at all. The approval
classifier compounds it: it knows
rm -rf,sudoandchmod, notdel /f /s /qorreg delete, so every Windows command falls through to thefail-closed default and asks. CI never caught it because the matrix is ubuntu and
macOS.
onboarding/test-reset.tshad never worked. It targeted./config/providers.json— a path inside the repository, where userconfiguration has never lived — while
getConfigDirputs it in~/.config/woopcode/. Every invocation raised ENOENT and reset nothing. It islisted in CLAUDE.md's command table, which is exactly how it stayed broken: the
only check on it was somebody running it by hand.
Alongside those, the README's first screen opened with "an early-stage project"
and a table of repository-aware, terminal-first, visible execution — claims
that carry no information because every competing agent makes them.
What
win32branch inconfig/paths.tsstays — it is correct, costs nothing, and is what WSL or a later port would want — but now carries a comment saying why it is unreachable.windows-latestis deliberately absent and the matrix records why. Adding it would produce a permanently red job documenting nothing the docs do not now say outright. The platform is a port, not a matrix entry.test-reset.tsrewrittengetConfigDirrather than rebuilding the path, so the two cannot drift again. Moves the whole config directory aside instead of blankingapiKeyfields, and refuses to clobber an existing backup.GEMINI_API_KEYonly, sending a user holding an OpenAI key to buy a credential they already had. Now names every supported vendor variable.homepage,description, versionhomepagepointed at the GitHub README rather than the deployed docs site; both it and the two deep links were checked withcurlbefore committing. Version 0.9.0.The tradeoff worth naming is the CI one. The obvious move was to add
windows-latestand fix the fallout, and that was the plan until the shelldependency turned up. Supporting the platform means replacing the
sh -cspawn,the
/procandpgrepprocess-group handling, the POSIX signal semantics, andteaching the classifier a second command vocabulary — the last of which is
security-critical. That is a port. Removing an untrue claim is the honest change
that fits in this PR; the alternative was a red job standing in for a decision
nobody had made.
The
test-reset.tsreset moves the directory rather than blanking keys becauseensureProviderConfiguredtreats a keyless provider and a missing configidentically, but the rest of a first run does not —
conversation.jsonandexecution-log.jsonare what separate "no key yet" from "never run before".Verified
bun run verify --allon the tree as it stands, after the final commit:Order-dependence sweep, since a test file was added:
The new
packages/tests/config/testReset.integration.test.tswas checked againstthe bug it covers: reverting
onboarding/test-reset.tsto the previous versionturns two of its three cases red. The third passes either way, because the old
script also errored on a missing backup.
Failure paths were exercised by running them rather than by trusting the
documentation, each against a temporary
XDG_CONFIG_HOME:providers.json→ moved aside with a timestamped name, defaultsrecreated, startup continues
approvalMode: "YOLO-TRUST-EVERYTHING"→ resolves toauto-read-only, thedefault, not to anything permissive
https://woop-code.vercel.appand both README deep links returned 200.bun cli.ts --versionprints0.9.0.Not verified: nothing here was run on Windows or WSL — the change removes a
claim rather than making one, so there is no Windows behaviour to test. And
verify.ts:55usesnew URL(".", import.meta.url).pathname, which yields/C:/...on Windows; harmless now that the platform is not claimed, but it isthe same class of bug and would bite a future port.