Skip to content

feat(verify): fixture-based value validation + skill docs for COOKIE pitfalls#1131

Merged
jackwener merged 2 commits intomainfrom
skill/cookie-verify-and-fixture-discipline
Apr 21, 2026
Merged

feat(verify): fixture-based value validation + skill docs for COOKIE pitfalls#1131
jackwener merged 2 commits intomainfrom
skill/cookie-verify-and-fixture-discipline

Conversation

@jackwener
Copy link
Copy Markdown
Owner

Summary

Addresses the three P0/P1 items from the 1point3acres adapter retro:

Runtime — opencli browser verify now validates values, not just "ran":

  • Reads ~/.opencli/sites/<site>/verify/<cmd>.json when present and checks row count / columns / types / patterns / notEmpty against live adapter output.
  • New flags --write-fixture / --update-fixture / --no-fixture.
  • Backward-compatible: no fixture ⇒ same behavior as today (runs the adapter, prints the output, exits 0 on success).
$ opencli browser verify 1point3acres/hot
  → 3 rows

  ✗ Adapter output does not match fixture:
    - [pattern] row[0] "url"="https://…thread-..." does not match /^https://EXPECTED-BOGUS-HOST//

Skill — opencli-adapter-author captures the gotchas that bit me writing the 1p3a COOKIE adapters:

  • adapter-template.md — new "COOKIE adapter 骨架" section:
    • Dual-domain page.getCookies({ domain: '.x' }) + ({ domain: 'www.x' }) merge, so HttpOnly auth cookies on the root domain don't silently drop
    • Why Node-side fetch(url, { headers: { Cookie } }) + TextDecoder('gbk') is the right shape for HTML / non-UTF-8 / navigateBefore: false adapters (vs page.evaluate(fetch(...)), which blows up with "Failed to fetch" under cross-origin or GBK)
    • Empty-state sentinel row over return []
  • api-discovery.md §4 — note that Discuz/phpBB/vBulletin engines set auth on the root domain, so a single-domain getCookies call comes up empty
  • SKILL.md runbook — Step 10 folds --write-fixture into the adapter-author loop; Step 12 adds the verify/<cmd>.json deliverable and forbids .dbg-* / raw HTML dumps in the repo tree

Fixture schema

{
  "args": { "limit": 3 },
  "expect": {
    "rowCount": { "min": 1, "max": 3 },
    "columns": ["rank", "tid", "title", "url"],
    "types":   { "rank": "number", "tid": "string|number", "url": "string" },
    "patterns": { "url": "^https://www\\\\.example\\\\.com/" },
    "notEmpty": ["title", "url"]
  }
}
  • types supports unions ("number|string") and "any" wildcard
  • patterns skip null/undefined values so optional fields don't false-positive
  • Derived fixtures from --write-fixture omit patterns/notEmpty — author hand-tunes

Test plan

  • npx vitest run src/ — 58 files / 820 passed / 2 skipped (same as main + new tests across verify-fixture.test.ts and cli.test.ts)
  • npx tsc --noEmit clean
  • opencli browser verify 1point3acres/hot — no fixture → prints rows + hint
  • --write-fixture → seeds ~/.opencli/sites/1point3acres/verify/hot.json with derived schema
  • Hand-tune patterns + notEmpty → re-verify → ✓ matches fixture
  • Flip pattern to wrong host → exit 1 + per-row failure list
  • --no-fixture → bypasses validation even when fixture exists
  • --write-fixture with existing fixture → no-op with hint to use --update-fixture

Non-goals

  • Exact-value equality match (BBS / market / news data is too volatile — shape-based checks catch the bugs that actually ship).
  • Recording HTTP request/response fixtures for offline replay (covered separately by ~/.opencli/sites/<site>/fixtures/<cmd>-<timestamp>.json, existing convention).

…pitfalls

`opencli browser verify` now loads `~/.opencli/sites/<site>/verify/<cmd>.json`
when present and validates row count / columns / types / patterns / notEmpty
against the live adapter output. Without a fixture, behavior is unchanged
(just runs the adapter and prints). New flags `--write-fixture`,
`--update-fixture`, `--no-fixture` seed / refresh / bypass the spec.

Motivation: previous verify only checked that the adapter exited 0 and
produced *something* — shape regressions (author name bleeding across rows,
a column silently becoming null after a site refresh, duplicated thread-level
time on every post) all passed "✓ Adapter works!" and shipped broken.

Skill doc updates (opencli-adapter-author):
- adapter-template.md: new "COOKIE adapter 骨架" section — HttpOnly +
  dual-domain cookie read via `page.getCookies`, Node-side fetch for HTML
  (explaining why `page.evaluate(fetch(...))` is the wrong tool when
  `navigateBefore: false` or the response is non-UTF-8), and empty-state
  sentinel row over `[]`
- api-discovery.md §4: note that BBS engines (Discuz/phpBB/vBulletin) set
  auth cookies on the root domain + HttpOnly, so single-domain `getCookies`
  calls silently miss them
- SKILL.md Step 10/12: make `--write-fixture` part of the runbook, forbid
  debug dumps outside `~/.opencli/sites/<site>/fixtures/` or `/tmp/`
Reviewer feedback blocker: fixture.args was Record<string, unknown>,
expanded as --key value only, so positional-subject adapters
(<tid>/<url>/<query>) couldn't be verified. Repo convention is
"主语优先 positional".

- verify-fixture.ts: args now accepts Record<string, unknown> | unknown[].
  Object → --k v pairs; array → verbatim passthrough. New helper
  expandFixtureArgs() centralizes the branching.
- cli.ts verify action: swap inline expansion for expandFixtureArgs().
- verify-fixture.test.ts: 6 new cases covering array form, mixed
  positional+flag, empty shapes, passthrough stringification.
- site-memory.md: Layer 2 tree now lists verify/<cmd>.json; new schema
  block distinguishes it from fixtures/<cmd>-<ts>.json; runbook timing
  section gets a Step 10 verify-write row. Repo-tree debug-dump ban
  clarified.
- adapter-template.md: new "Verify fixture" section with named-flag and
  positional recipes, honest about --write-fixture only seeding named.

Smoke-tested 1point3acres/thread (positional <tid>): fixture round-trip
green (args=["1173710","--limit","2"]).
@jackwener jackwener merged commit 5935191 into main Apr 21, 2026
13 checks passed
@jackwener jackwener deleted the skill/cookie-verify-and-fixture-discipline branch April 21, 2026 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant