diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml deleted file mode 100644 index d4fdf5f90..000000000 --- a/.github/workflows/docs.yml +++ /dev/null @@ -1,17 +0,0 @@ -name: Trigger Website Rebuild (Docs Updated) - -on: - push: - branches: [main] - paths: ['docs/**'] - -jobs: - dispatch: - runs-on: ubuntu-latest - steps: - - name: Trigger opencli-website rebuild - uses: peter-evans/repository-dispatch@v4 - with: - token: ${{ secrets.WEBSITE_DEPLOY_TOKEN }} - repository: jackwener/opencli-website - event-type: docs-updated diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..86ceedab9 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,20 @@ +# AGENTS.md + +Contributor guidance for AI coding tools (Cursor, Codex, Aider, etc.) and humans working in this fork. + +**The complete checklist lives in [`CLAUDE.md`](./CLAUDE.md)** — read it before editing `clis/*` or `extension/*` or opening a PR. + +The conventions were derived from observing how the upstream maintainer (`jackwener/OpenCLI`) amends PRs before merging. Following them upfront shortens review cycles. + +Quick summary of what's in `CLAUDE.md`: + +1. Use typed errors from `@jackwener/opencli/errors` — never `throw new Error()` +2. Validate `--limit` / `--page` fail-fast with `readPositiveInteger`, before `page.goto` +3. Prefer imperative `func:` over declarative `pipeline:` +4. Type-check after `JSON.parse` — `"null"` and `42` are valid JSON +5. Multi-source merge: merge first, filter last +6. Replace `Array.find` with scored sort when multiple matches are possible +7. Re-check socket / instance identity after every `await` +8. Test error paths, not just happy paths +9. Docs: name every lifecycle subtype when introducing limits +10. Rebuild `cli-manifest.json` after adapter arg changes diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..9cae1d2a0 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,139 @@ +# Contributing to GreyC/OpenCLI + +This fork's PRs are eventually upstreamed to `jackwener/OpenCLI`. The checklist below captures conventions the upstream maintainer reliably enforces by amending PRs before merging (observed across PRs #1538–#1542). Front-loading them shortens review. + +This file is auto-loaded by Claude Code and read by most AI coding tools (Cursor, Codex, Aider, etc.). Human contributors should read it before opening a PR. + +## Before submitting a PR — checklist + +### 1. Use typed errors, never `throw new Error(...)` + +In `clis/*` and `src/*`, import from `@jackwener/opencli/errors`: + +| Error | When | +|-------|------| +| `ArgumentError` | Invalid CLI arg (range, format, required). Throw **before** `page.goto`. | +| `AuthRequiredError` | Cookie expired / user not logged in (maps to exit 77). | +| `EmptyResultError` | Valid request, empty result set. | +| `CommandExecutionError` | Unexpected API shape, missing field, malformed payload. | + +Untyped `Error` collapses to a generic `COMMAND_EXEC` exit code and removes the caller's ability to differentiate "retry" vs "ask user to log in" vs "no data". + +### 2. Validate args fail-fast, before any navigation + +```js +import { readPositiveInteger } from './utils.js'; // boss adapter; similar helpers exist per-site + +const limit = readPositiveInteger(kwargs.limit, 'mycmd --limit', 20, 100); +const pageNum = readPositiveInteger(kwargs.page, 'mycmd --page', 1); +``` + +Reject bogus inputs before `page.goto` so they don't waste a browser context. + +### 3. Prefer imperative `func:` over declarative `pipeline:` + +Declarative `pipeline: [{navigate}, {evaluate}]` is fine for trivial scrapers but **cannot express error classification** (auth vs empty vs malformed all collapse). Use `func: async (page, kwargs) => { ... }` whenever you need to: + +- Distinguish error categories +- Branch on intermediate state +- Call multiple endpoints in sequence + +### 4. After `JSON.parse`, type-check the parsed value + +`JSON.parse` succeeding does not mean you got an object. `"null"`, `"true"`, `42`, `"[1,2]"` are all valid JSON that break `data.code` lookups. + +```js +const data = JSON.parse(text); +if (!data || typeof data !== 'object') { + throw new CommandExecutionError('API returned malformed response'); +} +if (!Array.isArray(data.zpData?.messages)) { + throw new CommandExecutionError('Expected messages array'); +} +``` + +### 5. Multi-source merge — merge first, filter last + +When combining a "primary" source with a "fallback" source, **do not gate the fallback on primary's count**. Primary may pass the count check but fail downstream filtering, leaving you short. + +```js +// Bad: skips fallback when primary has enough raw nodes +const combined = primary.length >= limit ? primary : [...primary, ...fallback]; + +// Good: always merge, let the row-level filter be the final gate +const combined = dedupe([...primary, ...fallback]); +``` + +### 6. `Array.find` smell — explicit scored selection + +When you write `xs.find(x => ...)`, ask: "if multiple match, which is correct?" + +- If answer is "any" → `find` is fine. +- If answer is "the focused one" / "the newest" / "the one with X" → write an explicit scored sort and pick `[0]`. Otherwise behaviour is order-dependent and surprising. + +### 7. Long-lived sockets + async handlers — guard stale messages after every await + +If a handler crosses an `await` boundary, the global state it depends on may have changed by the time it returns. Re-check identity **after every await**, not just at entry. + +```js +thisWs.onmessage = async (event) => { + if (ws !== thisWs) return; // entry guard + const result = await handleCommand(...); // long await + if (ws !== thisWs) return; // re-check + safeSend(thisWs, result); +}; +``` + +Also: `safeSend` checks `readyState === OPEN`, not "not CLOSING/CLOSED" — `CONNECTING` is also not sendable. + +### 8. Tests cover error paths, not just happy paths + +Every typed error you throw should have a test: + +```js +await expect(command.func(page, badInput)).rejects.toBeInstanceOf(ArgumentError); +``` + +Use the substring-dispatch mock pattern from `clis/boss/*.test.js`: + +```js +page.evaluate.mockImplementation(async (script) => { + if (script.includes('myEndpoint')) return mockPayload; + if (script.includes('document.cookie')) return 'fake-cookie'; + return {}; +}); +``` + +### 9. Docs — when introducing limits, name every lifecycle subtype + +If a concept has multiple lifecycles (owned vs bound, persistent vs ephemeral) and you mention a timeout/quota/limit, **specify which subtype it applies to in the same sentence**. Readers default to assuming the strictest variant applies to all. + +- Bad: "Browser sessions use a 10-minute idle timeout." +- Good: "**Owned** browser sessions use a 10-minute idle timeout. **Bound** sessions have no idle timer; they stay attached until …" + +### 10. After adapter changes, rebuild the manifest + +```bash +npm run build-manifest +git add cli-manifest.json +``` + +CLI option parsing reads from the **committed** `cli-manifest.json`, not from the adapter source at runtime. If you add `--side`, `--limit`, or any new option and forget to rebuild, the new flag is silently ignored when users run the CLI. + +## Verification before opening PR + +```bash +npm run build-manifest # if you touched clis//*.js or args +npx vitest run clis// # adapter tests +npx vitest run --project unit # unit tests +npm run typecheck -- --pretty false # type check +git diff --check # whitespace +``` + +## Related files + +- `clis/boss/utils.js` — reference for adapter helpers (`bossFetch`, `readPositiveInteger`, `assertOk`, `checkAuth`) +- `src/cli.ts` — the single-file CLI definition. Browser commands at lines 666+. +- `extension/src/background.ts` — Chrome extension service worker. Workspace lease policy at lines 209–261. +- `docs/guide/browser-bridge.md` — browser session lifecycle docs (owned vs bound) +- Memory: `~/.claude/projects/-Users-gangchen-Documents-Projects-OpenCLI/memory/feedback_opencli-pr-checklist.md` (Claude Code only — same content as this file, with owner-commit SHAs for each pattern) diff --git a/cli-manifest.json b/cli-manifest.json index 5d9285c80..9b8b066a9 100644 --- a/cli-manifest.json +++ b/cli-manifest.json @@ -3573,6 +3573,13 @@ "boss", "geek" ] + }, + { + "name": "raw", + "type": "boolean", + "default": false, + "required": false, + "help": "Return full message body including JD card payload (no 120-char truncation); extra fields appear in --fmt json/yaml" } ], "columns": [ diff --git a/clis/boss/chatmsg.js b/clis/boss/chatmsg.js index d422b4f70..e62ecc220 100644 --- a/clis/boss/chatmsg.js +++ b/clis/boss/chatmsg.js @@ -13,27 +13,37 @@ const TYPE_MAP = { 6: '名片', 7: '语音', 8: '视频', 9: '表情', }; -function mapBossMsg(m, friend) { +function mapBossMsg(m, friend, { raw = false } = {}) { const fromObj = m.from || {}; const isSelf = typeof fromObj === 'object' ? fromObj.uid !== friend.uid : false; - return { + const row = { from: isSelf ? '我' : (typeof fromObj === 'object' ? fromObj.name : friend.name), type: TYPE_MAP[m.type] || `其他(${m.type})`, text: m.text || m.body?.text || '', time: m.time ? new Date(m.time).toLocaleString('zh-CN') : '', }; + if (raw) { + row.security_id = m.securityId || null; + row.body = m.body || null; + } + return row; } -function mapGeekMsg(m, friend) { +function mapGeekMsg(m, friend, { raw = false } = {}) { const fromUid = m.from && m.from.uid; const isFromBoss = fromUid != null && String(fromUid) === String(friend.uid); - return { + const row = { from: isFromBoss ? '对方' : '我', type: TYPE_MAP[m.type] || `其他(${m.type})`, text: m.text || m.body?.text || m.body?.content || m.body?.showText || JSON.stringify(m.body || {}).slice(0, 120), time: m.time ? new Date(m.time).toLocaleString('zh-CN') : '', }; + if (raw) { + row.security_id = m.securityId || null; + row.body = m.body || null; + } + return row; } async function bossChatMsg(page, kwargs, existingFriend) { @@ -51,7 +61,7 @@ async function bossChatMsg(page, kwargs, existingFriend) { if (messages.length === 0) { throw new EmptyResultError('boss chatmsg', 'Boss returned no messages for this chat.'); } - return messages.map((m) => mapBossMsg(m, friend)); + return messages.map((m) => mapBossMsg(m, friend, { raw: !!kwargs.raw })); } async function geekChatMsg(page, kwargs, encryptSystemId) { @@ -59,7 +69,7 @@ async function geekChatMsg(page, kwargs, encryptSystemId) { if (!friend) throw new EmptyResultError('boss chatmsg', '未找到该聊天（geek 侧）'); if (!friend.securityId) throw new CommandExecutionError('该聊天缺少 securityId，无法获取历史消息'); const messages = await fetchGeekHistoryMsg(page, friend, { page: kwargs.page }); - return messages.map((m) => mapGeekMsg(m, friend)); + return messages.map((m) => mapGeekMsg(m, friend, { raw: !!kwargs.raw })); } cli({ @@ -75,13 +85,15 @@ cli({ { name: 'uid', required: true, positional: true, help: 'Encrypted UID (from chatlist)' }, { name: 'page', type: 'int', default: 1, help: 'Page number' }, { name: 'side', default: 'auto', choices: ['auto', 'boss', 'geek'], help: 'Identity side: auto (default), boss (recruiter), or geek (job-seeker)' }, + { name: 'raw', type: 'boolean', default: false, help: 'Return full message body including JD card payload (no 120-char truncation); extra fields appear in --fmt json/yaml' }, ], columns: ['from', 'type', 'text', 'time'], func: async (page, kwargs) => { requirePage(page); const uid = readRequiredString(kwargs.uid, 'chatmsg uid'); const pageNum = readPositiveInteger(kwargs.page, 'chatmsg --page', 1); - const normalizedKwargs = { ...kwargs, uid, page: pageNum }; + const raw = !!kwargs.raw; + const normalizedKwargs = { ...kwargs, uid, page: pageNum, raw }; const side = kwargs.side || 'auto'; if (side === 'boss') { @@ -112,6 +124,6 @@ cli({ if (!geekFriend) throw new EmptyResultError('boss chatmsg', 'uid 在招聘端与求职端聊天列表中均未找到'); if (!geekFriend.securityId) throw new CommandExecutionError('该聊天缺少 securityId，无法获取历史消息'); const messages = await fetchGeekHistoryMsg(page, geekFriend, { page: pageNum }); - return messages.map((m) => mapGeekMsg(m, geekFriend)); + return messages.map((m) => mapGeekMsg(m, geekFriend, { raw })); }, }); diff --git a/clis/boss/chatmsg.test.js b/clis/boss/chatmsg.test.js index bf4c0b38b..766f39561 100644 --- a/clis/boss/chatmsg.test.js +++ b/clis/boss/chatmsg.test.js @@ -209,6 +209,75 @@ describe('boss chatmsg', () => { ).rejects.toBeInstanceOf(CommandExecutionError); }); + describe('--raw mode (JD card exposure)', () => { + // Long content (>200 chars) sits inside body.jobDesc, not body.content, + // so the compact-mode short-circuit (m.body?.content) does not catch it + // and the JSON.stringify(...).slice(0, 120) truncation branch is exercised. + const JD_CARD_URL = 'https://www.zhipin.com/job_detail/abc-xyz-jobid-123.html?lid=test'; + const JD_CARD_CONTENT = '岗位职责：负责后端架构设计与核心服务开发，包括但不限于高并发、分布式系统、数据库优化、消息队列、缓存方案、监控告警、容灾备份等关键基础设施建设。任职要求：5年以上后端开发经验，精通 Java/Go，有大型互联网公司经验者优先，熟悉云原生技术栈（K8s/Docker/Service Mesh），具备良好的沟通协作能力，能承担技术攻坚任务，并在团队中起到技术引领作用。'; + const JD_CARD_FIXTURE = { + type: 99, + received: true, + time: 1704067200000, + securityId: 'jd-sec-id-xyz', + from: { uid: 67890, name: 'Boss张' }, + body: { + jobDesc: { + title: '高级后端工程师', + company: '某互联网公司', + salary: '40-60K·15薪', + city: '北京', + content: JD_CARD_CONTENT, + jobId: 'abc-xyz-jobid-123', + url: JD_CARD_URL, + }, + }, + }; + + function buildJdPageMock(msg) { + return createPageMock(async (script) => { + if (script.includes('document.cookie')) return 'test-enc-sys-id'; + if (script.includes('geekFilterByLabel')) { + return { code: 0, zpData: { friendList: [GEEK_FRIEND_LABEL] } }; + } + if (script.includes('getGeekFriendList.json')) { + return { code: 0, zpData: { result: [GEEK_FRIEND_ENRICHED] } }; + } + if (script.includes('geek/historyMsg')) { + return { code: 0, zpData: { messages: [msg] } }; + } + return {}; + }); + } + + it('--raw exposes full JD card url (no truncation)', async () => { + const page = buildJdPageMock(JD_CARD_FIXTURE); + const rows = await command.func(page, { uid: 'enc-geek-uid', page: 1, side: 'geek', raw: true }); + expect(rows).toHaveLength(1); + expect(rows[0].body).toBeDefined(); + expect(rows[0].body.jobDesc.url).toBe(JD_CARD_URL); + expect(rows[0].security_id).toBe('jd-sec-id-xyz'); + }); + + it('--raw preserves long JD content (>120 chars)', async () => { + const page = buildJdPageMock(JD_CARD_FIXTURE); + const rows = await command.func(page, { uid: 'enc-geek-uid', page: 1, side: 'geek', raw: true }); + expect(rows[0].body.jobDesc.content).toBe(JD_CARD_CONTENT); + expect(rows[0].body.jobDesc.content.length).toBeGreaterThan(120); + }); + + it('compact mode (no --raw) still truncates JD card body to <=120 chars and omits body/security_id', async () => { + const page = buildJdPageMock(JD_CARD_FIXTURE); + const rows = await command.func(page, { uid: 'enc-geek-uid', page: 1, side: 'geek' }); + expect(rows).toHaveLength(1); + expect(rows[0].text.length).toBeLessThanOrEqual(120); + expect(rows[0]).not.toHaveProperty('body'); + expect(rows[0]).not.toHaveProperty('security_id'); + // And the full URL is NOT present in the truncated text — this is the regression #13 documents. + expect(rows[0].text).not.toContain(JD_CARD_URL); + }); + }); + it('--side geek reports an empty history as EmptyResultError', async () => { const page = createPageMock(async (script) => { if (script.includes('document.cookie')) return 'test-enc-sys-id';