mirror of
https://github.com/jackwener/OpenCLI.git
synced 2026-09-14 18:25:42 +08:00
jakevin
4475d4efe3
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)
230 lines
10 KiB
JavaScript
230 lines
10 KiB
JavaScript
import { describe, expect, it } from 'vitest';
|
|
import { JSDOM } from 'jsdom';
|
|
import { __test__ } from './shared.js';
|
|
import { ArgumentError } from '@jackwener/opencli/errors';
|
|
|
|
const { extractMedia, parseTweetUrl, buildTwitterArticleScopeSource } = __test__;
|
|
|
|
describe('twitter parseTweetUrl', () => {
|
|
it('accepts exact Twitter/X tweet URLs and preserves query parameters', () => {
|
|
expect(parseTweetUrl('https://x.com/alice/status/2040254679301718161?s=20')).toEqual({
|
|
id: '2040254679301718161',
|
|
url: 'https://x.com/alice/status/2040254679301718161?s=20',
|
|
});
|
|
expect(parseTweetUrl('https://mobile.twitter.com/i/status/2040318731105313143')).toEqual({
|
|
id: '2040318731105313143',
|
|
url: 'https://mobile.twitter.com/i/status/2040318731105313143',
|
|
});
|
|
});
|
|
|
|
it('rejects non-https, off-domain, host-suffix, embedded, and path-suffix URLs', () => {
|
|
const invalid = [
|
|
'http://x.com/alice/status/2040254679301718161',
|
|
'https://evil.com/alice/status/2040254679301718161',
|
|
'https://x.com.evil.com/alice/status/2040254679301718161',
|
|
'https://evil.com/?next=https://x.com/alice/status/2040254679301718161',
|
|
'https://x.com/alice/status/2040254679301718161/photo/1',
|
|
];
|
|
for (const url of invalid) {
|
|
expect(() => parseTweetUrl(url)).toThrow(ArgumentError);
|
|
}
|
|
});
|
|
});
|
|
|
|
describe('twitter buildTwitterArticleScopeSource', () => {
|
|
// JSDOM-based tests prove the returned source actually works on real DOM —
|
|
// mocked `evaluate` tests in adapter specs only verify the script string
|
|
// contains expected tokens, but cannot catch silent matching bugs (cf.
|
|
// dianping #1312: mocked-evaluate single tests miss in-browser logic bugs).
|
|
function loadHelpers(tweetId, dom) {
|
|
const source = buildTwitterArticleScopeSource(tweetId);
|
|
const probe = new Function(
|
|
'document',
|
|
'window',
|
|
'URL',
|
|
`${source}\nreturn { findTargetArticle, __twHasLinkToTarget, __twGetStatusIdFromHref };`,
|
|
);
|
|
return probe(dom.window.document, dom.window, dom.window.URL);
|
|
}
|
|
function makeDom(html) {
|
|
return new JSDOM(`<html><body>${html}</body></html>`, { url: 'https://x.com/alice/status/2040254679301718161' });
|
|
}
|
|
|
|
it('finds the article whose link exactly matches the requested status id', () => {
|
|
const dom = makeDom(`
|
|
<article id="a"><a href="https://x.com/alice/status/2040254679301718161">link</a></article>
|
|
<article id="b"><a href="https://x.com/bob/status/9999999999999999999">link</a></article>
|
|
`);
|
|
const helpers = loadHelpers('2040254679301718161', dom);
|
|
const article = helpers.findTargetArticle();
|
|
expect(article?.id).toBe('a');
|
|
});
|
|
|
|
it('rejects substring matches — tweet id 123 must not match /status/1234567', () => {
|
|
// This is the codex-mini0 #1400 catch (substring vulnerability):
|
|
// `/status/123` was accepted as a substring of `/status/1234567`.
|
|
const dom = makeDom('<article><a href="https://x.com/alice/status/1234567">link</a></article>');
|
|
const helpers = loadHelpers('123', dom);
|
|
expect(helpers.findTargetArticle()).toBeUndefined();
|
|
});
|
|
|
|
it('rejects path-suffix attack — /status/<id>/photo/1 must not match status <id>', () => {
|
|
// Same regex anchor that parseTweetUrl uses — guards against attached
|
|
// paths like `/photo/1` that would otherwise pass with a loose suffix.
|
|
const dom = makeDom('<article><a href="https://x.com/alice/status/2040254679301718161/photo/1">link</a></article>');
|
|
const helpers = loadHelpers('2040254679301718161', dom);
|
|
expect(helpers.findTargetArticle()).toBeUndefined();
|
|
});
|
|
|
|
it('rejects off-domain links even when the path has the requested status id', () => {
|
|
const dom = makeDom('<article><a href="https://evil.com/alice/status/2040254679301718161">link</a></article>');
|
|
const helpers = loadHelpers('2040254679301718161', dom);
|
|
expect(helpers.findTargetArticle()).toBeUndefined();
|
|
});
|
|
|
|
it('rejects host-suffix and non-https status links', () => {
|
|
const dom = makeDom(`
|
|
<article id="suffix"><a href="https://x.com.evil.com/alice/status/2040254679301718161">link</a></article>
|
|
<article id="http"><a href="http://x.com/alice/status/2040254679301718161">link</a></article>
|
|
`);
|
|
const helpers = loadHelpers('2040254679301718161', dom);
|
|
expect(helpers.findTargetArticle()).toBeUndefined();
|
|
});
|
|
|
|
it('accepts exact Twitter/X status links with query and hash suffixes', () => {
|
|
const dom = makeDom('<article id="ok"><a href="https://mobile.twitter.com/alice/status/2040254679301718161?s=20#fragment">link</a></article>');
|
|
const helpers = loadHelpers('2040254679301718161', dom);
|
|
expect(helpers.findTargetArticle()?.id).toBe('ok');
|
|
});
|
|
|
|
it('matches /i/status/<id> URL form', () => {
|
|
const dom = makeDom('<article><a href="https://x.com/i/status/2040318731105313143">link</a></article>');
|
|
const helpers = loadHelpers('2040318731105313143', dom);
|
|
expect(helpers.findTargetArticle()).toBeTruthy();
|
|
});
|
|
|
|
it('__twHasLinkToTarget reports true on any descendant <a> matching tweet id', () => {
|
|
// Used by quote-card guard in quote.js — the quoted tweet card is not
|
|
// inside an <article>, but somewhere on the compose page.
|
|
const dom = makeDom(`
|
|
<div data-testid="card.wrapper">
|
|
<a href="https://x.com/alice/status/2040254679301718161">quoted card</a>
|
|
</div>
|
|
`);
|
|
const helpers = loadHelpers('2040254679301718161', dom);
|
|
expect(helpers.__twHasLinkToTarget(dom.window.document)).toBe(true);
|
|
});
|
|
|
|
it('__twGetStatusIdFromHref returns null on non-status URLs', () => {
|
|
const dom = makeDom('');
|
|
const helpers = loadHelpers('123', dom);
|
|
expect(helpers.__twGetStatusIdFromHref('https://x.com/alice/home')).toBeNull();
|
|
expect(helpers.__twGetStatusIdFromHref('https://x.com/alice/status/123/photo/1')).toBeNull();
|
|
expect(helpers.__twGetStatusIdFromHref('https://evil.com/alice/status/123')).toBeNull();
|
|
expect(helpers.__twGetStatusIdFromHref('https://x.com.evil.com/alice/status/123')).toBeNull();
|
|
expect(helpers.__twGetStatusIdFromHref('http://x.com/alice/status/123')).toBeNull();
|
|
expect(helpers.__twGetStatusIdFromHref('not a url')).toBeNull();
|
|
});
|
|
|
|
it('emits the canonical regex anchor — guards future maintainers from dropping ^ or $', () => {
|
|
const source = buildTwitterArticleScopeSource('123');
|
|
// Source-level assertion complements the JSDOM behavioural tests above.
|
|
// If a future refactor relaxes the anchor (e.g. drops ^ or $), the
|
|
// JSDOM tests would still pass on benign inputs but fail on adversarial
|
|
// cases. This token check ensures the regex shape itself is preserved.
|
|
expect(source).toContain('/^\\/(?:[^/]+|i)\\/status\\/(\\d+)\\/?$/');
|
|
});
|
|
});
|
|
|
|
describe('twitter extractMedia', () => {
|
|
it('returns false + empty list when legacy has no media', () => {
|
|
expect(extractMedia({})).toEqual({ has_media: false, media_urls: [] });
|
|
expect(extractMedia(undefined)).toEqual({ has_media: false, media_urls: [] });
|
|
expect(extractMedia({ extended_entities: { media: [] } })).toEqual({
|
|
has_media: false,
|
|
media_urls: [],
|
|
});
|
|
});
|
|
|
|
it('extracts photo urls from extended_entities', () => {
|
|
const result = extractMedia({
|
|
extended_entities: {
|
|
media: [
|
|
{ type: 'photo', media_url_https: 'https://pbs.twimg.com/media/a.jpg' },
|
|
{ type: 'photo', media_url_https: 'https://pbs.twimg.com/media/b.jpg' },
|
|
],
|
|
},
|
|
});
|
|
expect(result.has_media).toBe(true);
|
|
expect(result.media_urls).toEqual([
|
|
'https://pbs.twimg.com/media/a.jpg',
|
|
'https://pbs.twimg.com/media/b.jpg',
|
|
]);
|
|
});
|
|
|
|
it('prefers mp4 variant for video and animated_gif', () => {
|
|
const result = extractMedia({
|
|
extended_entities: {
|
|
media: [
|
|
{
|
|
type: 'video',
|
|
media_url_https: 'https://pbs.twimg.com/media/thumb.jpg',
|
|
video_info: {
|
|
variants: [
|
|
{ content_type: 'application/x-mpegURL', url: 'https://video.twimg.com/x.m3u8' },
|
|
{ content_type: 'video/mp4', url: 'https://video.twimg.com/x.mp4' },
|
|
],
|
|
},
|
|
},
|
|
{
|
|
type: 'animated_gif',
|
|
media_url_https: 'https://pbs.twimg.com/tweet_video_thumb/g.jpg',
|
|
video_info: {
|
|
variants: [
|
|
{ content_type: 'video/mp4', url: 'https://video.twimg.com/g.mp4' },
|
|
],
|
|
},
|
|
},
|
|
],
|
|
},
|
|
});
|
|
expect(result.has_media).toBe(true);
|
|
expect(result.media_urls).toEqual([
|
|
'https://video.twimg.com/x.mp4',
|
|
'https://video.twimg.com/g.mp4',
|
|
]);
|
|
});
|
|
|
|
it('falls back to media_url_https when no mp4 variant is available', () => {
|
|
const result = extractMedia({
|
|
extended_entities: {
|
|
media: [
|
|
{
|
|
type: 'video',
|
|
media_url_https: 'https://pbs.twimg.com/media/thumb.jpg',
|
|
video_info: { variants: [] },
|
|
},
|
|
],
|
|
},
|
|
});
|
|
expect(result).toEqual({
|
|
has_media: true,
|
|
media_urls: ['https://pbs.twimg.com/media/thumb.jpg'],
|
|
});
|
|
});
|
|
|
|
it('falls back to entities.media when extended_entities is missing', () => {
|
|
const result = extractMedia({
|
|
entities: {
|
|
media: [
|
|
{ type: 'photo', media_url_https: 'https://pbs.twimg.com/media/c.jpg' },
|
|
],
|
|
},
|
|
});
|
|
expect(result).toEqual({
|
|
has_media: true,
|
|
media_urls: ['https://pbs.twimg.com/media/c.jpg'],
|
|
});
|
|
});
|
|
});
|