Skip to content

feat: add Feishu QR scan-to-create registration with domain support#326

Open
yogkin wants to merge 1 commit intoNanmiCoder:mainfrom
yogkin:czm/add_feishu_qr_scan
Open

feat: add Feishu QR scan-to-create registration with domain support#326
yogkin wants to merge 1 commit intoNanmiCoder:mainfrom
yogkin:czm/add_feishu_qr_scan

Conversation

@yogkin
Copy link
Copy Markdown

@yogkin yogkin commented May 9, 2026

Summary

Feature Quality Contract

  • Changed surface:
  • Tests added or updated:
  • Coverage evidence:
  • E2E / live-model evidence:
  • Known risk / rollback:

Verification

  • I ran the relevant local checks, or explained why they do not apply.
  • I added or updated same-area tests for every production behavior change.
  • I ran bun run verify for code changes, including the coverage gate.
  • New or changed executable production lines meet the changed-line coverage threshold, or the blocker/maintainer override is documented.
  • I attached or summarized the quality report path, JUnit/log artifact path, and pass/fail/skip counts.
  • I ran E2E/live smoke for cross-boundary, provider/runtime, desktop chat, agent-loop, native, or release changes, or documented the blocker.

Risk

  • This PR does not touch CLI core paths, or it has maintainer approval for allow-cli-core-change.
  • Production code changes include matching tests, or have maintainer approval for allow-missing-tests.
  • Coverage baseline/threshold changes have maintainer approval for allow-coverage-baseline-change.
  • Quarantined tests still have owners, exit criteria, and unexpired review windows.
  • Provider/runtime changes were covered by mock contract tests, and live smoke was run or explicitly deferred.

@dosubot review this PR for changed-area risk, missing tests, docs impact, desktop startup risk, and CLI core impact.

@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label May 9, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 9, 2026

PR quality triage

Changed areas: area:adapters, area:desktop, area:server

CLI core policy: No CLI-core policy block detected.

Missing-test policy: Blocked by policy until a maintainer applies allow-missing-tests or matching tests are added.

Coverage baseline policy: No coverage-baseline policy block detected.

CLI core files:

  • none

Coverage policy files:

  • none

Expected checks:

  • change-policy
  • desktop-checks
  • server-checks
  • adapter-checks
  • desktop-native-checks
  • coverage-checks

Test coverage signals:

  • BLOCKING unless allow-missing-tests is applied: Desktop product files changed without a desktop test file in the PR.
  • BLOCKING unless allow-missing-tests is applied: Adapter product files changed without an adapter test file in the PR.

Risk notes:

  • Desktop state/API layer changed: verify store persistence, WebSocket behavior, and startup errors.

Hard merge gates still come from GitHub Actions, not AI review.

Dosu handoff: Dosu can be used as the AI reviewer for risk explanation, missing-test prompts, and maintainer Q&A. If it does not comment automatically from the PR template, ask:

@dosubot review this PR for changed-area risk, missing tests, docs impact, desktop startup risk, and CLI core impact.

@dosubot dosubot Bot added the enhancement New feature or request label May 9, 2026
@dosubot
Copy link
Copy Markdown

dosubot Bot commented May 9, 2026

Hi @yogkin! I'm Dosu and I’m helping the cc-haha team.

Here's a knowledge review of PR #326 across the requested risk areas:

Changed-Area Risk: MEDIUM

  • Touches adapter config (adapters/common/config.ts), server API routes, and desktop UI
  • New domain field defaults to 'feishu', maintaining backward compatibility
  • New API routes (/feishu/setup/begin, /feishu/setup/poll, /feishu/unbind) are additive — existing endpoints unchanged
  • Follows the established Dingtalk QR pattern, reducing novelty risk

Missing Tests: LOW RISK

  • ✅ Server-side flow covered: unbind, begin, and poll endpoints mocked and tested (+73 lines in adapters.test.ts)
  • ❌ No React component tests for the QR UI flow (timeout, error states)
  • ❌ Domain auto-detection logic not isolated in a unit test
  • Recommendation: consider adding a test verifying domain switching passes through to the Lark SDK client

Documentation Impact: MEDIUM-HIGH

  • FEISHU_DOMAIN env var introduced but not documented
  • QR registration flow is new user-facing functionality — existing Feishu docs need updating
  • Domain selection (Feishu vs Lark) and auto-detection behavior should be explained for non-Chinese users

Desktop Startup Risk: LOW

  • Config loading in loadConfig() adds an optional field with a safe default [1]
  • No changes to startup sequence, sidecar spawn logic, or initialization timing [2]
  • Worst case: invalid domain → Feishu adapter fails to init (logged), other adapters unaffected

CLI Core Impact: NONE

  • Zero CLI files modified. Changes are fully isolated to adapters, desktop UI, server API, and tests

Summary:

Area Risk Note
Changed-area Medium Core adapter config touched, but additive with defaults
Missing tests Low Server covered; UI tests absent but follows proven pattern
Docs impact Medium-High New env var + user-facing flow undocumented
Desktop startup Low Optional field, safe default, unchanged init sequence
CLI core None No CLI files modified

Recommendations:

  1. Document FEISHU_DOMAIN and the QR registration flow in the Feishu integration guide
  2. Consider a unit test for domain auto-detection logic and a UI integration test for QR timeout/error states
  3. Clarify migration path for users with existing manual Feishu appId/appSecret configurations

To reply, just mention @dosu.


Docs are dead. Just use Dosu.

Leave Feedback Ask Dosu about cc-haha Share Dosu with your team

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:adapters area:desktop area:server enhancement New feature or request needs-maintainer-approval size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant