feat(help): hard-gate empty positional help text + fix 18 offenders#1403
Merged
feat(help): hard-gate empty positional help text + fix 18 offenders#1403
Conversation
Why
- `opencli twitter followers --help` rendered:
Arguments:
user
with a blank trailing column. Both humans and agents could not
recover the parameter's purpose without reading source. WAWQAQ
surfaced this directly: "没有说明当后面的 followers [user] [options]
如果都没填的时候，获取的是什么？"
- This is metadata completeness, not stylistic taste. Failing closed
is the only way to keep the help surface trustworthy as adapters
land.
What
- src/build-manifest.ts: add `findManifestMetadataIssues()` that flags
any positional with empty / whitespace-only / missing `help`. Wired
into `main()` after the import-failures gate; build aborts non-zero
with a per-arg report (`site/cmd positional "name" (sourceFile)`).
- src/build-manifest.test.ts: cover the gate (positives + negatives,
scoped strictly to positionals — named flags are intentionally
out-of-scope).
- 18 adapter offenders (16 required + 2 optional) get explicit help
text:
twitter: followers/following/list-add/list-remove/list-tweets/
search/thread
reddit: search/subreddit/user/user-comments/user-posts
douyin: stats/update
bilibili: subtitle
jike: search
Optional positionals (`twitter followers/following [user]`) now
document the omit semantics — fetches the currently logged-in
account.
- CHANGELOG: document the build gate and the offender list.
Out of scope (planned follow-ups)
- Semantic-quality advisory: optional positional help should also
contain `default / omit / current / logged-in / required unless …`
keywords. That belongs to the planned Arg metadata v2 work
(`when_omitted / when_present / value_format` 3-field schema).
- Named-flag `help` quality. Named flags carry the flag name itself
in help, so a missing `help` is not as opaque; if we want to gate
those too, do it as a separate, intentional decision.
Validation
- `npm run build` → 799 entries, clean.
- `npm run typecheck` → clean.
- `npx vitest run --project unit --project adapter` → 257 + 4 files,
all green (build-manifest 13 tests, manifest gate added).
- Smoke: temporarily reverted `followers.js` help to empty → build
aborts with the exact `twitter/followers positional "user" (...)`
line; restored, build is clean again.
- `npm run check:silent-column-drop` and `check:typed-error-lint`
baselines unchanged.
jackwener
added a commit
that referenced
this pull request
May 7, 2026
…agement scoring, sibling dedupe + help docs (#1406) Round 21 follow-up to #1400 (P0 write-action symmetry, merged `644d4517`). 5 features + help docs unified into one PR per WAWQAQ "全部合成一个 PR" directive. ## Scope - **P1** (`cf10c098`): `twitter search` `--from / --has / --exclude / --product` filters, mapping to X `from:` / `filter:` / `-filter:` / `f=` operators; legacy `--filter top|live` preserved (--product win on conflict) - **P2** (`a484a69a`): new `twitter bookmark-folders` + `bookmark-folder <id>`; X Premium GraphQL `bookmarkFoldersSlice` + `BookmarkFolderTimeline`; queryId 三层 fallback (placeholder.json → client-web bundle → pinned constants) - **P3** (`f209f914`): `--top-by-engagement N` to 7 tweet-shaped read commands (search/timeline/likes/bookmarks/list-tweets/tweets/thread); single helper in `utils.js`; formula `likes×1 + retweets×3 + replies×2 + bookmarks×5 + log10(views+1)×0.5`; **N=0 reference equality no-op** → existing 157 twitter tests 0 churn - **P4** (`89283fa0`): `TWITTER_BEARER_TOKEN` + composer image helpers extracted to `utils.js` (12 GraphQL adapter dedup); reply hardening; quote adds `--image` - **P5** (`a3d10a48`): sibling article-scope helper extracted to `shared.js` (9 write commands reuse, dedup with #1400 P0 invariant) - **docs** (`2a358d80`): help-doc precision (positional-omitted defaults + download/bookmarks/notifications/timeline/lists description thicken; concurrent #1401/#1403 wording preserved) 47 files / +2594/-470. Tests **96 → 216 (+120)**, manifest 798 → 801 (+3), typed-error-lint 190 → 189 (resolved 1 grandfathered sentinel). ## Iteration history (3 review fix commits on top of 6 author commits) - `7f93779b` — codex-mini1 lead fix1: 3 blocker bundle (P5 host invariant + P2 safe-id + sentinel removal + P1 fallback fail-fast) - `2a29ecc6` — codex-mini1 lead fix2: P3 help formula consistency (doc/help text matches actual `log10(views+1)×0.5`) - `df4dcd76` — codex-mini1 lead fix3 (F-P-1 aux catch): P2 `bookmark-folder --limit` upfront validation (`Number(kwargs.limit ?? 20)` + reject non-positive/non-integer + regression `0/negative/fractional/NaN` + `page.goto` zero-call assert) ## 4 progressive blockers caught (codex-mini1 lead 3 rounds + F-P-1 aux 1 round) 1. **P5 host invariant gap** (lead): article-scope helper preserved exact `/status/<id>` path but ignored link host → off-domain `https://evil.com/alice/status/<target>` would satisfy `__twHasLinkToTarget`. Fixed: `https` + X/Twitter host or subdomain + exact `/status/<id>` or `/i/status/<id>` path; query/hash allowed; off-domain/host-suffix/non-https/path-suffix/substring-id rejected; JSDOM positive + 5 negative anchors. 2. **P2 listing→detail round-trip + sentinel** (lead): `bookmark-folders` accepted opaque IDs but `bookmark-folder <id>` only accepted numeric → round-trip broken; new `author: 'unknown'` sentinel created fabricated author URL. Fixed: `[A-Za-z0-9_-]+` opaque safe-id (rejects `/`, `?`, `%`, spaces) + `resolveTwitterQueryId()` sanitization for queryId resolution; sentinel removed → empty author + canonical `/i/status/<id>` URL. 3. **P1 fallback silent tab miss** (lead): pushState fail → fallback typing into search box, `clickProductTabIfNeeded()` silent return on tab not found → user `--product photos` silently degraded to Top results. Fixed: throw `CommandExecutionError` when requested `--product` tab cannot be selected + invalid `--from` / `--limit` upfront pre-nav reject + double-direction tests. 4. **P2 limit silent normalize** (aux): `const limit = kwargs.limit || 20` → `--limit 0` silent → 20; negative/non-integer pre-IO unchecked. Fixed: `Number(kwargs.limit ?? 20)` + require positive integer before `page.goto` + regression covers `0/negative/fractional/NaN` + `page.goto` zero-call. ## Cultural sediment (Round 21 audit checklist 7 rules / 6 dimensions) This PR **immediately validated 4 of 7 rules** in review pipeline: - (b) silent-clamp class — P1 fallback silent tab miss (silent semantic-downgrade) + P2 `|| 20` silent normalize - (e) ID exact-not-substring — P5 host invariant (was only path-exact, not host-exact) - (f) grandfathered-not-exempt — P5 helper-refactor boundary lost host invariant + P2 new adapter inherited grandfathered `'unknown'` sentinel - (g) fallback-must-have-success-criterion — P1 fallback path missing post-condition assertion 7 rules / 6 dimensions: - (a) cross-grep sibling URL pattern — structural - (b) silent-clamp class — failure mode (input) - (c) broad querySelector → article-scoping — scope - (d) missing-validation early reject — boundary - (e) ID exact-not-substring — identity - (f) grandfathered-not-exempt (corollary: applies to new file + new helper-refactor boundary; not original-file line-edit) — time-axis - (g) fallback-must-have-success-criterion (sub-rule g': fallback unit test must include post-condition assertion, not just "doesn't throw") — failure mode (output) **Cross-PR validation 4-chain on meta-anchor "Structural exactness for identity matching"**: - #1391 URL layer (`isFacebookAuthRedirectPath`: top-level anchor + `\.php` + `(/|$)` segment edge) - #1392 URL parser layer (`parseGrokSessionId`: bare UUID exact / URL host-exact-or-subdomain + path-exact) - #1400 DOM layer (article-scoping: status-id `/\/status\/${id}(?:\/|$)/` regex / segment-array exact) - #1406 P5 helper-refactor boundary (full URL invariant in shared helper: host+path re-anchored after extraction) - Common invariant: boundary-lock structural shape; **fuzzy match is silent-failure 温床**; lesson lifecycle = surface-shift not add-and-forget. **Audit framework self-discipline**: each rule must have grep-able detection signal, otherwise rule degenerates to mantra. Framework is "7 rules + sub-instance pattern in new surface", not frozen 7 rules. **Round 17 race-mitigation 第 9 连续 race-free execution**: standard alternation cadence (#1400 A 组 → #1406 B 组), lead final + aux final + `@pr-monitor squash?` trigger, pr-monitor proactive ack + serial squash, lead silent on closeout. ## Validation gates (final head `df4dcd76`) Local: Twitter adapter tests `25 files / 216 tests`, focused P1/P2/P3/P5 tests `99/99`, `node --check` touched runtime, `npx tsc --noEmit`, `npm run build`, manifest 801 entries, typed-error-lint `189/189`, silent-column-drop `103/103`, doc-coverage `140/140`, docs:build clean, listing-id advisory `13` unchanged (wikipedia/trending residual non-Twitter), `git diff --check` clean. GitHub: build×3 (ubuntu/macos/windows) SUCCESS, unit-test shards SUCCESS, bun-test SUCCESS, adapter-test SUCCESS, audit SUCCESS, doc-coverage SUCCESS, docs-build SUCCESS, smoke-test skipped, PR `CLEAN/MERGEABLE`. Reviewers: - Lead: @codex-mini1 (3 fix rounds, all caught proactively + amend P3 help consistency) - Aux: @First-principles-1 (better-solution triangulation on P2 queryId 三层 fallback + P5 invariant + P3 N=0 reference no-op + caught P2 limit silent normalize) - Author: @opencli-user (5-feature scope + 7-rule sediment co-author + corollary contributor)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
help.opencli twitter followers --helpused to renderArguments:\n userwith a blank column — unrecoverable for both humans and agents.[user]positional ontwitter followersandfollowingnow documents that omitting it fetches the currently logged-in account — exactly the gap WAWQAQ flagged in #OpenCLI:8bc52e11.This is PR α in the help-system overhaul plan agreed in #OpenCLI:8bc52e11. It is intentionally narrow: metadata completeness only, no semantic-quality enforcement, no named-flag scope. Follow-up PRs (A0 OptionSpec registry + browser dogfood, β Arg metadata v2 with
when_omitted/when_present/value_format, C list schema unify) will land separately.Why this is a hard gate, not advisory
opencli twitter followers --helprendered an empty trailing column for theuserarg. There is no way to recover the parameter's purpose without reading source. This is metadata completeness, not stylistic taste — failing closed is the only way to keep the help surface trustworthy as adapters land. Semantic quality (e.g. "what does the optional positional mean when omitted?") is intentionally NOT enforced here; that belongs to the planned Arg metadata v2 advisory pass.Affected adapters
followers(optional),following(optional),list-add,list-remove,list-tweets,search,threadsearch,subreddit,user,user-comments,user-postsstats,updatesubtitlesearchThe two optional positionals get omit-semantics text:
Test plan
npm run build→ 799 entries, cleannpm run typecheck→ cleannpx vitest run src/build-manifest.test.ts src/cli.test.ts src/help.test.ts src/commanderAdapter.test.ts→ 161 tests passednpx vitest run --project adapter→ 2007 tests passed (no regression)npm run check:silent-column-dropbaseline unchanged (103/103)npm run check:typed-error-lintbaseline unchanged (189/189)followers.jshelp to empty → build aborts withtwitter/followers positional "user" (twitter/followers.js); restored, build is cleannode dist/src/main.js twitter followers --helpshows the new help linecc @codex-coder for review per the plan in #OpenCLI:8bc52e11.