Skip to content

Commit 91d9fa6

Browse files
sanketsudakeclaude
andauthored
docs: add CLAUDE.md guidance + apply test-writing guidelines to the suite (#4)
* docs: add CLAUDE.md + AGENTS.md and .claude/resources guidance Add repo guidance for Claude Code / AI agents: - CLAUDE.md: slim index calling out the two load-bearing invariants (the result envelope is public API; the CLI never connects to Chrome directly — capabilities go through the chrome.Browser seam). - AGENTS.md: symlink to CLAUDE.md. - .claude/resources/architecture.md: envelope/exit-code contract, the internal/ package map, DI seams, and the daemon/connection model. - .claude/resources/development.md: build/test/lint commands, what CI runs, live-Chrome test behavior, and the release flow. - .claude/resources/test-writing-guidelines.md: adapted from the Fission conventions, keeping only what applies and noting where this repo deviates (stdlib not testify; testing.Short() not build tags). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: parallelize independent tests and name table subtests Apply the test-writing guidelines to the existing suite: - Add t.Parallel() to every independent unit test and subtest across the result, target, config, browser, daemon/lifecycle, and cli packages. - Convert loop-based table tests (cookie/attr/emulate/extract/grid verbs and result.TestExitCodeFor) to named t.Run subtests. Left sequential on purpose: the live-Chrome tests (shared browser), profile_test.go (t.Setenv), and daemon_test.go (real socket listeners). Verified: gofmt clean, go vet clean, and the full `go test -race ./...` (live Chrome included) passes — proving the added parallelism is safe. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 402ee9b commit 91d9fa6

22 files changed

Lines changed: 304 additions & 68 deletions

.claude/resources/architecture.md

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# Architecture
2+
3+
`chrome-cdp` attaches to the user's **already-running** Chrome over the DevTools Protocol (CDP) and drives it from the command line.
4+
It never launches a headless browser for real work — the whole point is to reuse the live session's cookies, logins, and extensions.
5+
6+
## The one-envelope, one-exit-code contract
7+
8+
Every command emits a single `result.Envelope` (`internal/result/result.go`) and exits with a code derived from it.
9+
This contract is the load-bearing interface: both humans and the Claude skill parse against it, so treat it as public API.
10+
11+
- `error.code` strings (fine-grained, e.g. `target_not_found`) map to process exit codes via `result.codeToExit` / `ExitCodeFor`.
12+
- Exit codes are the coarse, stable contract: `0` ok, `2` usage, `3` connection, `4` target/timeout, `5` cdp, `6` daemon.
13+
- Adding a new failure mode means adding a `Code*` constant **and** its `codeToExit` entry — an unmapped code silently degrades to `ExitGeneric`.
14+
- Usage/validation errors (exit `2`) are checked **before** touching Chrome, so a bad flag never launches or connects.
15+
16+
## Package map (`internal/`)
17+
18+
Data flows outermost → innermost: `cli` parses → resolves a `target` → gets a `chrome.Browser` (via `daemon` or direct) → emits a `result`.
19+
20+
- `result` — the envelope, `Err`, and the exit-code table. No dependencies; the root of the contract.
21+
- `target` — the target grammar (`idprefix | url:<s> | title:<s> | @N`) and `Resolve` against a tab list.
22+
- `config` — layered defaults: built-in < config file (`~/.config/chrome-cdp/config.toml`) < `CHROME_CDP_*` env < flag. `Builtin()`, `Resolve()`, `FromEnv()`.
23+
- `browser` — endpoint discovery: finds Chrome's `DevToolsActivePort` file and computes the per-endpoint key (see connection model below).
24+
- `chrome` — the `Browser` interface and its chromedp-backed implementation: snapshot, click/type/fill/select, grid, wait, raw CDP. The real driver logic lives here.
25+
- `chrometest``StubBrowser`, a permissive `chrome.Browser` double embedded by the `cli` and `daemon` tests.
26+
- `state` — the sticky current-target store, keyed per endpoint so distinct `--port`s don't share a "current tab".
27+
- `daemon` — the held-connection RPC: a background process holds the CDP attach so Chrome's consent prompt appears once per session, not per command.
28+
- `cli` — the cobra command tree (`app.go` = wiring + envelope emission, `commands.go` = the verbs). Knows nothing about how the browser connects; `main` injects that.
29+
30+
## Dependency injection at the seams (`cmd/chrome-cdp/main.go`)
31+
32+
`cli.App` is deliberately ignorant of daemons, sockets, and process spawning — it holds function seams that `main` wires up:
33+
34+
- `WithConnector` — how to get a `chrome.Browser` (daemon client vs. `--no-daemon` direct connect), invoked lazily only when a command needs Chrome.
35+
- `WithStickyTarget` — get/set the current target; keyed by `ConnOpts` so each endpoint has its own.
36+
- `WithDaemonCtl` — start/stop/status for the per-endpoint daemon.
37+
- `WithDefaults` — inject config+env defaults (tests keep `config.Builtin()`).
38+
39+
This is why tests can inject a `chrometest.StubBrowser` directly and never spawn a process.
40+
When adding a command that needs a new capability, add the method to the `chrome.Browser` interface, give it a default in `chrometest.StubBrowser` (one place), then implement it in `internal/chrome`.
41+
42+
## Connection & daemon model
43+
44+
- The **endpoint key** derives from the port file + effective `--port`; it names both the daemon socket and the sticky-state file.
45+
It cannot be computed once at startup — `--port` isn't known until cobra parses flags — so `main` computes it per command from `ConnOpts`.
46+
- The **daemon** (`chrome-cdp __daemon <socket>`, a hidden mode) holds one CDP connection for ~30 min and serves commands over a Unix socket, so the "Allow debugging?" consent fires once.
47+
`--no-daemon` bypasses it and connects directly (used by tests and one-shot scripts).
48+
- Chrome M136+ dropped the classic `--remote-debugging-port` for the default profile; `browser` reads `DevToolsActivePort` and connects directly, which is why it keeps working where older tools broke.
49+
50+
## Human vs. JSON rendering
51+
52+
`--json` emits the raw envelope; otherwise `renderHuman` prints a terse line (`✓ …` / `✗ …` to stderr), honoring `NO_COLOR` and `--quiet`.
53+
The human path is a courtesy; the JSON envelope is the contract. Never let a human-formatting change alter the envelope shape.

.claude/resources/development.md

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# Development
2+
3+
## Build, test, lint
4+
5+
```sh
6+
go build -o chrome-cdp ./cmd/chrome-cdp # build the binary (the checked-in ./chrome-cdp is gitignored)
7+
go test ./... # full suite — spawns a headless Chrome for the live-driver tests
8+
go test -short ./... # skip every live-Chrome test (no Chrome needed; what to run without a browser)
9+
go test -race ./... # what CI runs
10+
go test -run TestExitCodeFor ./internal/result # a single test
11+
go test ./internal/chrome/ -run TestFill # a single package + test
12+
gofmt -l . # CI fails if this lists any file
13+
go vet ./...
14+
```
15+
16+
There is no Makefile and no golangci config — CI is exactly the four steps in `.github/workflows/ci.yml`: `gofmt -l`, `go vet`, `go test -race ./...`, `go build ./...`, on ubuntu + macOS.
17+
Before pushing, run `gofmt -l .` (must be empty), `go vet ./...`, and `go test -race ./...` locally to match it.
18+
19+
## The live-Chrome tests
20+
21+
Tests in `internal/chrome/*_test.go` drive a **real** Chrome the test spawns.
22+
They guard themselves with `if testing.Short() { t.Skip(...) }` and also skip gracefully when no Chrome binary is present (so `macos-latest` in CI, which has no Chrome, is green without running them).
23+
CI runs the live path on `ubuntu-latest` (it ships `google-chrome-stable`).
24+
When iterating on non-driver code, prefer `go test -short ./...` — it's fast and needs no browser.
25+
26+
## Release
27+
28+
Tag-driven via GoReleaser (`.goreleaser.yaml`): pushing a `vX.Y.Z` tag runs `goreleaser release` (see `.github/workflows/release.yml`).
29+
`Version` in `internal/cli/commands.go` is set at build time via `-ldflags`; leave it `"dev"` in source.
30+
Validate config changes with `goreleaser check`.
31+
32+
## Conventions
33+
34+
- Follow the repo's markdown style for docs: **one sentence per line** (keeps `git diff` surgical; renders identically).
35+
- The user opens PRs — push the branch, don't open the PR, unless asked.
36+
- Never commit the built `./chrome-cdp` binary, `*.png`/`*.pdf` screenshots, or `.scratch/` (all gitignored).
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
# Test-writing guidelines
2+
3+
Adapted from the [Fission test-writing conventions](https://github.com/fission/fission/blob/main/.claude/resources/test-writing-guidelines.md), keeping only what applies to this Go CLI and reconciling with the conventions already in the tree.
4+
When a rule below disagrees with an existing test, follow the existing pattern in that package — consistency wins — and raise the discrepancy rather than silently rewriting.
5+
6+
## Structure (adopted)
7+
8+
- **Table-driven** with a `map[...]` or `[]struct` of cases; use `t.Run(name, …)` with descriptive case names for anything with more than a couple of cases (see `internal/result/result_test.go`).
9+
- **`t.Parallel()`** on independent tests and non-state-sharing subtests. Do **not** parallelize the live-Chrome tests — they share a spawned browser.
10+
- **`t.TempDir()`** for any filesystem fixture; never write into the repo or a hardcoded path.
11+
- **`t.Cleanup()`** for teardown instead of bare `defer` when the resource is set up mid-test.
12+
- Name tests `TestXxx` and test through the **exported API** of the package (the envelope, the `chrome.Browser` interface, `target.Resolve`) — not unexported internals, unless the internal is the unit.
13+
14+
## The `chrome.Browser` seam (project-specific)
15+
16+
- Unit-test CLI and daemon behavior against **`chrometest.StubBrowser`**, not a real browser. Embed it and override only the methods the test asserts on.
17+
- When you add a method to the `chrome.Browser` interface, give it a permissive default in `chrometest.StubBrowser` — that's the single place a new method gets its test default.
18+
- Assert on the **envelope and exit code** (the contract), not on incidental human-rendered text. A test that pins `error.code` → exit code is more valuable than one that pins a `✗ …` string.
19+
20+
## Live-Chrome tests (project-specific)
21+
22+
- Guard every test that drives a real Chrome with `if testing.Short() { t.Skip(...) }`, and skip gracefully when no Chrome binary is found. This project uses `testing.Short()` for this, **not** `//go:build integration` tags.
23+
- These tests legitimately use `context.Background()` (the driver context is tied to the browser lifecycle, not the test's). Elsewhere, prefer a scoped context.
24+
- Keep them hermetic: spawn the browser the test needs, drive a `data:`/local fixture page where possible, and tear it down in `t.Cleanup`.
25+
26+
## HTTP handlers (adopted, where relevant)
27+
28+
Use `net/http/httptest` (`NewRecorder` / `NewServer`) for any HTTP-facing code rather than a real network listener.
29+
30+
## Property-based tests & fuzzing (adopted, where valuable)
31+
32+
- The parser-shaped surfaces — the `target` grammar and the `--by` selector syntax — are good fuzz/property targets: round-trip stability, and "only exactly-valid input parses". Prefer `pgregory.net/rapid` over the frozen `testing/quick` if you add property tests; direct `go test -fuzz` at the parser boundary and check the corpus into `testdata/`.
33+
- Good properties here: `Resolve` is deterministic for a fixed tab list; a selector that round-trips through parse→format is unchanged; unknown `error.code` always maps to `ExitGeneric`.
34+
35+
## Assertions (project deviation — note before changing)
36+
37+
The existing suite uses the **standard library** (`t.Errorf` / `t.Fatalf`), not `testify`.
38+
Fission mandates `testify require`/`assert`; this repo does not depend on it.
39+
Match the standard-library style in existing files; do not introduce `testify` as a new dependency for a single test without agreement.
40+
41+
## Not applicable
42+
43+
These Fission rules target infrastructure this project doesn't have — ignore them: Kubernetes fake clientsets / `envtest`, `go-snaps` snapshots, `porcupine`/TLA+ linearizability, `framework.Connect(t)` integration harness, SPDX-header `make license`, and the Fission-specific builders.

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
CLAUDE.md

CLAUDE.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# CLAUDE.md
2+
3+
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
4+
5+
`chrome-cdp` is a Go CLI that drives the user's **already-running** local Chrome over the DevTools Protocol (via chromedp).
6+
It reuses the live session's logins and cookies, and speaks one JSON envelope + one stable exit-code contract to both humans and AI agents.
7+
8+
## Detailed guidance
9+
10+
Read the resource file that matches your task before making changes:
11+
12+
- **[Architecture](.claude/resources/architecture.md)** — the envelope/exit-code contract, the `internal/` package map, the `chrome.Browser` seam, and the connection/daemon model. Read this before touching anything cross-cutting.
13+
- **[Development](.claude/resources/development.md)** — build, test (`go test -short` to skip live Chrome), lint, what CI runs, and release.
14+
- **[Test-writing guidelines](.claude/resources/test-writing-guidelines.md)** — how tests are structured here (stub-driven unit tests, `testing.Short()`-guarded live-Chrome tests) and which conventions are adopted vs. deviated from.
15+
16+
## The two things most likely to trip you up
17+
18+
- **The result envelope is public API.** Every command emits one `result.Envelope`; both humans and the Claude skill parse it. A new failure mode needs a `Code*` constant *and* a `codeToExit` entry, or it silently degrades to `ExitGeneric`. Never let human-formatting changes alter the JSON shape. Validate usage/args *before* connecting to Chrome.
19+
- **The CLI never connects to Chrome directly.** `cli.App` holds function seams (`WithConnector`, `WithStickyTarget`, `WithDaemonCtl`) that `cmd/chrome-cdp/main.go` wires to the daemon. To add a browser capability: extend the `chrome.Browser` interface → add a default in `chrometest.StubBrowser` (one place) → implement in `internal/chrome`.
20+
21+
## Docs & style
22+
23+
User-facing docs live in `docs/` and `README.md`; the Agent Skill lives in `skills/drive-chrome-cdp/`.
24+
When editing any markdown, follow the repo style: **one sentence per line**.
25+
Push branches and let the user open PRs unless asked otherwise.

internal/browser/browser_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
)
88

99
func TestParseDevToolsActivePort(t *testing.T) {
10+
t.Parallel()
1011
port, wsPath, err := ParseDevToolsActivePort([]byte("9222\n/devtools/browser/2d642c44\n"))
1112
if err != nil {
1213
t.Fatalf("unexpected error: %v", err)
@@ -20,6 +21,7 @@ func TestParseDevToolsActivePort(t *testing.T) {
2021
}
2122

2223
func TestParseDevToolsActivePortErrors(t *testing.T) {
24+
t.Parallel()
2325
for _, in := range []string{"", "9222", "notaport\n/devtools/browser/x"} {
2426
if _, _, err := ParseDevToolsActivePort([]byte(in)); err == nil {
2527
t.Errorf("ParseDevToolsActivePort(%q) = nil error, want error", in)
@@ -28,6 +30,7 @@ func TestParseDevToolsActivePortErrors(t *testing.T) {
2830
}
2931

3032
func TestWSURLFromPortFile(t *testing.T) {
33+
t.Parallel()
3134
dir := t.TempDir()
3235
f := filepath.Join(dir, "DevToolsActivePort")
3336
if err := os.WriteFile(f, []byte("9222\n/devtools/browser/abc\n"), 0o600); err != nil {
@@ -43,12 +46,14 @@ func TestWSURLFromPortFile(t *testing.T) {
4346
}
4447

4548
func TestWSURLFromPortFileMissing(t *testing.T) {
49+
t.Parallel()
4650
if _, err := WSURLFromPortFile(filepath.Join(t.TempDir(), "nope")); err == nil {
4751
t.Error("expected error for missing port file")
4852
}
4953
}
5054

5155
func TestDecideConnection(t *testing.T) {
56+
t.Parallel()
5257
cases := []struct {
5358
name string
5459
p Probe
@@ -67,6 +72,7 @@ func TestDecideConnection(t *testing.T) {
6772
}
6873
for _, c := range cases {
6974
t.Run(c.name, func(t *testing.T) {
75+
t.Parallel()
7076
if got := DecideConnection(c.p); got != c.want {
7177
t.Errorf("DecideConnection(%+v) = %v, want %v", c.p, got, c.want)
7278
}
@@ -75,6 +81,7 @@ func TestDecideConnection(t *testing.T) {
7581
}
7682

7783
func TestEndpointKey(t *testing.T) {
84+
t.Parallel()
7885
// An explicit port yields a distinct, port-specific key (no collisions).
7986
if k := EndpointKey("", 9333); k != "127.0.0.1:9333" {
8087
t.Errorf("explicit port key = %q, want 127.0.0.1:9333", k)

internal/cli/app_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ func run(t *testing.T, b chrome.Browser, args ...string) (env map[string]any, st
4040
}
4141

4242
func TestListJSONEnvelope(t *testing.T) {
43+
t.Parallel()
4344
b := &fakeBrowser{tabs: []target.Info{
4445
{ID: "aa11", Title: "GitHub", URL: "https://github.com/"},
4546
{ID: "bb22", Title: "Inbox", URL: "https://mail.google.com/"},
@@ -58,6 +59,7 @@ func TestListJSONEnvelope(t *testing.T) {
5859
}
5960

6061
func TestEvalWithoutTargetIsTargetError(t *testing.T) {
62+
t.Parallel()
6163
b := &fakeBrowser{tabs: []target.Info{{ID: "aa11", Title: "X", URL: "u"}, {ID: "bb22", Title: "Y", URL: "v"}}}
6264
env, _, code := run(t, b, "eval", "1+1", "--json")
6365
if code != 4 {
@@ -72,6 +74,7 @@ func TestEvalWithoutTargetIsTargetError(t *testing.T) {
7274
}
7375

7476
func TestRawBadParamsIsUsageError(t *testing.T) {
77+
t.Parallel()
7578
b := &fakeBrowser{tabs: []target.Info{{ID: "aa11", Title: "X", URL: "u"}}}
7679
env, _, code := run(t, b, "raw", "Foo.bar", "not-json", "--target", "aa11", "--json")
7780
if code != 2 {
@@ -83,6 +86,7 @@ func TestRawBadParamsIsUsageError(t *testing.T) {
8386
}
8487

8588
func TestEvalResolvedTargetSucceeds(t *testing.T) {
89+
t.Parallel()
8690
b := &fakeBrowser{tabs: []target.Info{{ID: "aa11", Title: "X", URL: "u"}, {ID: "bb22", Title: "Y", URL: "v"}}}
8791
env, _, code := run(t, b, "eval", "1+1", "--target", "aa11", "--json")
8892
if code != 0 {
@@ -94,6 +98,7 @@ func TestEvalResolvedTargetSucceeds(t *testing.T) {
9498
}
9599

96100
func TestExitCodesCommand(t *testing.T) {
101+
t.Parallel()
97102
var out, errb bytes.Buffer
98103
app := New(&fakeBrowser{}, &out, &errb)
99104
code := app.Execute("exit-codes")

internal/cli/config_wire_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ func clickCapture(t *testing.T, defs config.Defaults, args ...string) *queryCapt
2424
}
2525

2626
func TestConfigDefaultsFeedFlagDefaults(t *testing.T) {
27+
t.Parallel()
2728
defs := config.Defaults{By: "search", Wait: "ready", Timeout: 5 * time.Second}
2829
// No --by/--wait on the command line, so the config defaults take effect.
2930
b := clickCapture(t, defs, "click", "#x", "--target", "aa11", "--json")
@@ -33,6 +34,7 @@ func TestConfigDefaultsFeedFlagDefaults(t *testing.T) {
3334
}
3435

3536
func TestExplicitFlagOverridesConfigDefault(t *testing.T) {
37+
t.Parallel()
3638
defs := config.Defaults{By: "search", Wait: "ready", Timeout: 5 * time.Second}
3739
// An explicit --by must win over the config default.
3840
b := clickCapture(t, defs, "click", "#x", "--target", "aa11", "--by", "id", "--json")
@@ -45,6 +47,7 @@ func TestExplicitFlagOverridesConfigDefault(t *testing.T) {
4547
}
4648

4749
func TestConfigTargetIsFallback(t *testing.T) {
50+
t.Parallel()
4851
// With no --target and no sticky target, the config default target resolves.
4952
defs := config.Defaults{By: "css", Wait: "visible", Timeout: 5 * time.Second, Target: "@1"}
5053
b := &fakeBrowser{tabs: []target.Info{{ID: "aa11", Title: "A", URL: "u"}}}

internal/cli/fill_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ func (f *fillCapture) Wait(_ context.Context, _ string, c chrome.WaitCond) (map[
2626
}
2727

2828
func TestFillAndWaitTextCommand(t *testing.T) {
29+
t.Parallel()
2930
b := &fillCapture{fakeBrowser: fakeBrowser{tabs: []target.Info{{ID: "aa11", Title: "A", URL: "u"}}}}
3031
env, _, code := run(t, b,
3132
"fill", "#h", "8", "--wait-text", "Saved", "--target", "aa11", "--json")

0 commit comments

Comments
 (0)