docs(skills): source-pack lessons from the Feishu live pass and the Notion loader review (#190)

* docs(skills): source-pack lessons from the Feishu live pass and the Notion loader review

Three families of hard-won guidance folded back into the skill:

- live-verification.md gains an OAuth-provider section (the credential
  is a flow: app + redirect URI + config PUT + browser authorization)
  and its four live-verified traps — provider scope unions the app
  cannot enable (Feishu 20027 → oomol-lab/open-connector#267),
  gateway-declared scopes the API does not honor (99991679 named
  im:message:readonly against declared get_as_user scopes → #268),
  capability/admin gates beyond scopes entirely (bot ability 232025,
  sensitive-permission approval), and grant-time token snapshots
  (re-authorize after every scope change).
- Probing now covers declared INPUT BOUNDS (Feishu's declared
  page_size max 100 vs the wire's 50, 99992402 → #269/#271) and the
  termination signal ON THE REAL FINAL PAGE (Feishu wiki's non-empty
  token beside has_more:false → has_more_path, #270).
- review-checklist.md hardens two items from the Notion approval
  review: the failure arm of every gate must be reachable through the
  PUBLIC entry point (serde's untagged buffering destroyed the
  evidence first_non_finite was checking for — dead code by
  construction), and every wire e2e asserts the EXACT input key set
  per request, absence included.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(skills): source-pack redaction and doc-sync lessons from the Feishu round-2 review

The round-2 pass on the Feishu pack (PR #186) found a P1 the skill's
existing rules were one level too shallow to prevent, plus four process
gaps. Folded back:

- Redaction must DECODE nested JSON-encoded strings and audit their
  leaves with the same allowlist (real member names survived inside
  body.content while the outer-tree audit passed), and the audit ships
  as an in-repo tripwire test so CI enforces the guarantee.
- Person-linked capture timestamps are coarsened; redacted
  cross-references must stay self-consistent (a row's url embedding
  its own guid); PII that ever reached a commit means branch-history
  rewrite, not a tip edit.
- Wire pins assert declared constants by VALUE (every table's
  page_size number), not key presence — pageSize is where a live
  contract defect actually surfaced.
- Zero-fixture-evidence columns are annotated doc-derived at the
  declaration.
- The pack doc's pushdown matrix is re-derived from the FINAL yaml
  after the live pass (a dropped pushdown must read `—`), and upstream
  gateway defects get filed as issues and linked from the pack doc.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: bakey

docs/superpowers/skills/source-pack/SKILL.md

@@ -128,9 +128,15 @@ missing invariant (with regression tests) is then prerequisite work.
 Implementation done and tests green is NOT done. Follow
 [references/live-verification.md](references/live-verification.md):
 have the user configure a real (free-tier) provider account in the
-gateway — you never touch the credential — then probe every action with
-the pack's exact inputs, diff real row keys against the mapped columns
-in both directions, scan every table end to end through skardi-server
+gateway — you never touch the credential; for OAuth providers the
+reference maps the full flow and its layered gates (scope unions the
+app cannot enable, gateway-declared scopes the API does not honor,
+capability flags and admin approvals beyond scopes, stale-token
+snapshots) — then probe every action with
+the pack's exact inputs AND the declared input bounds, diff real row
+keys against the mapped columns
+in both directions, verify termination on the real final page, scan
+every table end to end through skardi-server
 (registration passes the fingerprint gate against LIVE discovery; every
 mapped column extracts a real non-NULL value somewhere; pinned filters
 actually return rows; a small page size forces real multi-page

docs/superpowers/skills/source-pack/references/live-verification.md

@@ -55,6 +55,41 @@ accept it explicitly.
   the tables (e.g. for Notion: share a page and a database with the
   integration).
 
+### OAuth providers: the credential is a flow, and scopes are not the only gate
+
+For `authTypes: ["oauth2"]` providers the setup is heavier than an API
+key, and the Feishu pack's live pass (skardi PR #186) hit every trap in
+sequence — expect them:
+
+- **The flow**: the user creates the provider app themselves and
+  registers the gateway's redirect URI (`http://localhost:3000/oauth/callback`);
+  the user runs `PUT /api/oauth/configs/<service>` with
+  `{clientId, clientSecret}` from their env; then
+  `POST /api/oauth/authorizations {"service": "<service>"}` returns an
+  `authorizationUrl` the USER opens in a browser. You never touch the
+  secret or the browser session.
+- **The gateway may request an un-satisfiable scope set.** Providers
+  that build their OAuth scope list as the union of every action's
+  permissions can exceed what any real app enables (Feishu rejects the
+  authorize request outright, error 20027). Narrowing may require a
+  clearly-marked local patch to the provider definition plus an
+  upstream issue — precedent: oomol-lab/open-connector#267.
+- **Scopes granted ≠ scopes the API accepts.** A read can 401 with the
+  gateway-declared scopes present in the token: the provider error
+  often names the ACTUAL required scope, and the gateway's
+  `requiredScopes` metadata is then the bug (Feishu's 99991679 named
+  `im:message:readonly` while the actions declared `*.get_as_user` —
+  oomol-lab/open-connector#268). Read the provider's own error before
+  suspecting your pack.
+- **Capability gates sit beyond scopes entirely**: app-level abilities
+  (Feishu's bot capability, 232025) and tenant-admin approval of
+  high-sensitivity permissions are enforced independently of the OAuth
+  grant. Budget for console round-trips with the user.
+- **Every scope/permission change needs a FRESH token** — a
+  user_access_token snapshots its grants at authorization time, so
+  re-run the authorization after any change rather than debugging a
+  stale token.
+
 ## 1. Probe every action with real inputs first
 
 Before involving Skardi, call each chosen action directly
@@ -70,7 +105,20 @@ scratch directory (they become the fixture sources in §4). Check:
 - the real pagination envelope: force multi-page with a small page size
   (`pageSize: 1`) and confirm the continuation token appears where the
   pack's `next_cursor_path` / totals path expects it, and terminates on
-  the documented spelling.
+  the documented spelling;
+- **the declared input bounds, at the boundary**: send the pack's exact
+  page size and the schema's declared maximum — declared caps can
+  exceed the wire's (Feishu's `im/v1/messages` declares 100, hard-fails
+  above 50 with 99992402; skardi PR #186 shipped the corrected 50, and
+  oomol-lab/open-connector#269/#271 fixed the schema upstream);
+- **the termination signal on the REAL final page**: fetch to the end,
+  then deliberately follow whatever token the last page returns. Some
+  providers answer the final page with `has_more: false` beside a
+  NON-empty token (Feishu wiki's `"0||…"`), so null-token termination
+  refetches a finished scan and trips the loop guard — declare
+  `has_more_path` when the envelope carries an authoritative has-more
+  boolean, and pin the live shape with an e2e
+  (oomol-lab/open-connector#270 tracks the upstream normalization).
 
 ## 2. Diff real rows against the mapped columns — both directions
 
@@ -140,9 +188,36 @@ timestamp spellings, real URL shapes. Redaction methodology:
   timestamps, known structural enums). Anything that survives the
   filter gets looked at by eye. Real page titles hiding inside URL
   slugs are exactly what this catches.
+- **decode one level deeper**: any string value that itself parses as
+  JSON (Feishu's `body.content`, Slack blocks) must be decoded and its
+  string leaves run through the SAME allowlist. The Feishu round-2
+  blocker was exactly this: real member names survived inside a
+  JSON-encoded payload the outer-tree audit treated as one opaque
+  string.
+- **ship the audit as a tripwire test**, not a one-off pass: an
+  in-repo test that re-walks every fixture (including the nested
+  decode) so the redaction guarantee is enforced by CI, not by memory.
+  A cheap broad net helps too — e.g. "no CJK text in any fixture" when
+  the workspace's real names share a script no placeholder uses.
+- **coarsen capture timestamps** when rows encode person-linked events
+  (joins, messages, task completions): zero the trailing digits so the
+  instant stops being correlatable while magnitude and ordering (and
+  any digit-count-sensitive parsing) survive. Update tests that pinned
+  exact values.
+- **verify redaction self-consistency**: the deterministic counter only
+  helps if cross-references actually still line up — after redacting,
+  check that ids repeated across fields (a task's `url` embedding its
+  own `guid`) still match, and that provider-unique ids stayed unique.
+  A fixture whose value is "internally consistent live capture" must
+  survive its own cross-references.
 - deliberately-broken fixtures (the admission gate's schema-mismatch
   case) stay synthetic — say so in a comment.
 
+**If PII ever lands in a commit**: fixing the tip is not enough — the
+names remain in every earlier commit. Rewrite the branch history
+(squash/amend and force-push) so no reachable commit carries them, and
+say so in the review reply.
+
 Then update the fixture-driven tests to assert the live shapes, and the
 coverage-gap pins if columns moved.
 
@@ -152,7 +227,9 @@ The PR must let a reviewer see the verification without re-running it:
 per-table live results (row counts, which pinned filters returned rows,
 which columns carried real values), the pinned provider API version,
 what the live pass CHANGED (renamed/dropped/added columns and why), and
-what remains outside the fingerprint gate. Put the durable facts in the
+what remains outside the fingerprint gate. Upstream gateway defects the
+pass uncovered get FILED as issues on the gateway repo and linked from
+the pack doc — findings that live only in a PR body get lost. Put the durable facts in the
 module doc and pack doc; put the run evidence in the PR description or
 a comment. Stop the gateway and skardi-server when done, and remind the
 user to rotate any credential that was exposed.

docs/superpowers/skills/source-pack/references/review-checklist.md

@@ -25,6 +25,13 @@ The worst failure class: wrong results with a green status.
       authoritative total) — a filtered count is never a termination
       signal, and a missing signal means upstream contribution or
       deferral, not a heuristic.
+- [ ] Termination verified on the REAL final page, not assumed from the
+      envelope shape: providers can return a non-empty continuation
+      token beside `has_more: false` (Feishu wiki), which null-token
+      termination refetches until the loop guard kills the scan.
+      Declare `has_more_path` where the envelope carries an
+      authoritative has-more boolean, and pin the live final-page shape
+      with an e2e.
 - [ ] Short/empty non-final pages cannot truncate: if the envelope has
       an authoritative total, the strategy declares `total_pages_path`;
       if not, the heuristic's limits are documented.
@@ -77,9 +84,26 @@ The worst failure class: wrong results with a green status.
 - [ ] Per-declaration coverage: every table's own wire declarations
       (row path, input keys, pagination params) pinned by its own e2e —
       shared constants are not shared coverage.
+- [ ] Every wire e2e asserts the EXACT input key set per request
+      (sorted keys equal, absence included — page 1 carries no cursor),
+      not key presence: presence-only assertions cannot catch an
+      undeclared extra leaking onto the wire, and strict action schemas
+      turn that extra into a runtime 400.
+- [ ] Declared constants asserted by VALUE, not key presence — every
+      table's `page_size` pinned to its number on the wire (`pageSize`
+      is exactly where a live contract defect surfaced: declared 100,
+      wire caps at 50).
 - [ ] Both sides of every gate: the pass path (suite-wide) and the fail
       path (targeted test). A gate whose failure arm no test exercises
-      is dead code until proven otherwise.
+      is dead code until proven otherwise — and the failing input must
+      reach the gate THROUGH THE PUBLIC ENTRY POINT, not by calling the
+      guard function directly. Deserialization layers can destroy the
+      evidence before a post-hoc guard runs: serde_json's f64 visitor
+      converts a nested `.nan` to `null` during untagged buffering, so a
+      "reject non-finite" walk over the deserialized value could never
+      fire (the `first_non_finite` finding on the Notion PR — the fix
+      captures `serde_yaml::Value` and converts fallibly where the
+      evidence still exists).
 - [ ] Error tests assert the full identity (column/path/page/row/
       expected/found-kind) and that the offending VALUE never appears.
 - [ ] Negative-space guards for every "this deliberately doesn't
@@ -101,6 +125,18 @@ The worst failure class: wrong results with a green status.
       audited mechanically (every surviving string matched against an
       allowlist — real titles hide in URL slugs). Deliberately-broken
       fixtures (schema-mismatch) stay synthetic and say so.
+- [ ] The redaction audit DECODES nested JSON-encoded strings and
+      audits their leaves too (real names survived one decode level
+      down in a message payload), and it ships as an in-repo tripwire
+      test so CI enforces it. Person-linked capture timestamps are
+      coarsened; redacted cross-references stay self-consistent (an
+      id embedded in the row's own URL matches the row). If PII ever
+      reached a commit, the branch history was rewritten, not just the
+      tip.
+- [ ] Columns with ZERO fixture evidence (no captured row carries the
+      key) are annotated doc-derived at the declaration — under a
+      loose-schema pack, real rows are the only column truth, so an
+      evidence gap must be a reviewed fact, not an implicit one.
 - [ ] No real orgs/users/tokens anywhere; if a credential was ever
       pasted into a conversation or log during verification, the user
       was told to rotate it.
@@ -112,6 +148,12 @@ The worst failure class: wrong results with a green status.
 - [ ] No stale references: milestone numbers in Review notes, removed
       columns/tests still described, fixture-category lists, "pending"
       markers for work that has since landed.
+- [ ] The pack doc's table/pushdown matrix re-derived from the FINAL
+      yaml after the live pass — a pushdown the reconciliation dropped
+      must read `—`, not survive as a promise (the doc row is the
+      easiest artifact to forget when the wire invalidates a draft).
+- [ ] Upstream gateway defects found during verification are filed as
+      issues on the gateway repo and LINKED from the pack doc.
 - [ ] Operational consequences documented where behavior surprises:
       fingerprint pins fail on ANY schema change (additive included —
       re-capture and re-pin on upstream upgrades); raw-scan default-deny