Address Copilot review feedback from PR #16

[ndenny]

Resolves the Copilot review comments on #16 (the v0.0.1 release candidate) so that PR can go into main clean.

Fixed

#	Concern	Fix
skills/skills.go:176	updateClaudeMD ignored os.ReadFile errors — an existing-but-unreadable CLAUDE.md was treated as empty and then overwritten, destroying its contents	Only os.IsNotExist is treated as empty; any other read error is returned
oauth/authcode.go:240	Reflected-XSS hardening had no test coverage	Added three tests for the auth callback handler. The two escaping tests were verified to fail when html.EscapeString is removed
skills/skills.go:78	resolveTarget had no tests	Added table-driven tests: no flag, each target, and every mutually-exclusive combination
cli/request_test.go:75	TestAuthHookFailure depended on global state left by earlier tests	Pins rsh-profile to default. It also needed a non-nil configs map — Copilot spotted one of the two causes; both are fixed, and the test now passes standalone
README.md ×2, skills/skills.go:208, 3× skills/embed/*.md	Docs claimed create commands read JSON from stdin only, which contradicts cli.GetBody (and the README's own CLI Shorthand section immediately below)	Reworded to "stdin or CLI Shorthand arguments". Heredoc stdin is still the recommended form for agents, and the accurate "there is no --body/--data/-d flag" statement is kept
README.md:12	Installation and Manual installation were both ##	Manual installation, MacOS, Windows and Linux demoted to ### under Installation
skills/embed/setup-api-monitor.md:92	Validation gate said "monitor ID" but the API-monitor skill schedules a <call-id>	Changed to "call ID". The browser and MCP skills correctly use <monitor-id> and are unchanged

Not fixed — the premise is incorrect

main.go:13 — Copilot claimed the goreleaser -X main.commit / main.date / main.builtBy ldflags "will cause release builds to fail with 'cannot set -X main.commit' errors".

The Go linker silently ignores -X for symbols that don't exist; it is not an error. Verified two ways:

• go build -ldflags "-X main.commit=abc123 -X main.nonexistent=foo" . exits 0.
• Every release since these ldflags were added has built successfully, most recently v0.1.0-beta-1.

The flags are dead config rather than a build break. Left alone here — rewriting five .goreleaser/*.yaml files to strip no-ops immediately before the first formal release is risk without benefit. Worth a follow-up to either drop them or wire commit/date into --version, which versionExtraInfo already exists to serve.

Post-merge fixup

develop merged the claude-skills feature after this branch's resolveTarget tests were written, adding a 4th claudeSkills parameter. That broke go vet/go test on this branch (wrong argument count) and left the "no target"/"only one of" error-string assertions and the mutually-exclusive-combination coverage stale. Fixed: resolveTarget test cases now include claudeSkills, the expected error strings include --claude-skills, and a --claude-skills-only and --claude-skills+--claude-code case were added.

Verification

go build ./..., go vet ./... and go test ./... all pass. (gofmt -l flags five files — bulk/commands.go, bulk/metadata.go, cli/api.go, cli/autoconfig.go, cli/param.go — all pre-existing and untouched here.)

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR addresses prior automated review feedback from PR #16 by hardening file-update error handling, adding targeted test coverage for OAuth callback XSS escaping and skills target resolution, stabilizing a CLI request test that depended on global state, and correcting documentation to reflect actual input handling.

Changes:

• Prevent CLAUDE.md overwrites when os.ReadFile fails for reasons other than non-existence.
• Add/extend unit tests for resolveTarget and the OAuth auth callback handler (escaping + code passthrough).
• Update docs (README + embedded skill docs) to reflect “stdin or CLI shorthand” request bodies and fix heading/wording issues.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
skills/skills.go	Returns non-IsNotExist read errors from updateClaudeMD; updates generated agent guide wording about input sources.
skills/skills_test.go	Adds table-driven tests covering resolveTarget flag combinations and errors.
skills/embed/setup-mcp-monitor.md	Updates skill doc to reflect stdin or CLI shorthand input (preferring heredoc stdin).
skills/embed/setup-browser-monitor.md	Same stdin/shorthand doc correction for browser monitor skill.
skills/embed/setup-api-monitor.md	Same stdin/shorthand doc correction; fixes validation gate wording (“call ID”).
README.md	Fixes heading levels and clarifies stdin vs CLI shorthand for create/update bodies.
oauth/authcode_test.go	Adds tests validating auth callback HTML escapes attacker-controlled query params and still passes the code through.
cli/request_test.go	Stabilizes TestAuthHookFailure by initializing configs and setting rsh-profile.
cli/cli_test.go	Fixes a spelling typo in a comment.

Suppressed comments (1)

oauth/authcode_test.go:71

• Close the response body returned by httptest's recorder here as well; keeping the close pattern consistent across the file helps prevent future leaks when this helper grows.

	body, err := io.ReadAll(w.Result().Body)
	assert.NoError(t, err)

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟢 Approval recommended

The changes are low-risk test additions and documentation corrections plus one well-contained, verified error-handling fix, with test expectations confirmed against the actual code.

Review effort: Balanced
Findings: 1 · 2

Open (3)

• This test now initializes global state, but it also leaves global mutations behind…
• When asserting the error path, also assert the returned agent kind is the default (agentCustom).…
• In tests, close the httptest response body after reading it; this avoids leaking ReadClosers and…

[Copilot]

Copilot review overview

🟢 Approval recommended

The changes are small, low-risk documentation and test-isolation fixes plus a correctly-propagated read-error guard, all verified consistent with surrounding code and prior feedback.

Review effort: Balanced
Findings: None

Resolved since last review (3)

• This test now initializes global state, but it also leaves global mutations behind…
• When asserting the error path, also assert the returned agent kind is the default (agentCustom).…
• In tests, close the httptest response body after reading it; this avoids leaking ReadClosers and…