fix: address code review findings across codebase

[yesdevnull]

Summary

• Remove compiled binary from repo and add /trenchcoat to .gitignore
• Deduplicate matcher logic — extract lazyBodyReader(), resolveSequence(), findCandidates(), and selectBest() shared helpers from Match/MatchVerbose
• Fix WriteTimeout — was double-counting MaxDelayMs (150s → 90s)
• Fix proxy filter — switch from path.Match to doublestar.Match for ** glob support, consistent with coat URI matching
• Upgrade callsMu to sync.RWMutex for concurrent read access in assertions
• Add file watcher debouncing — 100ms time.AfterFunc debounce to handle editors producing multiple rapid events
• Nil-guard Header.Clone() in public API Requests() method
• Clarify substituteVars condition with explicit comments for shell :- semantics
• Fix append dedup counter — second file now uses _1_ instead of _2_

Test plan

• go test -race ./... — all packages pass
• gofmt — no formatting issues
• go vet ./... — clean
• golangci-lint run ./... — 0 issues

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR consolidates a set of review-driven fixes across the Trenchcoat CLI, server, matcher, coat loading/validation, and proxy capture components—primarily improving safety (path handling, timeouts), correctness (matching/proxy filtering), and testability/concurrency behavior.

Changes:

• Harden server/proxy behavior: add HTTP server timeouts, improve body_file resolution safety, and adjust proxy glob matching to doublestar.
• Refactor matcher to deduplicate Match/MatchVerbose logic via shared helpers and add body-size limit tests.
• Improve CLI/config ergonomics (bind Cobra flags into Viper keys), and tighten test/public API edge cases (nil Header.Clone(), updated expectations).

Reviewed changes

Copilot reviewed 6 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
trenchcoat.go	Nil-guards header cloning in the public test API Requests() output.
internal/server/verbose_test.go	Updates test expectations for rejecting absolute body_file paths without leaking them.
internal/server/server.go	Adds RWMutex for calls, sets HTTP server timeouts, makes delay context-aware, hardens body_file resolution.
internal/proxy/proxy.go	Switches filter globbing to doublestar and adds multiple proxy hardening changes (limits/headers/perms/timeouts).
internal/matcher/matcher_test.go	Adds tests for request body size limit behavior in matching.
internal/matcher/matcher.go	Refactors matching into shared helpers and centralizes lazy request body reading.
internal/config/config_test.go	Uses t.Chdir to simplify cwd manipulation in tests.
internal/coat/validate.go	Adds MaxDelayMs and centralizes response validation (delay and body_file constraints).
internal/coat/parse.go	Clarifies substituteVars behavior with comments for shell :- semantics.
internal/coat/load_test.go	Updates tests to reflect filtering invalid coats out of load results.
internal/coat/load.go	Wraps validation errors with %w and filters invalid coats from loadFile.
cmd/trenchcoat/serve.go	Binds flags into Viper keys, validates port/log-format, and adds file-watch debounce logic.
cmd/trenchcoat/proxy.go	Binds flags into Viper keys and validates port / mutual exclusivity of header options.
cmd/trenchcoat/commands_test.go	Updates newLogger tests for the new error-returning signature.
.gitignore	Ignores the compiled /trenchcoat binary.

Comments suppressed due to low confidence (3)

internal/proxy/proxy.go:512

• isHopByHopHeader only filters the standard hop-by-hop header names. Per RFC 7230, any header fields listed in the Connection header are also hop-by-hop and must be stripped when proxying. As written, a request/response with Connection: Foo will still forward Foo. Consider parsing Connection header values and treating the named headers as hop-by-hop in addition to the static list.

// isHopByHopHeader returns true if the header is a hop-by-hop header that
// should not be forwarded by proxies.
func isHopByHopHeader(h string) bool {
	_, ok := hopByHopHeaders[http.CanonicalHeaderKey(h)]
	return ok

internal/server/server.go:267

• In the context-cancel branch, timer.Stop() is called but the timer channel isn’t drained when Stop() returns false (timer already fired). In the edge case where the timer fires concurrently with context cancellation, this can leave a value buffered on timer.C until GC. Consider the standard pattern if !timer.Stop() { <-timer.C } (guarded by a non-blocking select) or using time.After with context helpers to avoid the drain logic.

			timer := time.NewTimer(time.Duration(delay) * time.Millisecond)
			select {
			case <-timer.C:
			case <-r.Context().Done():
				timer.Stop()
				return

internal/proxy/proxy.go:40

• The PR description calls out the proxy filter change, but this file also introduces other user-visible proxy behavior changes (body size limiting, hop-by-hop header filtering, tighter filesystem permissions, timeouts). Please update the PR description (and/or release notes) to reflect these so reviewers/users aren’t surprised.

// maxBodySize is the maximum number of bytes read from request or response bodies.
const maxBodySize = 10 << 20 // 10 MiB

// hopByHopHeaders are headers that must not be forwarded by proxies.
var hopByHopHeaders = map[string]struct{}{
	"Connection":          {},
	"Keep-Alive":          {},
	"Proxy-Authorization": {},

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 8 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

internal/proxy/proxy.go:316

• doublestar.Match follows filepath-style semantics (OS-specific path separators). Since urlPath is always a URL path using /, this can behave incorrectly on Windows builds. Use doublestar’s path-style matcher (e.g., doublestar.PathMatch) or otherwise force /-separator semantics for URL matching so filters like /api/** work consistently across platforms.

	matched, err := doublestar.Match(p.config.Filter, urlPath)
	if err != nil {
		p.logger.Error("invalid capture filter pattern", "filter", p.config.Filter, "error", err)
		return false
	}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

internal/proxy/proxy.go:316

• The proxy capture filter now uses doublestar.Match to support ** globs, but the current proxy filter tests only cover single-segment patterns like /api/*. Add a test case that asserts multi-segment matching (e.g. filter /api/** should match /api/v1/users, and /api/**/users matches deeper paths) so regressions in ** handling are caught.

	if p.config.Filter == "" {
		return true
	}
	matched, err := doublestar.Match(p.config.Filter, urlPath)
	if err != nil {
		p.logger.Error("invalid capture filter pattern", "filter", p.config.Filter, "error", err)
		return false
	}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.