diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3d5673c..d4bd497 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -26,5 +26,14 @@ jobs: - name: Scan i18n key references run: go run ./internal/i18n/cmd/check --scan-code + - name: Test Feishu shortcuts + run: go test ./shortcuts/feishu + + - name: Test workflow shortcuts + run: go test ./shortcuts/workflow + - name: Run Go tests run: go test ./... + + - name: Vet + run: go vet ./... diff --git a/README.md b/README.md index c7e7f29..2ae1252 100644 --- a/README.md +++ b/README.md @@ -632,6 +632,14 @@ gitlink-cli workflow +pr-summary --from shortcuts/workflow/testdata/pr_summary.j # Repository workflow report by read-only GitLink fetch gitlink-cli workflow +repo-report --owner Gitlink --repo gitlink-cli --format markdown +# Optional full PR review attribution. This deep-fetches formal reviews and +# PR-associated Issue journals for analyzed PRs, so keep it explicit. +gitlink-cli workflow +repo-report --owner Gitlink --repo gitlink-cli --include-pr-review-audit --format json > report.review-audit.json + +# Limit analysis only when an intentional sample is needed. +# By default, repo-report paginates through all open issues and pull requests. +gitlink-cli workflow +repo-report --owner Gitlink --repo gitlink-cli --issue-limit 20 --pr-limit 50 --format markdown + # Repository workflow report from a local JSON file gitlink-cli workflow +repo-report --from shortcuts/workflow/testdata/repo_report.json --format json ``` @@ -649,18 +657,26 @@ Safety: - They do not depend on LLM APIs. - `workflow +pr-summary` does not comment, approve, reject, or merge pull requests. - `workflow +repo-report` aggregates health, issue triage, and PR review summary signals without remote writes. +- `workflow +repo-report --include-pr-review-audit` remains read-only. It treats formal review objects as authoritative review evidence and keeps submitter, reviewer, participant, and system journal activity separate. ### Feishu Collaboration Export `feishu` turns `workflow +repo-report` JSON into Feishu collaboration outputs. +`workflow +repo-report` paginates through all open issues and pull requests by +default. Feishu cards label these values as analyzed counts. Passing +`--issue-limit` or `--pr-limit` intentionally limits the analysis and the +resulting values must not be interpreted as repository totals. + Stable usage: ```bash gitlink-cli workflow +repo-report --owner "$GITLINK_OWNER" --repo "$GITLINK_REPO" --format json > report.json +gitlink-cli workflow +repo-report --owner "$GITLINK_OWNER" --repo "$GITLINK_REPO" --include-pr-review-audit --format json > report.review-audit.json gitlink-cli feishu +notify --from-workflow-json report.json --format json gitlink-cli feishu +notify --from-workflow-json report.json --send --format table +gitlink-cli feishu +owner-digest --from-workflow-json report.review-audit.json --format table gitlink-cli feishu +weekly-report --from-workflow-json report.json --format markdown gitlink-cli feishu +owner-digest --from-workflow-json report.json --format markdown diff --git a/README.zh-CN.md b/README.zh-CN.md index cc48e55..f770289 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -555,6 +555,8 @@ gitlink-cli dataset +delete-attachment --owner me --repo proj --uuid --ye `feishu` 将 `workflow +repo-report` JSON 转成飞书协作内容。 +`workflow +repo-report` 默认分页读取并分析全部开放 Issue 和 PR。飞书卡片会把这些值明确标为“已分析数量”。只有显式传 `--issue-limit` 或 `--pr-limit` 时才会采样，此时结果不能解释为仓库总量。 + #### 稳定层：自定义机器人通知 稳定层只依赖飞书群自定义机器人。它适合把 GitLink 项目状态、周报、Owner 摘要和贡献者摘要推送到群里。默认只预览，真实发送必须显式传 `--send`。 @@ -562,6 +564,9 @@ gitlink-cli dataset +delete-attachment --owner me --repo proj --uuid --ye ```bash gitlink-cli workflow +repo-report --owner "$GITLINK_OWNER" --repo "$GITLINK_REPO" --format json > report.json +# 仅在明确需要采样时设置上限 +gitlink-cli workflow +repo-report --owner "$GITLINK_OWNER" --repo "$GITLINK_REPO" --issue-limit 20 --pr-limit 50 --format json > report.sample.json + gitlink-cli feishu +notify --from-workflow-json report.json --format json gitlink-cli feishu +notify --from-workflow-json report.json --send --format table diff --git a/docs/FEISHU_CAPABILITY_LAYERS.md b/docs/FEISHU_CAPABILITY_LAYERS.md index b43b89c..b15a56a 100644 --- a/docs/FEISHU_CAPABILITY_LAYERS.md +++ b/docs/FEISHU_CAPABILITY_LAYERS.md @@ -270,3 +270,33 @@ Authorization policy: GitLink write permissions must be defined by GitLink official maintainers, project owners, and deployers. This module must not hard-code a write-action authorization policy. ``` + +## Next-Stage Read-Only PR Activity Layer + +Before any GitLink action gateway, a read-only PR activity layer should support: + +```text +Complete open/merged/closed inventory. +Formal review status. +Review and journal attribution by submitter/reviewer/participant/system. +Previous-snapshot comparison. +Maintainer-role enrichment when authenticated member data is available. +Review-content fingerprints and change detection. +``` + +The generic actor, review, fallback, and snapshot rules are defined in +`docs/FEISHU_PR_ACTIVITY_STRATEGY.md`. Member lookup is optional enrichment: +when GitLink authentication or permission is unavailable, the CLI must not +guess that a participant is a maintainer. + +Current implementation status: + +```text +implemented: complete open/merged/closed PR inventory +implemented: optional read-only formal review and journal actor attribution +implemented: conservative reviewed/unreviewed classification +implemented: needs_re_review when submitter activity or PR updates happen after reviewer feedback +not implemented: previous-snapshot diff +not implemented: maintainer-role enrichment +not implemented: review-content fingerprint persistence +``` diff --git a/docs/FEISHU_OPENAPI_INVENTORY.md b/docs/FEISHU_OPENAPI_INVENTORY.md index d1c8c2e..9da2a26 100644 --- a/docs/FEISHU_OPENAPI_INVENTORY.md +++ b/docs/FEISHU_OPENAPI_INVENTORY.md @@ -16,6 +16,21 @@ Layer 3: Future callback-based GitLink action gateway No implemented command in this branch performs GitLink write operations. +## GitLink Read Sources Used by Feishu Exports + +The Feishu commands consume `workflow +repo-report` JSON. The workflow report +uses these GitLink read-only sources when the corresponding flags are enabled: + +| GitLink read source | Used by | Status | Notes | +| --- | --- | --- | --- | +| `GET /v1/{owner}/{repo}/issues?category=opened` | Issue analysis | Implemented | Paginates all open issues by default | +| `GET /v1/{owner}/{repo}/pulls?status=0` | Open PR analysis | Implemented | Paginates all open PRs by default | +| `GET /v1/{owner}/{repo}/pulls?status=1` | PR lifecycle totals | Implemented | Merged total only | +| `GET /v1/{owner}/{repo}/pulls?status=2` | PR lifecycle totals | Implemented | Closed/rejected total only | +| `GET /v1/{owner}/{repo}/pulls/{number}/reviews` | Optional review audit | Implemented read-only | Formal review objects are authoritative review evidence | +| `GET /v1/{owner}/{repo}/issues/{issue_id}/journals` | Optional review audit | Implemented read-only | Counts submitter/reviewer/participant/system activity without storing raw comment text | +| Repository member lookup | Optional role enrichment | Not implemented in this branch | Maintainer identity is not guessed when auth is unavailable | + ## Source Index Official Feishu / Lark references used for this inventory: @@ -503,6 +518,11 @@ Does not require the Feishu module to know a GitLink token. Provides summary-level issue, PR, contributor, recommendation, and health fields. ``` +`workflow +repo-report` paginates all open issues and pull requests by default. +`--issue-limit` and `--pr-limit` are explicit sampling controls. Counts in +Feishu output are labeled as analyzed counts and must not be interpreted as +repository totals after an explicit limit is applied. + Required future source expansion: ```text @@ -532,6 +552,11 @@ Audit source: future action gateway audit log ``` +Next-stage PR activity comparison uses the read-only PR review and associated +Issue journal endpoints. Actor attribution and snapshot-diff rules are defined +in `docs/FEISHU_PR_ACTIVITY_STRATEGY.md`. This remains planned; current Feishu +commands do not crawl or copy PR conversations. + Reason: ```text diff --git a/docs/FEISHU_PR_ACTIVITY_STRATEGY.md b/docs/FEISHU_PR_ACTIVITY_STRATEGY.md new file mode 100644 index 0000000..85f9728 --- /dev/null +++ b/docs/FEISHU_PR_ACTIVITY_STRATEGY.md @@ -0,0 +1,246 @@ +# PR Activity, Review Attribution, and Snapshot Strategy + +Status: next-stage design. No GitLink write operation is implemented here. + +## Goal + +Provide repository owners with a general, cross-repository view of: + +- all current open pull requests; +- merged and closed/rejected totals; +- state transitions since the previous assessment; +- whether a pull request received a formal review; +- whether conversation feedback came from the submitter, a reviewer, a + maintainer, another participant, a bot, or a system event; +- whether review/comment content changed since the previous snapshot. + +The model must not depend on a specific repository, user login, PR number, or +organization role name. + +## Verified Read Sources + +The GitLink API surfaces needed by this design are read-only: + +```text +GET /v1/{owner}/{repo}/pulls +GET /v1/{owner}/{repo}/pulls/{number} +GET /v1/{owner}/{repo}/pulls/{number}/reviews +GET /v1/{owner}/{repo}/issues/{issue_id}/journals +GET repository members when the current identity has permission +``` + +Local validation confirmed: + +- list responses expose PR state, author, associated issue ID, and pagination + totals; +- formal reviews expose reviewer identity, status, content, and time; +- journals expose actor identity, comment text, state events, created time, and + updated time; +- repository-member lookup may return 401 for an unauthenticated read. The + implementation must degrade to `participant`, not guess maintainer status. + +## Actor Classification + +Normalize identities by stable user ID first and login second. + +| Actor class | Evidence | +| --- | --- | +| `submitter` | Actor matches the PR author | +| `reviewer` | Actor owns a formal review or is in the assigned reviewer set | +| `maintainer` | Repository membership data proves a configured privileged role | +| `participant` | Authenticated human who is none of the above | +| `bot` | Explicit bot/application identity | +| `system` | State transition or generated event without human review content | +| `unknown` | Identity is incomplete | + +Role precedence: + +```text +system/bot -> submitter -> formal reviewer -> maintainer -> participant -> unknown +``` + +A user can be both maintainer and reviewer. Event attribution records the most +specific event relationship (`reviewer`) and may retain `is_maintainer=true` as +an additional property. + +## Review Standard + +Do not treat every comment as a review. + +### Authoritative review + +A formal review object with: + +```text +status: approved | rejected | common +reviewer identity +created_at +``` + +is authoritative review evidence. + +### Review-like journal feedback + +A journal comment is review feedback only when: + +1. it has non-empty human-authored content; +2. it is not a creation/status/system event; +3. the actor is a formal/assigned reviewer or a proven maintainer; +4. the actor is not the PR submitter, unless the UI explicitly marks a + self-review; +5. the normalized content is not only an acknowledgement such as `LGTM`, + `thanks`, or a generated status line, unless the product policy explicitly + enables acknowledgement reviews. + +When member data is unavailable, a comment from an unassigned actor remains +`participant_feedback`, not `maintainer_review`. + +## Risk Is Separate From Review + +Current `workflow +repo-report` risk is rule-based. A list-metadata keyword hit +is a risk hint, not proof that a reviewer found a problem. + +The next-stage output should keep separate fields: + +```text +metadata_risk_hint +code_change_risk +formal_review_status +review_feedback_status +merge_readiness +``` + +Detailed code risk requires files and commits. Bulk list metadata alone must +not be presented as a formal review conclusion. + +## Snapshot Model + +Recommended local snapshot: + +```json +{ + "schema_version": 1, + "repository": "owner/repo", + "generated_at": "RFC3339", + "totals": { + "open": 0, + "merged": 0, + "closed": 0 + }, + "pull_requests": [ + { + "number": 1, + "state": "open", + "author_id": "stable-id", + "updated_at": "RFC3339", + "head_revision": "optional", + "formal_review_status": "unreviewed", + "review_fingerprint": "sha256", + "conversation_fingerprint": "sha256", + "events": [] + } + ] +} +``` + +Raw access tokens and private profile fields must never enter snapshots. + +## Content Fingerprints + +Normalize review/comment content before hashing: + +1. normalize line endings; +2. trim leading/trailing whitespace; +3. collapse repeated whitespace outside code blocks; +4. remove generated status-only markup; +5. preserve code and semantic text; +6. hash actor ID, event type, normalized content, and event time. + +Store hashes and bounded summaries by default. Raw comment content should be +included only in an explicitly local evidence file. + +## Snapshot Diff + +Compare the current snapshot with `--previous-snapshot` and emit: + +```text +new_open_prs +newly_merged_prs +newly_closed_prs +reopened_prs +new_formal_reviews +review_status_changes +new_reviewer_feedback +edited_reviewer_feedback +submitter_responses +participant_feedback +``` + +State transitions are determined by PR number plus previous/current state, not +by subtracting aggregate totals. + +## Fetch Strategy + +Default inventory: + +1. paginate all PR list states; +2. record exact list totals and basic identity/state fields; +3. compare with the previous snapshot; +4. deep-fetch reviews and journals only for new or updated PRs. + +Optional full audit: + +```text +--full-review-audit +``` + +This explicitly deep-fetches every PR and may require hundreds of API calls. +Use bounded concurrency, retry/backoff, and a request summary. It must remain +read-only. + +## Planned Commands + +```text +gitlink-cli workflow +pr-activity-snapshot +gitlink-cli workflow +pr-activity-diff +gitlink-cli feishu +owner-activity-digest +``` + +The Feishu digest should show aggregate transitions and the most important +changed PRs. It must link to GitLink for full comments rather than copying an +unbounded conversation into a card. + +## Current Boundary + +Implemented in the current branch: + +- correct GitLink Issue and PR list filters; +- complete pagination for `workflow +repo-report` by default; +- PR lifecycle totals for open, merged, and closed/rejected states; +- explicit analyzed-count labels and scope notes; +- optional read-only formal review and journal actor attribution through + `workflow +repo-report --include-pr-review-audit`. + +The implemented audit follows the conservative review standard in this +document: + +- a formal `/pulls/{number}/reviews` object marks the PR as reviewed; +- journal comments from the PR submitter are counted as submitter responses, + not reviews; +- journal comments from an actor who also has a formal review on the PR are + counted as reviewer feedback; +- other human comments are counted as participant feedback; +- status changes and empty/generated events are counted as system events; +- a reviewed PR is marked `needs_re_review` when a later submitter comment, + later commit, or later PR update timestamp is newer than the latest reviewer + feedback timestamp; +- maintainer classification is not guessed when repository-member data is not + available. + +Not implemented in the current branch: + +- snapshot persistence; +- member-role enrichment; +- review-content diffing; +- Feishu activity-diff cards; +- any GitLink write operation. diff --git a/docs/pr-draft.md b/docs/pr-draft.md index 48bc8c6..29dc902 100644 --- a/docs/pr-draft.md +++ b/docs/pr-draft.md @@ -1,105 +1,184 @@ -# feat(workflow): add agent workflow commands for repository maintenance +# feat(feishu): add layered Feishu collaboration exports for workflow reports ## Summary -This PR adds four read-only workflow commands for repository maintenance: +- Add Feishu custom-bot cards for GitLink workflow reports. +- Add weekly, owner, and contributor digests. +- Add Bitable-ready schemas and records. +- Add experimental DocX, Bitable, and Task OpenAPI writes. +- Add read-only Open Platform readiness diagnostics. +- Add English and zh-CN Feishu output. +- Fix workflow Issue/PR list filters and paginate all open items by default. +- Add optional read-only PR review audit for formal reviews and comment actor attribution. +- Keep all GitLink write operations out of scope. -- `workflow +triage` -- `workflow +health` -- `workflow +pr-summary` -- `workflow +repo-report` +## Data Correctness -The commands provide rule-based, explainable analysis with stable `json`, concise `table`, -and copy-friendly `markdown` output. +`workflow +repo-report` now uses the GitLink API parameters used by the native +Issue and PR commands: -## Motivation +```text +Issue open filter: category=opened +PR open filter: status=0 +``` -Open-source maintainers often spend time on repetitive information organization before -making actual decisions: +It paginates all open issues and pull requests by default. An explicit +`--issue-limit` or `--pr-limit` enables sampling. -- Issue triage cost -- PR review cost -- repository health visibility -- Agent needs stable structured output +Feishu output labels these values as analyzed counts and includes a scope note. +This avoids presenting a limited sample as a repository total. -This PR adds workflow-level analysis on top of the existing GitLink CLI shortcut architecture -without introducing LLM dependencies or remote write behavior. +## PR Review Attribution -## Changes +`workflow +repo-report --include-pr-review-audit` performs a read-only audit of +the analyzed PRs: -### `workflow +triage` +```text +formal /pulls/{number}/reviews objects mark a PR as reviewed +submitter comments are counted separately and do not mark a PR as reviewed +participant comments are counted separately and do not mark a PR as reviewed +comments from actors with formal review identity are counted as reviewer feedback +maintainer role is not guessed when member lookup is unavailable +``` -- Classifies issues by type -- Scores priority and confidence -- Detects missing bug-report information -- Produces risk flags, recommended actions, suggested comments, and reasoning +This keeps metadata risk, formal review status, and conversation attribution as +separate signals. -### `workflow +health` +## Stable Surface -- Scores repository health -- Covers issue/PR backlog, activity, release, CI, docs, license, contributing, and Agent readiness signals -- Tolerates unknown metrics without failing the command +```text +feishu +bot-test +feishu +notify +feishu +weekly-report +feishu +owner-digest +feishu +contributor-digest +feishu +bitable-schema +feishu +bitable-records +feishu +task-preview +``` -### `workflow +pr-summary` +Stable commands preview locally by default. Custom-bot delivery requires +explicit `--send`. -- Summarizes PR metadata, changed files, and commits -- Produces change type, risk level, review focus, test suggestions, merge checklist, and reasoning -- Supports local JSON input and remote read-only PR fetch +## Readiness Diagnostics -### `workflow +repo-report` +```text +feishu +app-check +feishu +doc-check +feishu +bitable-check +feishu +task-check +``` -- Aggregates health, issue triage, and PR summary signals -- Produces a repository workflow report with score, risk level, recommendations, and reasoning -- Supports partial read-only remote aggregation when optional sections are unavailable +Local mode checks configuration only. `--remote` performs read/check OpenAPI +calls and does not create or modify Feishu or GitLink resources. + +## Experimental Surface + +```text +feishu +doc-export +feishu +bitable-sync +feishu +task-create +``` + +These commands require a Feishu self-built app and explicit `--send`. ## Safety -- Remote mode is read-only -- No LLM dependency -- No labels/comments/close operations -- No PR approve/reject/merge operations -- No `internal/output` change -- No new third-party dependency -- Test fixtures do not contain secrets or tokens +- Preview/check by default. +- Real Feishu side effects require explicit `--send`. +- Remote readiness calls require explicit `--remote`. +- GitLink write operations are not implemented. +- Card buttons are navigation-only. +- Secrets and resource IDs come from ignored local env files. +- CLI and smoke output redact sensitive values. +- Bitable sync never deletes records. -## Tests +## Real Validation -```bash -gofmt -w shortcuts/workflow/*.go shortcuts/register.go +The local test enterprise validated: + +```text +custom bot card delivery +English and zh-CN cards +DocX append +Bitable search/create/update +Feishu Task create +app/doc/bitable/task readiness diagnostics +``` + +The current real repository report validated complete default pagination: + +```text +open issues analyzed: 9 +open pull requests analyzed: 166 +open/merged/closed PR lifecycle totals: 166 / 65 / 74 +full review-audit result: 166 audited, 4 reviewed, 162 unreviewed +needs re-review after reviewer feedback: 0 +``` + +Task creation was not repeated during the final smoke because Feishu-side +deduplication is not implemented. + +## Validation Commands + +```powershell +go run . workflow +repo-report --owner "$env:GITLINK_OWNER" --repo "$env:GITLINK_REPO" --format json > .local\report.json +go run . workflow +repo-report --owner "$env:GITLINK_OWNER" --repo "$env:GITLINK_REPO" --include-pr-review-audit --format json > .local\report.review-audit.full.json + +go run . feishu +app-check --remote --format table +go run . feishu +doc-check --remote --format table +go run . feishu +bitable-check --tables reports,issues,prs,tasks --remote --format table +go run . feishu +task-check --remote --format table + +go run . feishu +notify --from-workflow-json .local\report.json --send --format table +go run . feishu +owner-digest --from-workflow-json .local\report.review-audit.full.json --send --format table +go run . feishu +doc-export --from-workflow-json .local\report.json --send --format table +go run . feishu +bitable-sync --from-workflow-json .local\report.json --send --format table + +go test ./shortcuts/feishu go test ./shortcuts/workflow go test ./... +go vet ./... ``` -Coverage includes: +## Review and Comment Attribution Boundary -- triage rules -- health scoring -- PR summary rules -- repo report aggregation -- fetch normalization -- partial failure handling -- `json` / `table` / `markdown` rendering -- local `--from` fixtures -- command wiring tests +Formal reviews and PR-associated Issue journals are consumed only by the +optional read-only audit path. Previous-snapshot comparison, member-role +enrichment, and review-content fingerprint persistence remain designed in: -## Documentation - -- `README.md` -- `docs/workflow-agent-design.md` -- `docs/workflow-agent-test-report.md` -- `skills/gitlink-workflow/SKILL.md` - -## Known Limitations - -- `workflow +release-notes` is not implemented. -- `workflow +stale` is not implemented. -- Real GitLink API shapes may require follow-up normalization. - -## Examples - -```bash -gitlink-cli workflow +triage --from shortcuts/workflow/testdata/issue_bug.json --format table -gitlink-cli workflow +health --from shortcuts/workflow/testdata/health_good.json --format markdown -gitlink-cli workflow +pr-summary --from shortcuts/workflow/testdata/pr_summary.json --format markdown -gitlink-cli workflow +repo-report --from shortcuts/workflow/testdata/repo_report.json --format markdown +```text +docs/FEISHU_PR_ACTIVITY_STRATEGY.md ``` + +## Evidence + +```text +reports/FEISHU_SMOKE_20260626.md +reports/FEISHU_SMOKE_20260627.md +reports/FEISHU_SMOKE_EVIDENCE_20260627.md +reports/FEISHU_PERMISSION_MATRIX.md +docs/FEISHU_OPENAPI_INVENTORY.md +``` + +## Out of Scope + +- GitLink issue comment or close. +- GitLink PR review, approve, reject, or merge. +- GitLink member management. +- Feishu callback server. +- Feishu-to-GitLink identity mapping. +- Automatic Base/table/field/view creation. +- Task project/section/assignee placement. +- Feishu-side Task deduplication. +- PR activity snapshot persistence. +- Review-content fingerprint diffing. +- Maintainer-role enrichment without authenticated member data. + +## Reviewer Questions + +- Should webhook export remain the stable main path? +- Should DocX, Bitable, and Task writes remain experimental? +- Should full PR review activity be a separate workflow command? +- Should member-role enrichment require authenticated GitLink access? +- Should future Feishu callbacks live in gitlink-cli or a separate service? diff --git a/reports/FEISHU_SMOKE_20260627.md b/reports/FEISHU_SMOKE_20260627.md new file mode 100644 index 0000000..62de5a6 --- /dev/null +++ b/reports/FEISHU_SMOKE_20260627.md @@ -0,0 +1,242 @@ +# Feishu Smoke Report + +Date: 2026-06-27 + +## Branch and Base Commit + +```text +branch: feat/feishu-export-clean +base commit: d7812df1af49519f9eb84def218bd3d5a9fdf02f +``` + +This smoke run included uncommitted data-correctness fixes that are documented +below and will receive a new commit after final validation. + +## Environment + +```text +Real Feishu test enterprise: used +Real GitLink repository: used +Custom bot: used +Self-built Feishu app: used +DocX target: used +Five split Bitable tables: used +GitLink write operations: not used +``` + +Real credentials and resource IDs remained in the ignored file: + +```text +.local/feishu-gitlink.env.ps1 +``` + +## Readiness Diagnostics + +| Command | Result | Side effect | +| --- | --- | --- | +| `feishu +app-check` | pass | none | +| `feishu +doc-check` | pass | none | +| `feishu +bitable-check --tables reports,issues,prs,tasks` | pass | none | +| `feishu +task-check` | pass with expected project/section/dedupe warnings | none | +| `feishu +app-check --remote` | pass | tenant token check only | +| `feishu +doc-check --remote` | pass with write-permission checks skipped | Wiki/DocX read/check only | +| `feishu +bitable-check --remote` | pass for four tables | sentinel search only | +| `feishu +task-check --remote` | pass with expected warnings | tenant token check only | + +## Data-Correctness Finding + +The first report generated: + +```text +issues analyzed: 19 +pull requests analyzed: 10 +``` + +The GitLink UI showed: + +```text +open issues: 9 +open pull requests: 166 +``` + +The values were real API-derived values, but the workflow request semantics +were wrong: + +1. Issue workflow fetch sent `state=open`. GitLink Issue list requires + `category=opened`, so the API ignored the filter and returned 9 open plus 10 + closed issues. +2. PR workflow fetch sent `state=open`. GitLink PR list requires `status=0`, so + the API ignored the filter and returned all states. +3. The old repo-report defaults analyzed only 20 issues and 10 PRs. +4. The API can cap a requested page at 50 records, so stopping only because a + page is shorter than the requested limit can truncate a report. + +## Data-Correctness Fix + +The branch now: + +```text +uses category=opened for open Issue queries +uses status=0 for open PR queries +paginates until the API total_count is reached +deduplicates list items by stable identifiers +recognizes GitLink PR index as the user-facing PR number +analyzes all open issues and PRs by default +uses --issue-limit/--pr-limit only as explicit sampling controls +labels Feishu counts as analyzed counts +adds a scope note that sampled values are not repository totals +``` + +Real post-fix result: + +```text +open issues analyzed: 9 +open pull requests analyzed: 166 +open PR lifecycle total: 166 +merged PR lifecycle total: 65 +closed/rejected PR lifecycle total: 74 +``` + +These values match the GitLink web UI badges used during the smoke run. + +## PR Risk Source + +The bulk repo report uses PR list metadata, not changed files, commits, reviews, +or journal comments. The 13 critical metadata classifications came from the +existing `security-sensitive keyword` rule: + +| PR | Metadata hit | +| --- | --- | +| 76 | token | +| 77 | token | +| 115 | token | +| 146 | token | +| 167 | secret | +| 171 | token, secret, credential | +| 173 | secret | +| 183 | secret | +| 189 | token | +| 225 | token | +| 254 | token | +| 280 | token | +| 293 | token | + +This is a metadata risk hint, not a formal reviewer conclusion. Detailed code +risk requires files and commits. Formal review status must be reported +separately. + +## Review and Journal API Validation + +Three historical PRs were used only as local read-only samples: + +| Sample | Formal review | Journal result | +| --- | --- | --- | +| PR 95 | one approved review with reviewer identity and content | review comments and merged event readable | +| PR 29 | one approved review with reviewer identity and content | review comment and merged event readable | +| PR 75 | no formal review object | review-like comments and rejected/closed event readable | + +The repository member list returned 401 without GitLink authentication. +Therefore a generic implementation must not guess maintainer identity. It can +still reliably distinguish the submitter, formal reviewer, participant, and +system event. The cross-repository strategy is documented in: + +```text +docs/FEISHU_PR_ACTIVITY_STRATEGY.md +``` + +## Full PR Review Audit + +After the strategy was implemented as an explicit read-only audit path, the +full open-PR inventory was audited with: + +```text +workflow +repo-report --include-pr-review-audit +``` + +Result: + +```text +PRs analyzed: 166 +PRs review-audited: 166 +PRs with formal review evidence: 4 +PRs without formal review evidence: 162 +PRs needing re-review after reviewer feedback: 0 +formal reviews: 4 +reviewer comments: 6 +submitter comments: 0 +participant comments: 436 +system events: 0 +audit errors: 0 +``` + +Review judgment is conservative: + +```text +formal /pulls/{number}/reviews objects mark a PR as reviewed +submitter comments do not mark a PR as reviewed +participant comments do not mark a PR as reviewed +comments by actors with formal review identity are counted as reviewer feedback +reviewed PRs are marked needs_re_review when later submitter comments, later commits, or later PR updates appear after the last reviewer feedback +maintainer identity is not guessed without member-role data +``` + +## Real Feishu Writes + +| Command group | Result | +| --- | --- | +| Eight webhook test/report/digest sends | HTTP 200, Feishu code 0 | +| Corrected full-analysis notify card | HTTP 200, Feishu code 0 | +| Corrected full-analysis owner digest | HTTP 200, Feishu code 0 | +| Full review-audit notify card | HTTP 200, Feishu code 0 | +| Full review-audit owner digest | HTTP 200, Feishu code 0 | +| Final English smoke notify card | HTTP 200, Feishu code 0 | +| Final English smoke owner digest | HTTP 200, Feishu code 0 | +| English and Chinese DocX append | 9 blocks each | +| Corrected Chinese DocX append | 11 blocks | +| Bitable full-analysis upsert | reports 1 updated; issues 5 updated; PRs 5 created/3 updated; contributors 1 updated; tasks 2 created/7 updated | +| Task create | intentionally skipped in this run to avoid duplicates | + +## Current Boundaries + +```text +No GitLink write operation. +No callback server. +No automatic Base/table/view creation. +No Feishu-side Task dedupe. +No PR activity snapshot persistence. +No previous-snapshot review diff. +No review-content fingerprint persistence. +No maintainer-role guess when member lookup is unavailable. +``` + +## Test Results + +| Check | Result | +| --- | --- | +| `go test ./shortcuts/feishu` | pass | +| `go test ./shortcuts/workflow` | pass | +| `go test ./shortcuts` | pass | +| `go test ./...` | pass | +| `go build .` | pass | +| `go vet ./...` | pass | +| `go run ./internal/i18n/cmd/check` | blocked by existing Windows locale line-ending issue | +| `go run ./internal/i18n/cmd/check --scan-code` | blocked by the same formatting check | + +The i18n line-ending and missing-key fix remains in its independent branch/PR +and is intentionally not duplicated into this Feishu change. + +## Screenshot Status + +The requested Windows computer-use connection failed twice during plugin +initialization: + +```text +failed to write kernel assets: path not found +``` + +No screenshot was fabricated or committed. Text evidence and API-derived +results remain the evidence for this run. See: + +```text +reports/FEISHU_SMOKE_EVIDENCE_20260627.md +``` diff --git a/reports/FEISHU_SMOKE_EVIDENCE_20260627.md b/reports/FEISHU_SMOKE_EVIDENCE_20260627.md new file mode 100644 index 0000000..3cfbf3c --- /dev/null +++ b/reports/FEISHU_SMOKE_EVIDENCE_20260627.md @@ -0,0 +1,61 @@ +# Feishu Smoke Evidence + +Date: 2026-06-27 + +Branch: + +```text +feat/feishu-export-clean +``` + +Base commit: + +```text +d7812df1af49519f9eb84def218bd3d5a9fdf02f +``` + +## Evidence Files + +| Evidence | Expected file | Status | Notes | +| --- | --- | --- | --- | +| Custom bot notify card | `reports/images/feishu-card-notify-redacted.png` | not captured | computer-use initialization failed | +| Owner digest card | `reports/images/feishu-owner-digest-redacted.png` | not captured | corrected card was sent successfully | +| DocX append result | `reports/images/feishu-docx-append-redacted.png` | not captured | corrected 11-block append passed | +| Bitable sync result | `reports/images/feishu-bitable-sync-redacted.png` | not captured | real upsert passed | +| Task create result | `reports/images/feishu-task-create-redacted.png` | not captured | historical result retained; creation was not repeated | +| Diagnostics output | `reports/images/feishu-diagnostics-terminal-redacted.png` | not captured | local and remote checks passed | + +No placeholder or fabricated image file is committed. + +## Text Evidence + +```text +reports/FEISHU_SMOKE_20260626.md +reports/FEISHU_SMOKE_20260627.md +reports/FEISHU_PERMISSION_MATRIX.md +reports/FEISHU_API_COLLECTION_CHECKLIST_20260626.md +docs/FEISHU_OPENAPI_INVENTORY.md +docs/FEISHU_PR_ACTIVITY_STRATEGY.md +``` + +## Redaction Checklist + +```text +[x] No webhook URL committed +[x] No app secret committed +[x] No tenant_access_token committed +[x] No document token committed +[x] No Base app token committed +[x] No table ID committed +[x] No task ID committed +[x] No open_id / union_id committed +[x] No personal account credential committed +[x] No unredacted screenshot committed +``` + +## Capture Rule + +Screenshots may be added only after the Windows automation connection works and +each image is reviewed for resource IDs, personal identities, and unrelated +conversation content. Until then, this document records the missing visual +evidence explicitly rather than presenting a fake pass. diff --git a/shortcuts/feishu/card.go b/shortcuts/feishu/card.go index f3ecb4a..0cc60b0 100644 --- a/shortcuts/feishu/card.go +++ b/shortcuts/feishu/card.go @@ -58,19 +58,41 @@ func BuildWorkflowCard(report workflow.RepoReportResult, include []string, title } if hasItem(include, "issues") { elements = append(elements, fields([]fieldValue{ - {Label: feishuLabel(lang, "issues"), Value: fmt.Sprintf("%d", report.IssueSummary.Total)}, + {Label: feishuLabel(lang, "issues_analyzed"), Value: fmt.Sprintf("%d", report.IssueSummary.Total)}, {Label: feishuLabel(lang, "high_risk_issues"), Value: fmt.Sprintf("%d", report.IssueSummary.HighRisk)}, {Label: feishuLabel(lang, "missing_info"), Value: fmt.Sprintf("%d", report.IssueSummary.MissingInfo)}, })) } if hasItem(include, "prs") { elements = append(elements, fields([]fieldValue{ - {Label: feishuLabel(lang, "pull_requests"), Value: fmt.Sprintf("%d", report.PRSummary.Total)}, + {Label: feishuLabel(lang, "prs_analyzed"), Value: fmt.Sprintf("%d", report.PRSummary.Total)}, {Label: feishuLabel(lang, "high_risk_prs"), Value: fmt.Sprintf("%d", report.PRSummary.HighRisk)}, })) + if report.PRLifecycle != nil { + elements = append(elements, fields([]fieldValue{ + {Label: feishuLabel(lang, "open_prs"), Value: fmt.Sprintf("%d", report.PRLifecycle.Open)}, + {Label: feishuLabel(lang, "merged_prs"), Value: fmt.Sprintf("%d", report.PRLifecycle.Merged)}, + {Label: feishuLabel(lang, "closed_prs"), Value: fmt.Sprintf("%d", report.PRLifecycle.ClosedOrRejected)}, + })) + } + if report.PRReviewAudit != nil { + elements = append(elements, fields([]fieldValue{ + {Label: feishuLabel(lang, "review_audited"), Value: fmt.Sprintf("%d", report.PRReviewAudit.Audited)}, + {Label: feishuLabel(lang, "reviewed_prs"), Value: fmt.Sprintf("%d", report.PRReviewAudit.Reviewed)}, + {Label: feishuLabel(lang, "unreviewed_prs"), Value: fmt.Sprintf("%d", report.PRReviewAudit.Unreviewed)}, + {Label: feishuLabel(lang, "needs_re_review"), Value: fmt.Sprintf("%d", report.PRReviewAudit.NeedsReReview)}, + {Label: feishuLabel(lang, "formal_reviews"), Value: fmt.Sprintf("%d", report.PRReviewAudit.FormalReviews)}, + })) + elements = append(elements, div(fmt.Sprintf("**%s**\n%s", + feishuLabel(lang, "review_actor_attribution"), + bulletList(reviewAuditActorLines(report.PRReviewAudit, lang), 6)))) + } if len(report.PRSummary.ReviewFocus) > 0 { elements = append(elements, div(fmt.Sprintf("**%s**\n%s", feishuLabel(lang, "review_focus"), bulletList(localizeFeishuLines(report.PRSummary.ReviewFocus, lang), 4)))) } + if lines := riskSourceLines(report.PRSummary.RiskSources); len(lines) > 0 { + elements = append(elements, div(fmt.Sprintf("**%s**\n%s", feishuLabel(lang, "risk_sources"), bulletList(lines, 8)))) + } } if len(report.Recommendations) > 0 { elements = append(elements, div(fmt.Sprintf("**%s**\n%s", feishuLabel(lang, "recommendations"), bulletList(localizeFeishuLines(report.Recommendations, lang), 5)))) @@ -78,6 +100,7 @@ func BuildWorkflowCard(report workflow.RepoReportResult, include []string, title if strings.TrimSpace(docURL) != "" { elements = append(elements, actionButton(feishuLabel(lang, "open_feishu_report"), docURL)) } + elements = append(elements, note(feishuLabel(lang, "analysis_scope"))) elements = append(elements, note(feishuLabel(lang, "preview_note"))) return baseCard(title, templateForRisk(report.RiskLevel), elements) } diff --git a/shortcuts/feishu/digest.go b/shortcuts/feishu/digest.go index 967f990..e11d2ea 100644 --- a/shortcuts/feishu/digest.go +++ b/shortcuts/feishu/digest.go @@ -3,6 +3,7 @@ package feishu import ( "fmt" "io" + "sort" "strings" "text/tabwriter" @@ -10,24 +11,27 @@ import ( ) type RoleDigest struct { - Role string `json:"role"` - Repository string `json:"repository"` - RepositoryURL string `json:"repository_url,omitempty"` - DocURL string `json:"doc_url,omitempty"` - HealthScore *int `json:"health_score,omitempty"` - HealthRisk string `json:"health_risk,omitempty"` - RiskLevel string `json:"risk_level"` - ReportScore int `json:"report_score"` - IssueTotal int `json:"issue_total"` - IssueHighRisk int `json:"issue_high_risk"` - IssueMissingInfo int `json:"issue_missing_info"` - PRTotal int `json:"pr_total"` - PRHighRisk int `json:"pr_high_risk"` - ReviewFocus []string `json:"review_focus,omitempty"` - Recommendations []string `json:"recommendations,omitempty"` - AttentionItems []string `json:"attention_items,omitempty"` - NextSteps []string `json:"next_steps,omitempty"` - BoundaryDescription string `json:"boundary_description"` + Role string `json:"role"` + Repository string `json:"repository"` + RepositoryURL string `json:"repository_url,omitempty"` + DocURL string `json:"doc_url,omitempty"` + HealthScore *int `json:"health_score,omitempty"` + HealthRisk string `json:"health_risk,omitempty"` + RiskLevel string `json:"risk_level"` + ReportScore int `json:"report_score"` + IssueTotal int `json:"issue_total"` + IssueHighRisk int `json:"issue_high_risk"` + IssueMissingInfo int `json:"issue_missing_info"` + PRTotal int `json:"pr_total"` + PRHighRisk int `json:"pr_high_risk"` + PRRiskSources map[string]int `json:"pr_risk_sources,omitempty"` + PRLifecycle *workflow.RepoPRLifecycle `json:"pr_lifecycle,omitempty"` + PRReviewAudit *workflow.RepoPRReviewAudit `json:"pr_review_audit,omitempty"` + ReviewFocus []string `json:"review_focus,omitempty"` + Recommendations []string `json:"recommendations,omitempty"` + AttentionItems []string `json:"attention_items,omitempty"` + NextSteps []string `json:"next_steps,omitempty"` + BoundaryDescription string `json:"boundary_description"` } func BuildOwnerDigest(report workflow.RepoReportResult, docURL string) RoleDigest { @@ -70,6 +74,9 @@ func BuildOwnerDigest(report workflow.RepoReportResult, docURL string) RoleDiges IssueMissingInfo: report.IssueSummary.MissingInfo, PRTotal: report.PRSummary.Total, PRHighRisk: report.PRSummary.HighRisk, + PRRiskSources: report.PRSummary.RiskSources, + PRLifecycle: report.PRLifecycle, + PRReviewAudit: report.PRReviewAudit, ReviewFocus: report.PRSummary.ReviewFocus, Recommendations: report.Recommendations, AttentionItems: uniqueDigestStrings(attention), @@ -118,6 +125,9 @@ func BuildContributorDigest(report workflow.RepoReportResult, docURL string) Rol IssueMissingInfo: report.IssueSummary.MissingInfo, PRTotal: report.PRSummary.Total, PRHighRisk: report.PRSummary.HighRisk, + PRRiskSources: report.PRSummary.RiskSources, + PRLifecycle: report.PRLifecycle, + PRReviewAudit: report.PRReviewAudit, ReviewFocus: report.PRSummary.ReviewFocus, Recommendations: report.Recommendations, AttentionItems: limitStrings(uniqueDigestStrings(attention), 8), @@ -140,8 +150,8 @@ func buildDigestCard(digest RoleDigest, title string, role string, lang string) fields([]fieldValue{ {Label: feishuLabel(lang, "report_score"), Value: fmt.Sprintf("%d", digest.ReportScore)}, {Label: feishuLabel(lang, "risk_level"), Value: digest.RiskLevel}, - {Label: feishuLabel(lang, "issues"), Value: fmt.Sprintf("%d", digest.IssueTotal)}, - {Label: feishuLabel(lang, "pull_requests"), Value: fmt.Sprintf("%d", digest.PRTotal)}, + {Label: feishuLabel(lang, "issues_analyzed"), Value: fmt.Sprintf("%d", digest.IssueTotal)}, + {Label: feishuLabel(lang, "prs_analyzed"), Value: fmt.Sprintf("%d", digest.PRTotal)}, }), fields([]fieldValue{ {Label: feishuLabel(lang, "high_risk_issues"), Value: fmt.Sprintf("%d", digest.IssueHighRisk)}, @@ -156,9 +166,31 @@ func buildDigestCard(digest RoleDigest, title string, role string, lang string) {Label: feishuLabel(lang, "health_risk"), Value: digest.HealthRisk}, })) } + if digest.PRLifecycle != nil { + elements = append(elements, fields([]fieldValue{ + {Label: feishuLabel(lang, "open_prs"), Value: fmt.Sprintf("%d", digest.PRLifecycle.Open)}, + {Label: feishuLabel(lang, "merged_prs"), Value: fmt.Sprintf("%d", digest.PRLifecycle.Merged)}, + {Label: feishuLabel(lang, "closed_prs"), Value: fmt.Sprintf("%d", digest.PRLifecycle.ClosedOrRejected)}, + })) + } + if digest.PRReviewAudit != nil { + elements = append(elements, fields([]fieldValue{ + {Label: feishuLabel(lang, "review_audited"), Value: fmt.Sprintf("%d", digest.PRReviewAudit.Audited)}, + {Label: feishuLabel(lang, "reviewed_prs"), Value: fmt.Sprintf("%d", digest.PRReviewAudit.Reviewed)}, + {Label: feishuLabel(lang, "unreviewed_prs"), Value: fmt.Sprintf("%d", digest.PRReviewAudit.Unreviewed)}, + {Label: feishuLabel(lang, "needs_re_review"), Value: fmt.Sprintf("%d", digest.PRReviewAudit.NeedsReReview)}, + {Label: feishuLabel(lang, "formal_reviews"), Value: fmt.Sprintf("%d", digest.PRReviewAudit.FormalReviews)}, + })) + elements = append(elements, div(fmt.Sprintf("**%s**\n%s", + feishuLabel(lang, "review_actor_attribution"), + bulletList(reviewAuditActorLines(digest.PRReviewAudit, lang), 6)))) + } if len(digest.AttentionItems) > 0 { elements = append(elements, div(fmt.Sprintf("**%s**\n%s", feishuLabel(lang, "attention"), bulletList(localizeFeishuLines(digest.AttentionItems, lang), 5)))) } + if lines := riskSourceLines(digest.PRRiskSources); len(lines) > 0 { + elements = append(elements, div(fmt.Sprintf("**%s**\n%s", feishuLabel(lang, "risk_sources"), bulletList(lines, 8)))) + } if len(digest.NextSteps) > 0 { elements = append(elements, div(fmt.Sprintf("**%s**\n%s", feishuLabel(lang, "suggested_next_steps"), bulletList(localizeFeishuLines(digest.NextSteps, lang), 5)))) } @@ -168,6 +200,7 @@ func buildDigestCard(digest RoleDigest, title string, role string, lang string) if digest.DocURL != "" { elements = append(elements, actionButton(feishuLabel(lang, "open_feishu_report"), digest.DocURL)) } + elements = append(elements, note(feishuLabel(lang, "analysis_scope"))) elements = append(elements, note(localizedBoundary(digest, lang))) template := templateForRisk(digest.RiskLevel) if role == "contributor" && digest.PRSummaryNeedsAttention() { @@ -228,6 +261,40 @@ func writeDigestMarkdown(w io.Writer, digest RoleDigest, lang string) error { lines = append(lines, fmt.Sprintf("- Health score: `%d`; health risk: `%s`", *digest.HealthScore, firstNonEmpty(digest.HealthRisk, "unknown"))) } } + if digest.PRLifecycle != nil { + if isChineseLang(lang) { + lines = append(lines, fmt.Sprintf("- PR 生命周期：开放 `%d`，已合并 `%d`，已关闭/拒绝 `%d`", + digest.PRLifecycle.Open, + digest.PRLifecycle.Merged, + digest.PRLifecycle.ClosedOrRejected, + )) + } else { + lines = append(lines, fmt.Sprintf("- PR lifecycle: open `%d`, merged `%d`, closed/rejected `%d`", + digest.PRLifecycle.Open, + digest.PRLifecycle.Merged, + digest.PRLifecycle.ClosedOrRejected, + )) + } + } + if digest.PRReviewAudit != nil { + if isChineseLang(lang) { + lines = append(lines, fmt.Sprintf("- Review 判别：已归因 `%d`，已被 review `%d`，未被 review `%d`，待重新 review `%d`，正式 Review `%d`", + digest.PRReviewAudit.Audited, + digest.PRReviewAudit.Reviewed, + digest.PRReviewAudit.Unreviewed, + digest.PRReviewAudit.NeedsReReview, + digest.PRReviewAudit.FormalReviews, + )) + } else { + lines = append(lines, fmt.Sprintf("- Review audit: audited `%d`, reviewed `%d`, unreviewed `%d`, needs re-review `%d`, formal reviews `%d`", + digest.PRReviewAudit.Audited, + digest.PRReviewAudit.Reviewed, + digest.PRReviewAudit.Unreviewed, + digest.PRReviewAudit.NeedsReReview, + digest.PRReviewAudit.FormalReviews, + )) + } + } if digest.RepositoryURL != "" { if isChineseLang(lang) { lines = append(lines, "- GitLink 仓库："+digest.RepositoryURL) @@ -254,6 +321,12 @@ func writeDigestMarkdown(w io.Writer, digest RoleDigest, lang string) error { return err } } + if lines := riskSourceLines(digest.PRRiskSources); len(lines) > 0 { + heading := feishuLabel(lang, "risk_sources") + if _, err := fmt.Fprintf(w, "\n## %s\n\n%s\n", heading, bulletList(lines, 8)); err != nil { + return err + } + } if len(digest.NextSteps) > 0 { heading := "Suggested next steps" if isChineseLang(lang) { @@ -272,28 +345,42 @@ func digestMarkdownLines(digest RoleDigest, lang string) []string { return []string{ fmt.Sprintf("- 报告分数：`%d`", digest.ReportScore), fmt.Sprintf("- 风险等级：`%s`", firstNonEmpty(digest.RiskLevel, "unknown")), - fmt.Sprintf("- Issue：总数 `%d`，高风险 `%d`，信息缺失 `%d`", digest.IssueTotal, digest.IssueHighRisk, digest.IssueMissingInfo), - fmt.Sprintf("- PR：总数 `%d`，高风险 `%d`", digest.PRTotal, digest.PRHighRisk), + fmt.Sprintf("- 已分析 Issue：`%d`，其中高风险 `%d`，信息缺失 `%d`", digest.IssueTotal, digest.IssueHighRisk, digest.IssueMissingInfo), + fmt.Sprintf("- 已分析 PR：`%d`，其中高风险 `%d`", digest.PRTotal, digest.PRHighRisk), + "- " + feishuLabel(lang, "analysis_scope"), } } return []string{ fmt.Sprintf("- Report score: `%d`", digest.ReportScore), fmt.Sprintf("- Risk level: `%s`", firstNonEmpty(digest.RiskLevel, "unknown")), - fmt.Sprintf("- Issues: `%d` total, `%d` high risk, `%d` missing info", digest.IssueTotal, digest.IssueHighRisk, digest.IssueMissingInfo), - fmt.Sprintf("- Pull requests: `%d` total, `%d` high risk", digest.PRTotal, digest.PRHighRisk), + fmt.Sprintf("- Issues analyzed: `%d`, including `%d` high risk and `%d` missing info", digest.IssueTotal, digest.IssueHighRisk, digest.IssueMissingInfo), + fmt.Sprintf("- Pull requests analyzed: `%d`, including `%d` high risk", digest.PRTotal, digest.PRHighRisk), + "- " + feishuLabel(lang, "analysis_scope"), } } func writeDigestTable(w io.Writer, digest RoleDigest, lang string) error { tw := tabwriter.NewWriter(w, 0, 0, 2, ' ', 0) - header := "ROLE\tREPOSITORY\tRISK\tSCORE\tISSUES\tHIGH_RISK_ISSUES\tPRS\tHIGH_RISK_PRS\tATTENTION" + header := "ROLE\tREPOSITORY\tRISK\tSCORE\tISSUES_ANALYZED\tHIGH_RISK_ISSUES\tPRS_ANALYZED\tHIGH_RISK_PRS\tOPEN_PRS\tMERGED_PRS\tCLOSED_PRS\tREVIEWED_PRS\tUNREVIEWED_PRS\tNEEDS_RE_REVIEW\tATTENTION" if isChineseLang(lang) { - header = "角色\t仓库\t风险\t分数\tIssue\t高风险Issue\tPR\t高风险PR\t关注项" + header = "角色\t仓库\t风险\t分数\t已分析Issue\t高风险Issue\t已分析PR\t高风险PR\t开放PR\t已合并PR\t已关闭PR\t已Review PR\t未Review PR\t待重新Review\t关注项" } if _, err := fmt.Fprintln(tw, header); err != nil { return err } - if _, err := fmt.Fprintf(tw, "%s\t%s\t%s\t%d\t%d\t%d\t%d\t%d\t%d\n", + openPRs, mergedPRs, closedPRs := 0, 0, 0 + if digest.PRLifecycle != nil { + openPRs = digest.PRLifecycle.Open + mergedPRs = digest.PRLifecycle.Merged + closedPRs = digest.PRLifecycle.ClosedOrRejected + } + reviewedPRs, unreviewedPRs, needsReReviewPRs := 0, 0, 0 + if digest.PRReviewAudit != nil { + reviewedPRs = digest.PRReviewAudit.Reviewed + unreviewedPRs = digest.PRReviewAudit.Unreviewed + needsReReviewPRs = digest.PRReviewAudit.NeedsReReview + } + if _, err := fmt.Fprintf(tw, "%s\t%s\t%s\t%d\t%d\t%d\t%d\t%d\t%d\t%d\t%d\t%d\t%d\t%d\t%d\n", digest.Role, digest.Repository, digest.RiskLevel, @@ -302,6 +389,12 @@ func writeDigestTable(w io.Writer, digest RoleDigest, lang string) error { digest.IssueHighRisk, digest.PRTotal, digest.PRHighRisk, + openPRs, + mergedPRs, + closedPRs, + reviewedPRs, + unreviewedPRs, + needsReReviewPRs, len(digest.AttentionItems), ); err != nil { return err @@ -309,6 +402,34 @@ func writeDigestTable(w io.Writer, digest RoleDigest, lang string) error { return tw.Flush() } +func riskSourceLines(sources map[string]int) []string { + if len(sources) == 0 { + return nil + } + keys := make([]string, 0, len(sources)) + for key := range sources { + keys = append(keys, key) + } + sort.Strings(keys) + lines := make([]string, 0, len(keys)) + for _, key := range keys { + lines = append(lines, fmt.Sprintf("%s: %d", key, sources[key])) + } + return lines +} + +func reviewAuditActorLines(audit *workflow.RepoPRReviewAudit, lang string) []string { + if audit == nil { + return nil + } + return []string{ + fmt.Sprintf("%s: %d", feishuLabel(lang, "reviewer_comments"), audit.ReviewerComments), + fmt.Sprintf("%s: %d", feishuLabel(lang, "submitter_comments"), audit.SubmitterComments), + fmt.Sprintf("%s: %d", feishuLabel(lang, "participant_comments"), audit.ParticipantComments), + fmt.Sprintf("%s: %d", feishuLabel(lang, "system_events"), audit.SystemEvents), + } +} + func digestHealth(report workflow.RepoReportResult) (*int, string) { if report.Health == nil { return nil, "" diff --git a/shortcuts/feishu/doc_export.go b/shortcuts/feishu/doc_export.go index 0eb6452..a3dc826 100644 --- a/shortcuts/feishu/doc_export.go +++ b/shortcuts/feishu/doc_export.go @@ -189,6 +189,26 @@ func BuildDocBlocks(report workflow.RepoReportResult, lang string) []DocBlock { textBlock(fmt.Sprintf(feishuLabel(lang, "doc_health"), healthScore, healthRisk)), textBlock(fmt.Sprintf(feishuLabel(lang, "doc_issues"), report.IssueSummary.Total, report.IssueSummary.HighRisk, report.IssueSummary.MissingInfo)), textBlock(fmt.Sprintf(feishuLabel(lang, "doc_prs"), report.PRSummary.Total, report.PRSummary.HighRisk)), + textBlock(feishuLabel(lang, "analysis_scope")), + } + if report.PRLifecycle != nil { + blocks = append(blocks, textBlock(fmt.Sprintf( + "%s=%d; %s=%d; %s=%d", + feishuLabel(lang, "open_prs"), report.PRLifecycle.Open, + feishuLabel(lang, "merged_prs"), report.PRLifecycle.Merged, + feishuLabel(lang, "closed_prs"), report.PRLifecycle.ClosedOrRejected, + ))) + } + if report.PRReviewAudit != nil { + blocks = append(blocks, textBlock(fmt.Sprintf( + "%s=%d; %s=%d; %s=%d; %s=%d; %s=%d", + feishuLabel(lang, "review_audited"), report.PRReviewAudit.Audited, + feishuLabel(lang, "reviewed_prs"), report.PRReviewAudit.Reviewed, + feishuLabel(lang, "unreviewed_prs"), report.PRReviewAudit.Unreviewed, + feishuLabel(lang, "needs_re_review"), report.PRReviewAudit.NeedsReReview, + feishuLabel(lang, "formal_reviews"), report.PRReviewAudit.FormalReviews, + ))) + blocks = append(blocks, textBlock(feishuLabel(lang, "review_actor_attribution")+":\n"+joinLines(reviewAuditActorLines(report.PRReviewAudit, lang), 6))) } if len(report.PRSummary.ReviewFocus) > 0 { blocks = append(blocks, textBlock(feishuLabel(lang, "doc_review_focus")+":\n"+joinLines(localizeFeishuLines(report.PRSummary.ReviewFocus, lang), 6))) diff --git a/shortcuts/feishu/feishu_test.go b/shortcuts/feishu/feishu_test.go index d191b14..3554fbc 100644 --- a/shortcuts/feishu/feishu_test.go +++ b/shortcuts/feishu/feishu_test.go @@ -77,6 +77,18 @@ func TestBuildWorkflowCardIncludesDocButton(t *testing.T) { if err != nil { t.Fatalf("readWorkflowReport returned error: %v", err) } + report.PRSummary.RiskSources = map[string]int{"security-sensitive keyword": 2} + report.PRReviewAudit = &workflow.RepoPRReviewAudit{ + Audited: 3, + Reviewed: 2, + Unreviewed: 1, + NeedsReReview: 1, + FormalReviews: 2, + ReviewerComments: 4, + SubmitterComments: 3, + ParticipantComments: 1, + SystemEvents: 2, + } card := BuildWorkflowCard(report, parseList(defaultInclude), "", "en", "https://example.feishu.cn/wiki/node") encoded, err := json.Marshal(card) if err != nil { @@ -85,6 +97,15 @@ func TestBuildWorkflowCardIncludesDocButton(t *testing.T) { if !strings.Contains(string(encoded), "Open Feishu report") { t.Fatalf("card missing doc button: %s", string(encoded)) } + if !strings.Contains(string(encoded), "Issues analyzed") || !strings.Contains(string(encoded), "not repository totals") { + t.Fatalf("card missing analyzed-count boundary: %s", string(encoded)) + } + if !strings.Contains(string(encoded), "PR risk rule sources") || !strings.Contains(string(encoded), "security-sensitive keyword: 2") { + t.Fatalf("card missing PR risk sources: %s", string(encoded)) + } + if !strings.Contains(string(encoded), "Reviewed PRs") || !strings.Contains(string(encoded), "Needs re-review") || !strings.Contains(string(encoded), "Reviewer comments: 4") { + t.Fatalf("card missing PR review audit: %s", string(encoded)) + } } func TestWebhookClientSendsPayload(t *testing.T) { @@ -186,6 +207,11 @@ func TestOwnerAndContributorDigestMapping(t *testing.T) { if owner.IssueTotal != report.IssueSummary.Total || owner.PRTotal != report.PRSummary.Total { t.Fatalf("owner digest counts = %+v", owner) } + report.PRReviewAudit = &workflow.RepoPRReviewAudit{Audited: 2, Reviewed: 1, Unreviewed: 1, NeedsReReview: 1, FormalReviews: 1} + owner = BuildOwnerDigest(report, "https://tenant.feishu.cn/wiki/node") + if owner.PRReviewAudit == nil || owner.PRReviewAudit.Reviewed != 1 { + t.Fatalf("owner digest missing review audit: %+v", owner) + } contributor := BuildContributorDigest(report, "") if contributor.Role != "contributor" { t.Fatalf("contributor digest role = %q", contributor.Role) @@ -201,6 +227,9 @@ func TestOwnerAndContributorDigestMapping(t *testing.T) { if !strings.Contains(string(encoded), "Open GitLink repository") { t.Fatalf("owner card missing repository button: %s", string(encoded)) } + if !strings.Contains(string(encoded), "Issues analyzed") || !strings.Contains(string(encoded), "not repository totals") { + t.Fatalf("owner card missing analyzed-count boundary: %s", string(encoded)) + } } func TestTaskCandidatesAreStable(t *testing.T) { diff --git a/shortcuts/feishu/l10n.go b/shortcuts/feishu/l10n.go index f7d0022..6c6af69 100644 --- a/shortcuts/feishu/l10n.go +++ b/shortcuts/feishu/l10n.go @@ -49,6 +49,7 @@ func localizeFeishuLines(values []string, lang string) []string { var feishuLabelsEN = map[string]string{ "attention": "Attention", + "analysis_scope": "Counts are items analyzed from workflow JSON and may be limited by --issue-limit/--pr-limit; they are not repository totals.", "boundary_contributor": "Contributor digest is role-oriented, not personalized. It does not use Feishu open_id or union_id routing.", "boundary_owner": "Owner digest is a read-only summary. It does not modify GitLink or Feishu resources.", "bot_generated": "Generated by gitlink-cli feishu +bot-test.", @@ -56,8 +57,8 @@ var feishuLabelsEN = map[string]string{ "bot_status": "Status", "bot_title": "GitLink Feishu integration test", "doc_health": "Health score: %s; health risk: %s", - "doc_issues": "Issues: total=%d, high_risk=%d, missing_info=%d", - "doc_prs": "Pull Requests: total=%d, high_risk=%d", + "doc_issues": "Issues analyzed: %d; high_risk=%d, missing_info=%d", + "doc_prs": "Pull Requests analyzed: %d; high_risk=%d", "doc_reasoning": "Reasoning", "doc_recommendations": "Recommendations", "doc_report_score": "Report score: %d", @@ -70,20 +71,36 @@ var feishuLabelsEN = map[string]string{ "high_risk_issues": "High-risk issues", "high_risk_prs": "High-risk PRs", "issues": "Issues", + "issues_analyzed": "Issues analyzed", "missing_info": "Missing info", "missing_info_issues": "Missing-info issues", + "merged_prs": "Merged PRs", + "closed_prs": "Closed/rejected PRs", + "open_prs": "Open PRs", "open_feishu_report": "Open Feishu report", "open_gitlink_repository": "Open GitLink repository", "owner_digest_title": "GitLink owner digest: %s", "contributor_digest_title": "GitLink contributor digest: %s", "preview_note": "Preview is read-only. Bitable records are generated locally by +bitable-records.", "pull_requests": "Pull requests", + "prs_analyzed": "PRs analyzed", "ready": "Ready", "recommendations": "Recommendations", "report_score": "Report score", "repository": "Repository", "review_focus": "Review focus", + "review_audited": "PRs review-audited", + "reviewed_prs": "Reviewed PRs", + "unreviewed_prs": "Unreviewed PRs", + "needs_re_review": "Needs re-review", + "formal_reviews": "Formal reviews", + "review_actor_attribution": "Review actor attribution", + "reviewer_comments": "Reviewer comments", + "submitter_comments": "Submitter comments", + "participant_comments": "Participant comments", + "system_events": "System events", "risk_level": "Risk level", + "risk_sources": "PR risk rule sources", "source": "Source", "suggested_next_steps": "Suggested next steps", "task_description_default": "Workflow recommendation from gitlink-cli repo report.", @@ -96,6 +113,7 @@ var feishuLabelsEN = map[string]string{ var feishuLabelsZH = map[string]string{ "attention": "需要关注", + "analysis_scope": "数量表示 workflow JSON 中实际分析的条目，可能受 --issue-limit/--pr-limit 限制，不代表仓库总量。", "boundary_contributor": "贡献者摘要是按角色生成的汇总，不是基于飞书 open_id 或 union_id 的个人定向推送。", "boundary_owner": "Owner 摘要是只读汇总，不会修改 GitLink 或飞书资源。", "bot_generated": "由 gitlink-cli feishu +bot-test 生成。", @@ -103,8 +121,8 @@ var feishuLabelsZH = map[string]string{ "bot_status": "状态", "bot_title": "GitLink 飞书集成测试", "doc_health": "健康分：%s；健康风险：%s", - "doc_issues": "Issue：总数=%d，高风险=%d，信息缺失=%d", - "doc_prs": "PR：总数=%d，高风险=%d", + "doc_issues": "已分析 Issue：%d；高风险=%d，信息缺失=%d", + "doc_prs": "已分析 PR：%d；高风险=%d", "doc_reasoning": "判断依据", "doc_recommendations": "建议操作", "doc_report_score": "报告分数：%d", @@ -117,20 +135,36 @@ var feishuLabelsZH = map[string]string{ "high_risk_issues": "高风险 Issue", "high_risk_prs": "高风险 PR", "issues": "Issue", + "issues_analyzed": "已分析 Issue", "missing_info": "信息缺失", "missing_info_issues": "信息缺失 Issue", + "merged_prs": "已合并 PR", + "closed_prs": "已关闭/拒绝 PR", + "open_prs": "开放 PR", "open_feishu_report": "打开飞书报告", "open_gitlink_repository": "打开 GitLink 仓库", "owner_digest_title": "GitLink Owner 摘要：%s", "contributor_digest_title": "GitLink 贡献者摘要：%s", "preview_note": "当前为只读预览。多维表格记录由 +bitable-records 在本地生成。", "pull_requests": "PR", + "prs_analyzed": "已分析 PR", "ready": "就绪", "recommendations": "建议操作", "report_score": "报告分数", "repository": "仓库", "review_focus": "审查重点", + "review_audited": "已审查归因 PR", + "reviewed_prs": "已被 review 的 PR", + "unreviewed_prs": "未被 review 的 PR", + "needs_re_review": "待重新 review", + "formal_reviews": "正式 Review", + "review_actor_attribution": "Review 评论来源归因", + "reviewer_comments": "Reviewer 评论", + "submitter_comments": "提交者评论", + "participant_comments": "参与者评论", + "system_events": "系统事件", "risk_level": "风险等级", + "risk_sources": "PR 风险规则来源", "source": "来源", "suggested_next_steps": "建议下一步", "task_description_default": "来自 gitlink-cli 仓库报告的工作流建议。", diff --git a/shortcuts/workflow/api_types.go b/shortcuts/workflow/api_types.go index 4a4fe53..8eb51fa 100644 --- a/shortcuts/workflow/api_types.go +++ b/shortcuts/workflow/api_types.go @@ -104,7 +104,7 @@ func apiList(data interface{}) []interface{} { case []interface{}: return v case map[string]interface{}: - for _, key := range []string{"issues", "pulls", "pull_requests", "files", "commits", "releases", "builds", "items", "records", "data"} { + for _, key := range []string{"issues", "pulls", "pull_requests", "reviews", "journals", "comments", "notes", "files", "commits", "releases", "builds", "items", "records", "data"} { if raw, ok := v[key]; ok { if items := apiList(raw); len(items) > 0 { return items diff --git a/shortcuts/workflow/api_types_test.go b/shortcuts/workflow/api_types_test.go index ec549f3..7cf5b0f 100644 --- a/shortcuts/workflow/api_types_test.go +++ b/shortcuts/workflow/api_types_test.go @@ -495,8 +495,27 @@ func TestQueryWithPageLimit(t *testing.T) { func TestIssueListQuery(t *testing.T) { q := issueListQuery("open") - if q.Get("state") != "open" { - t.Fatalf("issueListQuery state = %q", q.Get("state")) + if q.Get("category") != "opened" || q.Get("state") != "" { + t.Fatalf("issueListQuery = %v, want category=opened", q) + } + q = issueListQuery("closed") + if q.Get("category") != "closed" { + t.Fatalf("issueListQuery closed = %v", q) + } +} + +func TestPullListQuery(t *testing.T) { + q := pullListQuery("open") + if q.Get("status") != "0" || q.Get("state") != "" { + t.Fatalf("pullListQuery = %v, want status=0", q) + } + q = pullListQuery("merged") + if q.Get("status") != "1" { + t.Fatalf("pullListQuery merged = %v", q) + } + q = pullListQuery("closed") + if q.Get("status") != "2" { + t.Fatalf("pullListQuery closed = %v", q) } } diff --git a/shortcuts/workflow/health_fetch.go b/shortcuts/workflow/health_fetch.go index 62066c5..a9aa6a2 100644 --- a/shortcuts/workflow/health_fetch.go +++ b/shortcuts/workflow/health_fetch.go @@ -39,7 +39,7 @@ func FetchHealthInput(ctx *common.RuntimeContext, opts HealthFetchOptions) (Heal input.RecentActivityKnown, input.RecentActivityDays, input = updateRecentActivity(input, latestTimeFromItems(issues)) } - if prs, err := fetchAllListItems(ctx, workflowRepoPath(owner, repo)+"/pulls", issueListQuery("open"), 100); err != nil { + if prs, err := fetchAllListItems(ctx, workflowRepoPath(owner, repo)+"/pulls", pullListQuery("open"), 100); err != nil { notes = append(notes, ScoringNote{Metric: "open_prs", Note: fmt.Sprintf("pull request probe failed: %v", err)}) } else { input.OpenPRs = len(prs) @@ -222,16 +222,50 @@ func queryWithPageLimit(base url.Values, page, limit int) url.Values { func issueListQuery(state string) url.Values { q := url.Values{} - q.Set("state", state) + switch strings.ToLower(strings.TrimSpace(state)) { + case "open", "opened", "": + q.Set("category", "opened") + case "closed": + q.Set("category", "closed") + case "all": + q.Set("category", "all") + default: + q.Set("category", state) + } + return q +} + +func pullListQuery(state string) url.Values { + q := url.Values{} + switch strings.ToLower(strings.TrimSpace(state)) { + case "open", "opened": + q.Set("status", "0") + case "merged": + q.Set("status", "1") + case "closed": + q.Set("status", "2") + case "all", "": + default: + q.Set("status", state) + } return q } func fetchAllListItems(ctx *common.RuntimeContext, path string, baseQuery url.Values, pageSize int) ([]map[string]interface{}, error) { + return fetchListItems(ctx, path, baseQuery, pageSize, 0) +} + +func fetchListItems(ctx *common.RuntimeContext, path string, baseQuery url.Values, pageSize, maxItems int) ([]map[string]interface{}, error) { if pageSize <= 0 { - pageSize = 100 + pageSize = 50 + } + if maxItems > 0 && pageSize > maxItems { + pageSize = maxItems } all := []map[string]interface{}{} - for page := 1; ; page++ { + seen := map[string]struct{}{} + totalCount := 0 + for page := 1; page <= 1000; page++ { query := cloneValues(baseQuery) query.Set("page", fmt.Sprintf("%d", page)) query.Set("limit", fmt.Sprintf("%d", pageSize)) @@ -240,6 +274,12 @@ func fetchAllListItems(ctx *common.RuntimeContext, path string, baseQuery url.Va if err != nil { return nil, err } + if env.Meta != nil && env.Meta.TotalCount > totalCount { + totalCount = env.Meta.TotalCount + } + if dataTotal := apiListTotal(env.Data); dataTotal > totalCount { + totalCount = dataTotal + } items := apiList(env.Data) pageItems := make([]map[string]interface{}, 0, len(items)) for _, raw := range items { @@ -250,14 +290,52 @@ func fetchAllListItems(ctx *common.RuntimeContext, path string, baseQuery url.Va if len(pageItems) == 0 { break } - all = append(all, pageItems...) - if len(pageItems) < pageSize { + before := len(all) + for _, item := range pageItems { + key := listItemIdentity(item) + if key != "" { + if _, exists := seen[key]; exists { + continue + } + seen[key] = struct{}{} + } + all = append(all, item) + if maxItems > 0 && len(all) >= maxItems { + return all[:maxItems], nil + } + } + if totalCount > 0 && len(all) >= totalCount { + break + } + if len(all) == before { + break + } + if totalCount == 0 && len(pageItems) < pageSize { break } } return all, nil } +func apiListTotal(data interface{}) int { + object := apiObject(data) + for _, key := range []string{"total_count", "opened_count", "total"} { + if total := apiInt(object[key]); total > 0 { + return total + } + } + return 0 +} + +func listItemIdentity(item map[string]interface{}) string { + for _, key := range []string{"id", "database_id", "index", "number", "iid", "project_issues_index"} { + if value := apiString(item[key]); value != "" { + return key + ":" + value + } + } + return "" +} + func cloneValues(values url.Values) url.Values { if values == nil { return url.Values{} diff --git a/shortcuts/workflow/pr_fetch.go b/shortcuts/workflow/pr_fetch.go index e8a854f..b70e3c9 100644 --- a/shortcuts/workflow/pr_fetch.go +++ b/shortcuts/workflow/pr_fetch.go @@ -156,7 +156,7 @@ func prAPIObject(data interface{}) map[string]interface{} { } func normalizePRSummaryItem(item map[string]interface{}) (PRSummaryInput, bool) { - number := firstPRInt(item, "number", "iid", "pull_request_number") + number := firstPRInt(item, "number", "index", "iid", "pull_request_number") title := firstPRString(item, "title", "subject") if number == 0 && strings.TrimSpace(title) == "" { return PRSummaryInput{}, false @@ -164,18 +164,28 @@ func normalizePRSummaryItem(item map[string]interface{}) (PRSummaryInput, bool) body := firstPRString(item, "body", "description", "content") state := firstPRString(item, "state", "status") author := firstPRAuthor(item) + issueID := firstPRIssueID(item) base := firstPRBranch(item, "base_branch", "target_branch", "base") head := firstPRBranch(item, "head_branch", "source_branch", "head") + createdAt := firstPRTime(item, "created_at", "createdAt") + updatedAt := apiLatestTime( + firstPRTime(item, "updated_at", "updatedAt"), + firstPRTime(item, "last_updated_at", "lastUpdatedAt"), + firstPRTime(item, "last_activity_at", "lastActivityAt"), + ) additions := firstPRInt(item, "additions", "additions_count") deletions := firstPRInt(item, "deletions", "deletions_count") return PRSummaryInput{ Number: number, + IssueID: issueID, Title: title, Author: author, State: state, BaseBranch: base, HeadBranch: head, + CreatedAt: createdAt, + UpdatedAt: updatedAt, Body: body, Additions: additions, Deletions: deletions, @@ -298,6 +308,26 @@ func firstPRCommitAuthor(item map[string]interface{}) string { return "" } +func firstPRIssueID(item map[string]interface{}) int { + for _, key := range []string{"issue_id", "issueId"} { + if value, ok := item[key]; ok { + if id := apiInt(value); id != 0 { + return id + } + } + } + for _, key := range []string{"issue", "issue_info"} { + if raw, ok := item[key].(map[string]interface{}); ok { + for _, field := range []string{"id", "issue_id"} { + if id := apiInt(raw[field]); id != 0 { + return id + } + } + } + } + return 0 +} + func firstPRBranch(item map[string]interface{}, keys ...string) string { for _, key := range keys { if value, ok := item[key]; ok { diff --git a/shortcuts/workflow/pr_review_audit.go b/shortcuts/workflow/pr_review_audit.go new file mode 100644 index 0000000..6cbb7b0 --- /dev/null +++ b/shortcuts/workflow/pr_review_audit.go @@ -0,0 +1,354 @@ +package workflow + +import ( + "fmt" + "net/url" + "sort" + "strings" + "time" + + "github.com/gitlink-org/gitlink-cli/shortcuts/common" +) + +const ( + reviewStandardFormal = "formal_review" + reviewStandardReviewerJournal = "reviewer_journal_feedback" + reviewStandardUnreviewed = "unreviewed" + + actorSubmitter = "submitter" + actorReviewer = "reviewer" + actorParticipant = "participant" + actorBot = "bot" + actorSystem = "system" + actorUnknown = "unknown" +) + +type normalizedPRReview struct { + Actor string + Status string + At time.Time +} + +func fetchPRReviewAudit(ctx *common.RuntimeContext, owner, repo string, prs []PRSummaryInput, maxItems int) (*RepoPRReviewAudit, []ScoringNote) { + if maxItems > 0 && len(prs) > maxItems { + prs = prs[:maxItems] + } + audit := &RepoPRReviewAudit{ + Source: "remote-read-only-fetch:reviews-and-journals", + PullRequests: make([]PRReviewAudit, 0, len(prs)), + } + notes := []ScoringNote{} + for _, pr := range prs { + item, itemNotes := fetchOnePRReviewAudit(ctx, owner, repo, pr) + notes = append(notes, itemNotes...) + audit.PullRequests = append(audit.PullRequests, item) + audit.Audited++ + if item.Reviewed { + audit.Reviewed++ + } else { + audit.Unreviewed++ + } + if item.NeedsReReview { + audit.NeedsReReview++ + } + audit.FormalReviews += item.FormalReviewCount + audit.ReviewerComments += item.ReviewerComments + audit.SubmitterComments += item.SubmitterComments + audit.ParticipantComments += item.ParticipantComments + audit.BotEvents += item.BotEvents + audit.SystemEvents += item.SystemEvents + audit.UnknownActorEvents += item.UnknownActorEvents + if len(item.Notes) > 0 { + audit.Errors += len(item.Notes) + } + } + return audit, uniqueScoringNotes(notes) +} + +func fetchOnePRReviewAudit(ctx *common.RuntimeContext, owner, repo string, pr PRSummaryInput) (PRReviewAudit, []ScoringNote) { + result := PRReviewAudit{ + Number: pr.Number, + Author: pr.Author, + IssueID: pr.IssueID, + ReviewStandard: reviewStandardUnreviewed, + FormalReviewStatus: "unreviewed", + } + notes := []ScoringNote{} + if result.Number <= 0 { + result.Notes = append(result.Notes, "missing PR number") + return result, []ScoringNote{{Metric: "repo_report_pr_review_audit", Note: "skipped PR review audit: missing PR number"}} + } + + if result.IssueID == 0 || strings.TrimSpace(result.Author) == "" { + base, err := fetchPRBase(ctx, owner, repo, result.Number) + if err != nil { + note := fmt.Sprintf("PR #%d base detail unavailable for review audit: %v", result.Number, err) + result.Notes = append(result.Notes, note) + notes = append(notes, ScoringNote{Metric: "repo_report_pr_review_audit", Note: note}) + } else { + if result.IssueID == 0 { + result.IssueID = base.IssueID + } + if strings.TrimSpace(result.Author) == "" { + result.Author = base.Author + } + if pr.CreatedAt.IsZero() { + pr.CreatedAt = base.CreatedAt + } + if pr.UpdatedAt.IsZero() { + pr.UpdatedAt = base.UpdatedAt + } + } + } + + reviews, err := fetchFormalPRReviews(ctx, owner, repo, result.Number) + if err != nil { + note := fmt.Sprintf("PR #%d formal reviews unavailable: %v", result.Number, err) + result.Notes = append(result.Notes, note) + notes = append(notes, ScoringNote{Metric: "repo_report_pr_review_audit", Note: note}) + } else { + result.FormalReviewCount = len(reviews) + result.FormalReviewStatus = summarizeFormalReviewStatus(reviews) + result.Reviewers = uniqueReviewerNames(reviews) + if latest := latestReviewTime(reviews); !latest.IsZero() { + result.LatestReviewerAt = latest.Format(time.RFC3339) + } + if result.FormalReviewCount > 0 { + result.Reviewed = true + result.ReviewStandard = reviewStandardFormal + } + } + + reviewerSet := reviewerSet(reviews) + var latestReviewerAt time.Time + if result.LatestReviewerAt != "" { + latestReviewerAt = apiTime(result.LatestReviewerAt) + } + var latestSubmitterAt time.Time + if result.IssueID == 0 { + note := fmt.Sprintf("PR #%d conversation journal skipped: missing associated issue id", result.Number) + result.Notes = append(result.Notes, note) + notes = append(notes, ScoringNote{Metric: "repo_report_pr_review_audit", Note: note}) + } else { + journals, err := fetchPRJournals(ctx, owner, repo, result.IssueID) + if err != nil { + note := fmt.Sprintf("PR #%d conversation journal unavailable: %v", result.Number, err) + result.Notes = append(result.Notes, note) + notes = append(notes, ScoringNote{Metric: "repo_report_pr_review_audit", Note: note}) + } else { + for _, journal := range journals { + switch classifyJournalActor(journal, result.Author, reviewerSet) { + case actorReviewer: + result.ReviewerComments++ + latestReviewerAt = apiLatestTime(latestReviewerAt, journalTime(journal)) + case actorSubmitter: + result.SubmitterComments++ + latestSubmitterAt = apiLatestTime(latestSubmitterAt, journalTime(journal)) + case actorParticipant: + result.ParticipantComments++ + case actorBot: + result.BotEvents++ + case actorSystem: + result.SystemEvents++ + default: + result.UnknownActorEvents++ + } + } + if !result.Reviewed && result.ReviewerComments > 0 { + result.Reviewed = true + result.ReviewStandard = reviewStandardReviewerJournal + } + } + } + if !latestReviewerAt.IsZero() { + result.LatestReviewerAt = latestReviewerAt.Format(time.RFC3339) + } + if !latestSubmitterAt.IsZero() { + result.LatestSubmitterAt = latestSubmitterAt.Format(time.RFC3339) + } + latestCommitAt := latestCommitTime(pr.Commits) + if !latestCommitAt.IsZero() { + result.LatestCommitAt = latestCommitAt.Format(time.RFC3339) + } + if !pr.UpdatedAt.IsZero() { + result.LatestPRUpdateAt = pr.UpdatedAt.Format(time.RFC3339) + } + result.NeedsReReview = needsReReview(latestReviewerAt, latestSubmitterAt, latestCommitAt, pr.UpdatedAt) + + return result, notes +} + +func fetchFormalPRReviews(ctx *common.RuntimeContext, owner, repo string, number int) ([]normalizedPRReview, error) { + items, err := fetchListItems(ctx, prPath(owner, repo, number)+"/reviews", url.Values{}, 50, 0) + if err != nil { + return nil, err + } + reviews := make([]normalizedPRReview, 0, len(items)) + for _, item := range items { + review := normalizedPRReview{ + Actor: firstReviewActor(item), + Status: strings.ToLower(strings.TrimSpace(firstPRString(item, "status", "state", "review_status"))), + At: journalTime(item), + } + if review.Actor == "" && review.Status == "" { + continue + } + if review.Status == "" { + review.Status = "common" + } + reviews = append(reviews, review) + } + return reviews, nil +} + +func latestReviewTime(reviews []normalizedPRReview) time.Time { + var latest time.Time + for _, review := range reviews { + latest = apiLatestTime(latest, review.At) + } + return latest +} + +func journalTime(item map[string]interface{}) time.Time { + return apiLatestTime( + firstPRTime(item, "updated_at", "updatedAt"), + firstPRTime(item, "created_at", "createdAt"), + ) +} + +func latestCommitTime(commits []PRCommit) time.Time { + var latest time.Time + for _, commit := range commits { + latest = apiLatestTime(latest, commit.Date) + } + return latest +} + +func needsReReview(latestReviewerAt, latestSubmitterAt, latestCommitAt, latestPRUpdateAt time.Time) bool { + if latestReviewerAt.IsZero() { + return false + } + return isAfter(latestSubmitterAt, latestReviewerAt) || + isAfter(latestCommitAt, latestReviewerAt) || + isAfter(latestPRUpdateAt, latestReviewerAt) +} + +func isAfter(value, baseline time.Time) bool { + return !value.IsZero() && !baseline.IsZero() && value.After(baseline) +} + +func fetchPRJournals(ctx *common.RuntimeContext, owner, repo string, issueID int) ([]map[string]interface{}, error) { + return fetchListItems(ctx, fmt.Sprintf("/v1/%s/%s/issues/%d/journals", owner, repo, issueID), url.Values{}, 50, 0) +} + +func firstReviewActor(item map[string]interface{}) string { + for _, key := range []string{"reviewer", "user", "author", "creator"} { + if value, ok := item[key]; ok { + if actor := apiAuthor(value); actor != "" { + return actor + } + } + } + return "" +} + +func summarizeFormalReviewStatus(reviews []normalizedPRReview) string { + if len(reviews) == 0 { + return "unreviewed" + } + hasApproved := false + hasCommon := false + for _, review := range reviews { + switch strings.ToLower(strings.TrimSpace(review.Status)) { + case "rejected", "reject", "changes_requested", "request_changes": + return "rejected" + case "approved", "approve": + hasApproved = true + default: + hasCommon = true + } + } + if hasApproved { + return "approved" + } + if hasCommon { + return "common" + } + return "reviewed" +} + +func uniqueReviewerNames(reviews []normalizedPRReview) []string { + set := map[string]string{} + for _, review := range reviews { + key := normalizeActorID(review.Actor) + if key != "" { + set[key] = review.Actor + } + } + keys := make([]string, 0, len(set)) + for key := range set { + keys = append(keys, key) + } + sort.Strings(keys) + out := make([]string, 0, len(keys)) + for _, key := range keys { + out = append(out, set[key]) + } + return out +} + +func reviewerSet(reviews []normalizedPRReview) map[string]bool { + set := map[string]bool{} + for _, review := range reviews { + if key := normalizeActorID(review.Actor); key != "" { + set[key] = true + } + } + return set +} + +func classifyJournalActor(item map[string]interface{}, author string, reviewers map[string]bool) string { + category := strings.ToLower(strings.TrimSpace(firstPRString(item, "operate_category", "category", "type", "event"))) + content := strings.TrimSpace(firstPRString(item, "notes", "note", "body", "content", "operate_content")) + actor := firstReviewActor(item) + if isSystemJournalEvent(category, content, actor) { + return actorSystem + } + if actor == "" { + return actorUnknown + } + if isBotActor(actor) { + return actorBot + } + actorID := normalizeActorID(actor) + if actorID != "" && actorID == normalizeActorID(author) { + return actorSubmitter + } + if actorID != "" && reviewers[actorID] { + return actorReviewer + } + return actorParticipant +} + +func isSystemJournalEvent(category, content, actor string) bool { + if strings.TrimSpace(content) == "" { + return true + } + switch category { + case "status", "state", "system", "relation", "assignee", "label", "milestone": + return true + } + if strings.TrimSpace(actor) == "" && category != "" { + return true + } + return false +} + +func isBotActor(actor string) bool { + actor = strings.ToLower(strings.TrimSpace(actor)) + return strings.Contains(actor, "bot") || strings.Contains(actor, "机器人") || strings.Contains(actor, "automation") +} + +func normalizeActorID(actor string) string { + return strings.ToLower(strings.TrimSpace(actor)) +} diff --git a/shortcuts/workflow/pr_summary.go b/shortcuts/workflow/pr_summary.go index 398bf1e..dc925fe 100644 --- a/shortcuts/workflow/pr_summary.go +++ b/shortcuts/workflow/pr_summary.go @@ -32,11 +32,14 @@ const ( type PRSummaryInput struct { Repository string `json:"repository"` Number int `json:"number"` + IssueID int `json:"issue_id,omitempty"` Title string `json:"title"` Author string `json:"author"` State string `json:"state"` BaseBranch string `json:"base_branch"` HeadBranch string `json:"head_branch"` + CreatedAt time.Time `json:"created_at,omitempty"` + UpdatedAt time.Time `json:"updated_at,omitempty"` Body string `json:"body,omitempty"` ChangedFiles []PRChangedFile `json:"changed_files"` Commits []PRCommit `json:"commits"` @@ -75,6 +78,7 @@ type PRSummaryResult struct { CommitCount int `json:"commit_count"` ChangeType string `json:"change_type"` RiskLevel string `json:"risk_level"` + RiskReasons []string `json:"risk_reasons,omitempty"` ReviewFocus []string `json:"review_focus"` TestSuggestions []string `json:"test_suggestions"` MergeChecklist []string `json:"merge_checklist"` @@ -208,6 +212,7 @@ func AnalyzePRSummary(input PRSummaryInput, lang string) PRSummaryResult { CommitCount: len(input.Commits), ChangeType: changeType, RiskLevel: riskLevel, + RiskReasons: riskReasons, ReviewFocus: reviewFocus, TestSuggestions: testSuggestions, MergeChecklist: mergeChecklist, diff --git a/shortcuts/workflow/pr_summary_test.go b/shortcuts/workflow/pr_summary_test.go index 29983a9..4a8e6f7 100644 --- a/shortcuts/workflow/pr_summary_test.go +++ b/shortcuts/workflow/pr_summary_test.go @@ -82,6 +82,9 @@ func TestAnalyzePRSummaryAuthTokenCriticalRisk(t *testing.T) { if result.RiskLevel != PRRiskCritical { t.Fatalf("RiskLevel = %q, want %q", result.RiskLevel, PRRiskCritical) } + if len(result.RiskReasons) != 1 || result.RiskReasons[0] != "security-sensitive keyword" { + t.Fatalf("RiskReasons = %v, want security-sensitive keyword", result.RiskReasons) + } } func TestAnalyzePRSummaryMixedFiles(t *testing.T) { diff --git a/shortcuts/workflow/render.go b/shortcuts/workflow/render.go index 366c2e2..32bd9ea 100644 --- a/shortcuts/workflow/render.go +++ b/shortcuts/workflow/render.go @@ -164,10 +164,22 @@ func writeRepoReportTable(w io.Writer, result RepoReportResult, lang string) err if len(result.Recommendations) > 0 { topRecommendation = truncateTableText(result.Recommendations[0], 96) } - if _, err := fmt.Fprintln(tw, "REPOSITORY\tREPORT_SCORE\tRISK\tHEALTH_SCORE\tISSUES\tHIGH_RISK_ISSUES\tPRS\tHIGH_RISK_PRS\tTOP_RECOMMENDATION"); err != nil { + openPRs, mergedPRs, closedPRs := "N/A", "N/A", "N/A" + if result.PRLifecycle != nil { + openPRs = fmt.Sprintf("%d", result.PRLifecycle.Open) + mergedPRs = fmt.Sprintf("%d", result.PRLifecycle.Merged) + closedPRs = fmt.Sprintf("%d", result.PRLifecycle.ClosedOrRejected) + } + reviewedPRs, unreviewedPRs, needsReReviewPRs := "N/A", "N/A", "N/A" + if result.PRReviewAudit != nil { + reviewedPRs = fmt.Sprintf("%d", result.PRReviewAudit.Reviewed) + unreviewedPRs = fmt.Sprintf("%d", result.PRReviewAudit.Unreviewed) + needsReReviewPRs = fmt.Sprintf("%d", result.PRReviewAudit.NeedsReReview) + } + if _, err := fmt.Fprintln(tw, "REPOSITORY\tREPORT_SCORE\tRISK\tHEALTH_SCORE\tISSUES_ANALYZED\tHIGH_RISK_ISSUES\tPRS_ANALYZED\tHIGH_RISK_PRS\tOPEN_PRS\tMERGED_PRS\tCLOSED_PRS\tREVIEWED_PRS\tUNREVIEWED_PRS\tNEEDS_RE_REVIEW\tTOP_RECOMMENDATION"); err != nil { return err } - if _, err := fmt.Fprintf(tw, "%s\t%d\t%s\t%s\t%d\t%d\t%d\t%d\t%s\n", + if _, err := fmt.Fprintf(tw, "%s\t%d\t%s\t%s\t%d\t%d\t%d\t%d\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n", result.Repository, result.ReportScore, result.RiskLevel, @@ -176,6 +188,12 @@ func writeRepoReportTable(w io.Writer, result RepoReportResult, lang string) err result.IssueSummary.HighRisk, result.PRSummary.Total, result.PRSummary.HighRisk, + openPRs, + mergedPRs, + closedPRs, + reviewedPRs, + unreviewedPRs, + needsReReviewPRs, topRecommendation, ); err != nil { return err @@ -269,6 +287,36 @@ func writeRepoReportMarkdown(w io.Writer, result RepoReportResult, lang string) } writeCountMapMarkdown(w, "By type", result.PRSummary.ByType) writeCountMapMarkdown(w, "By risk", result.PRSummary.ByRisk) + writeCountMapMarkdown(w, "Risk rule sources", result.PRSummary.RiskSources) + if result.PRLifecycle != nil { + if _, err := fmt.Fprintf(w, "- Lifecycle totals: open `%d`, merged `%d`, closed/rejected `%d`, all states `%d`\n", + result.PRLifecycle.Open, + result.PRLifecycle.Merged, + result.PRLifecycle.ClosedOrRejected, + result.PRLifecycle.Total, + ); err != nil { + return err + } + } + if result.PRReviewAudit != nil { + if _, err := fmt.Fprintf(w, "- Review audit: audited `%d`, reviewed `%d`, unreviewed `%d`, needs re-review `%d`, formal reviews `%d`\n", + result.PRReviewAudit.Audited, + result.PRReviewAudit.Reviewed, + result.PRReviewAudit.Unreviewed, + result.PRReviewAudit.NeedsReReview, + result.PRReviewAudit.FormalReviews, + ); err != nil { + return err + } + if _, err := fmt.Fprintf(w, "- Review actor attribution: reviewer comments `%d`, submitter comments `%d`, participant comments `%d`, system events `%d`\n", + result.PRReviewAudit.ReviewerComments, + result.PRReviewAudit.SubmitterComments, + result.PRReviewAudit.ParticipantComments, + result.PRReviewAudit.SystemEvents, + ); err != nil { + return err + } + } if len(result.PRSummary.ReviewFocus) > 0 { if _, err := fmt.Fprintln(w, "- Review focus:"); err != nil { return err diff --git a/shortcuts/workflow/repo_report.go b/shortcuts/workflow/repo_report.go index 4043be6..f7a7fca 100644 --- a/shortcuts/workflow/repo_report.go +++ b/shortcuts/workflow/repo_report.go @@ -12,24 +12,76 @@ import ( ) type RepoReportInput struct { - Repository string `json:"repository"` - Health *HealthInput `json:"health,omitempty"` - Issues []IssueInput `json:"issues,omitempty"` - PullRequests []PRSummaryInput `json:"pull_requests,omitempty"` - Source string `json:"source"` + Repository string `json:"repository"` + Health *HealthInput `json:"health,omitempty"` + Issues []IssueInput `json:"issues,omitempty"` + PullRequests []PRSummaryInput `json:"pull_requests,omitempty"` + PRLifecycle *RepoPRLifecycle `json:"pr_lifecycle,omitempty"` + PRReviewAudit *RepoPRReviewAudit `json:"pr_review_audit,omitempty"` + Source string `json:"source"` } type RepoReportResult struct { - Repository string `json:"repository"` - Health *HealthResult `json:"health,omitempty"` - IssueSummary RepoIssueSummary `json:"issue_summary"` - PRSummary RepoPRSummary `json:"pr_summary"` - Recommendations []string `json:"recommendations"` - RiskLevel string `json:"risk_level"` - ReportScore int `json:"report_score"` - Sections []string `json:"sections"` - Reasoning []string `json:"reasoning"` - Source string `json:"source"` + Repository string `json:"repository"` + Health *HealthResult `json:"health,omitempty"` + IssueSummary RepoIssueSummary `json:"issue_summary"` + PRSummary RepoPRSummary `json:"pr_summary"` + PRLifecycle *RepoPRLifecycle `json:"pr_lifecycle,omitempty"` + PRReviewAudit *RepoPRReviewAudit `json:"pr_review_audit,omitempty"` + Recommendations []string `json:"recommendations"` + RiskLevel string `json:"risk_level"` + ReportScore int `json:"report_score"` + Sections []string `json:"sections"` + Reasoning []string `json:"reasoning"` + Source string `json:"source"` +} + +type RepoPRLifecycle struct { + Open int `json:"open"` + Merged int `json:"merged"` + ClosedOrRejected int `json:"closed_or_rejected"` + Total int `json:"total"` + Source string `json:"source"` +} + +type RepoPRReviewAudit struct { + Audited int `json:"audited"` + Reviewed int `json:"reviewed"` + Unreviewed int `json:"unreviewed"` + NeedsReReview int `json:"needs_re_review"` + FormalReviews int `json:"formal_reviews"` + ReviewerComments int `json:"reviewer_comments"` + SubmitterComments int `json:"submitter_comments"` + ParticipantComments int `json:"participant_comments"` + BotEvents int `json:"bot_events"` + SystemEvents int `json:"system_events"` + UnknownActorEvents int `json:"unknown_actor_events"` + Errors int `json:"errors"` + Source string `json:"source"` + PullRequests []PRReviewAudit `json:"pull_requests,omitempty"` +} + +type PRReviewAudit struct { + Number int `json:"number"` + Author string `json:"author,omitempty"` + IssueID int `json:"issue_id,omitempty"` + Reviewed bool `json:"reviewed"` + NeedsReReview bool `json:"needs_re_review"` + ReviewStandard string `json:"review_standard"` + FormalReviewStatus string `json:"formal_review_status"` + FormalReviewCount int `json:"formal_review_count"` + ReviewerComments int `json:"reviewer_comments"` + SubmitterComments int `json:"submitter_comments"` + ParticipantComments int `json:"participant_comments"` + BotEvents int `json:"bot_events"` + SystemEvents int `json:"system_events"` + UnknownActorEvents int `json:"unknown_actor_events"` + Reviewers []string `json:"reviewers,omitempty"` + LatestReviewerAt string `json:"latest_reviewer_at,omitempty"` + LatestSubmitterAt string `json:"latest_submitter_at,omitempty"` + LatestCommitAt string `json:"latest_commit_at,omitempty"` + LatestPRUpdateAt string `json:"latest_pr_update_at,omitempty"` + Notes []string `json:"notes,omitempty"` } type RepoIssueSummary struct { @@ -44,6 +96,7 @@ type RepoPRSummary struct { Total int `json:"total"` ByType map[string]int `json:"by_type"` ByRisk map[string]int `json:"by_risk"` + RiskSources map[string]int `json:"risk_sources,omitempty"` HighRisk int `json:"high_risk"` ReviewFocus []string `json:"review_focus"` } @@ -54,11 +107,14 @@ func newRepoReportShortcut() *common.Shortcut { Description: "Generate a read-only repository workflow report", Flags: []common.Flag{ {Name: "from", Usage: "Read repository report input from a JSON file"}, - {Name: "issue-limit", Usage: "Maximum issues to fetch and analyze", Default: "20"}, - {Name: "pr-limit", Usage: "Maximum pull requests to fetch and summarize", Default: "10"}, + {Name: "issue-limit", Usage: "Maximum issues to fetch and analyze; 0 analyzes all open issues", Default: "0"}, + {Name: "pr-limit", Usage: "Maximum pull requests to fetch and summarize; 0 analyzes all open pull requests", Default: "0"}, {Name: "stale-days", Usage: "Days before an issue or PR is considered stale", Default: "30"}, {Name: "include-issues", Usage: "Include issue triage summary", Bool: true, Default: "true"}, {Name: "include-prs", Usage: "Include pull request summary", Bool: true, Default: "true"}, + {Name: "include-pr-lifecycle", Usage: "Include open/merged/closed PR totals", Bool: true, Default: "true"}, + {Name: "include-pr-review-audit", Usage: "Read formal reviews and PR conversation journals for actor attribution", Bool: true, Default: "false"}, + {Name: "pr-review-audit-limit", Usage: "Maximum analyzed PRs to review-audit; 0 audits every analyzed PR", Default: "0"}, {Name: "include-health", Usage: "Include repository health summary", Bool: true, Default: "true"}, {Name: "lang", Usage: "Output language: en or zh-CN", Default: langEN}, }, @@ -104,11 +160,11 @@ func collectRepoReportInput(ctx *common.RuntimeContext) (RepoReportInput, []Scor return input, nil, nil } - issueLimit, err := parseIntArg(ctx.Arg("issue-limit"), 20, "issue-limit") + issueLimit, err := parseIntArg(ctx.Arg("issue-limit"), 0, "issue-limit") if err != nil { return RepoReportInput{}, nil, err } - prLimit, err := parseIntArg(ctx.Arg("pr-limit"), 10, "pr-limit") + prLimit, err := parseIntArg(ctx.Arg("pr-limit"), 0, "pr-limit") if err != nil { return RepoReportInput{}, nil, err } @@ -116,14 +172,21 @@ func collectRepoReportInput(ctx *common.RuntimeContext) (RepoReportInput, []Scor if err != nil { return RepoReportInput{}, nil, err } + prReviewAuditLimit, err := parseIntArg(ctx.Arg("pr-review-audit-limit"), 0, "pr-review-audit-limit") + if err != nil { + return RepoReportInput{}, nil, err + } return FetchRepoReportInput(ctx, RepoReportFetchOptions{ - IssueLimit: issueLimit, - PRLimit: prLimit, - StaleDays: staleDays, - IncludeIssues: parseBoolArg(ctx.Arg("include-issues")), - IncludePRs: parseBoolArg(ctx.Arg("include-prs")), - IncludeHealth: parseBoolArg(ctx.Arg("include-health")), + IssueLimit: issueLimit, + PRLimit: prLimit, + StaleDays: staleDays, + PRReviewAuditLimit: prReviewAuditLimit, + IncludeIssues: parseBoolArg(ctx.Arg("include-issues")), + IncludePRs: parseBoolArg(ctx.Arg("include-prs")), + IncludePRLifecycle: parseBoolArg(ctx.Arg("include-pr-lifecycle")), + IncludePRReviewAudit: parseBoolArg(ctx.Arg("include-pr-review-audit")), + IncludeHealth: parseBoolArg(ctx.Arg("include-health")), }) } @@ -198,6 +261,8 @@ func AnalyzeRepoReport(input RepoReportInput, lang string) RepoReportResult { Health: healthResult, IssueSummary: issueSummary, PRSummary: prSummary, + PRLifecycle: input.PRLifecycle, + PRReviewAudit: input.PRReviewAudit, Recommendations: recommendations, RiskLevel: risk, ReportScore: reportScore, @@ -231,8 +296,9 @@ func summarizeRepoIssues(issues []IssueInput, lang string) (RepoIssueSummary, [] func summarizeRepoPRs(inputs []PRSummaryInput, lang string) (RepoPRSummary, []PRSummaryResult) { summary := RepoPRSummary{ - ByType: map[string]int{}, - ByRisk: map[string]int{}, + ByType: map[string]int{}, + ByRisk: map[string]int{}, + RiskSources: map[string]int{}, } results := make([]PRSummaryResult, 0, len(inputs)) focus := []string{} @@ -244,6 +310,9 @@ func summarizeRepoPRs(inputs []PRSummaryInput, lang string) (RepoPRSummary, []PR summary.ByRisk[result.RiskLevel]++ if result.RiskLevel == PRRiskHigh || result.RiskLevel == PRRiskCritical { summary.HighRisk++ + for _, reason := range result.RiskReasons { + summary.RiskSources[reason]++ + } } focus = append(focus, result.ReviewFocus...) } diff --git a/shortcuts/workflow/repo_report_fetch.go b/shortcuts/workflow/repo_report_fetch.go index 9610a71..c7d1a3b 100644 --- a/shortcuts/workflow/repo_report_fetch.go +++ b/shortcuts/workflow/repo_report_fetch.go @@ -2,21 +2,23 @@ package workflow import ( "fmt" - "net/url" "strings" "github.com/gitlink-org/gitlink-cli/shortcuts/common" ) type RepoReportFetchOptions struct { - Owner string - Repo string - IssueLimit int - PRLimit int - StaleDays int - IncludeIssues bool - IncludePRs bool - IncludeHealth bool + Owner string + Repo string + IssueLimit int + PRLimit int + StaleDays int + PRReviewAuditLimit int + IncludeIssues bool + IncludePRs bool + IncludePRLifecycle bool + IncludePRReviewAudit bool + IncludeHealth bool } func FetchRepoReportInput(ctx *common.RuntimeContext, opts RepoReportFetchOptions) (RepoReportInput, []ScoringNote, error) { @@ -24,12 +26,6 @@ func FetchRepoReportInput(ctx *common.RuntimeContext, opts RepoReportFetchOption if err != nil { return RepoReportInput{}, nil, fmt.Errorf("workflow +repo-report remote mode requires --owner and --repo or a Git remote: %w", err) } - if opts.IssueLimit <= 0 { - opts.IssueLimit = 20 - } - if opts.PRLimit <= 0 { - opts.PRLimit = 10 - } if opts.StaleDays <= 0 { opts.StaleDays = 30 } @@ -60,13 +56,7 @@ func FetchRepoReportInput(ctx *common.RuntimeContext, opts RepoReportFetchOption } if opts.IncludeIssues { - issues, err := FetchIssuesForTriage(ctx, TriageFetchOptions{ - Owner: owner, - Repo: repo, - State: "open", - Limit: opts.IssueLimit, - Page: 1, - }) + issues, err := fetchIssueListForReport(ctx, owner, repo, opts.IssueLimit) if err != nil { notes = append(notes, ScoringNote{Metric: "repo_report_issues", Note: fmt.Sprintf("issue fetch failed: %v", err)}) } else { @@ -82,6 +72,16 @@ func FetchRepoReportInput(ctx *common.RuntimeContext, opts RepoReportFetchOption } else { input.PullRequests = prs successes++ + if opts.IncludePRLifecycle { + lifecycle, lifecycleNotes := fetchPRLifecycle(ctx, owner, repo) + input.PRLifecycle = lifecycle + notes = append(notes, lifecycleNotes...) + } + if opts.IncludePRReviewAudit { + audit, auditNotes := fetchPRReviewAudit(ctx, owner, repo, prs, opts.PRReviewAuditLimit) + input.PRReviewAudit = audit + notes = append(notes, auditNotes...) + } if len(prs) > 0 { notes = append(notes, ScoringNote{ Metric: "repo_report_prs", @@ -100,26 +100,65 @@ func FetchRepoReportInput(ctx *common.RuntimeContext, opts RepoReportFetchOption return input, uniqueScoringNotes(notes), nil } -func fetchPRListForReport(ctx *common.RuntimeContext, owner, repo string, limit int) ([]PRSummaryInput, error) { - if limit <= 0 { - limit = 10 +func fetchPRLifecycle(ctx *common.RuntimeContext, owner, repo string) (*RepoPRLifecycle, []ScoringNote) { + states := []struct { + name string + value *int + }{ + {name: "open"}, + {name: "merged"}, + {name: "closed"}, } - query := url.Values{} - query.Set("state", "open") - query.Set("page", "1") - query.Set("limit", fmt.Sprintf("%d", limit)) + lifecycle := &RepoPRLifecycle{Source: "remote-read-only-fetch:list-totals"} + states[0].value = &lifecycle.Open + states[1].value = &lifecycle.Merged + states[2].value = &lifecycle.ClosedOrRejected + notes := []ScoringNote{} + successes := 0 + for _, state := range states { + total, err := fetchPRStateTotal(ctx, owner, repo, state.name) + if err != nil { + notes = append(notes, ScoringNote{ + Metric: "repo_report_pr_lifecycle", + Note: fmt.Sprintf("%s PR total unavailable: %v", state.name, err), + }) + continue + } + *state.value = total + successes++ + } + if successes == 0 { + return nil, notes + } + lifecycle.Total = lifecycle.Open + lifecycle.Merged + lifecycle.ClosedOrRejected + return lifecycle, notes +} +func fetchPRStateTotal(ctx *common.RuntimeContext, owner, repo, state string) (int, error) { + query := pullListQuery(state) + query.Set("page", "1") + query.Set("limit", "1") env, err := ctx.CallAPIWithQuery("GET", workflowRepoPath(owner, repo)+"/pulls", query) + if err != nil { + return 0, err + } + if env.Meta != nil && env.Meta.TotalCount > 0 { + return env.Meta.TotalCount, nil + } + if total := apiListTotal(env.Data); total > 0 { + return total, nil + } + return len(apiList(env.Data)), nil +} + +func fetchPRListForReport(ctx *common.RuntimeContext, owner, repo string, limit int) ([]PRSummaryInput, error) { + pageSize := reportPageSize(limit) + items, err := fetchListItems(ctx, workflowRepoPath(owner, repo)+"/pulls", pullListQuery("open"), pageSize, limit) if err != nil { return nil, err } - items := apiList(env.Data) inputs := make([]PRSummaryInput, 0, len(items)) - for _, raw := range items { - item, ok := raw.(map[string]interface{}) - if !ok { - continue - } + for _, item := range items { input, ok := normalizePRSummaryItem(item) if !ok { continue @@ -130,9 +169,37 @@ func fetchPRListForReport(ctx *common.RuntimeContext, owner, repo string, limit input.State = "open" } inputs = append(inputs, input) - if len(inputs) >= limit { + if limit > 0 && len(inputs) >= limit { break } } return inputs, nil } + +func fetchIssueListForReport(ctx *common.RuntimeContext, owner, repo string, limit int) ([]IssueInput, error) { + pageSize := reportPageSize(limit) + items, err := fetchListItems(ctx, workflowRepoPath(owner, repo)+"/issues", issueListQuery("open"), pageSize, limit) + if err != nil { + return nil, err + } + issues := make([]IssueInput, 0, len(items)) + for _, item := range items { + issue, ok := normalizeIssueItem(item) + if !ok { + continue + } + issues = append(issues, issue) + if limit > 0 && len(issues) >= limit { + break + } + } + return issues, nil +} + +func reportPageSize(limit int) int { + const maxPageSize = 50 + if limit > 0 && limit < maxPageSize { + return limit + } + return maxPageSize +} diff --git a/shortcuts/workflow/repo_report_fetch_test.go b/shortcuts/workflow/repo_report_fetch_test.go index 37e9838..c8d1969 100644 --- a/shortcuts/workflow/repo_report_fetch_test.go +++ b/shortcuts/workflow/repo_report_fetch_test.go @@ -1,6 +1,7 @@ package workflow import ( + "fmt" "net/http" "net/http/httptest" "strings" @@ -173,6 +174,210 @@ func TestFetchRepoReportInputPRListMetadata(t *testing.T) { } } +func TestFetchRepoReportInputPaginatesAllOpenItemsByDefault(t *testing.T) { + prPages := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/v1/owner/repo/issues.json": + if got := r.URL.Query().Get("category"); got != "opened" { + t.Fatalf("issue category = %q, want opened", got) + } + writeWorkflowJSON(t, w, map[string]interface{}{ + "total_count": 2, + "issues": []map[string]interface{}{ + {"id": 1, "number": 1, "title": "First open issue"}, + {"id": 2, "number": 2, "title": "Second open issue"}, + }, + }) + case "/v1/owner/repo/pulls.json": + if got := r.URL.Query().Get("status"); got != "0" { + t.Fatalf("PR status = %q, want 0", got) + } + page := mustParseInt(r.URL.Query().Get("page"), 1) + prPages++ + start := (page - 1) * 50 + end := start + 50 + if end > 120 { + end = 120 + } + pulls := make([]map[string]interface{}, 0, end-start) + for index := start + 1; index <= end; index++ { + pulls = append(pulls, map[string]interface{}{ + "id": 1000 + index, + "index": index, + "title": fmt.Sprintf("PR %d", index), + }) + } + writeWorkflowJSON(t, w, map[string]interface{}{ + "total_count": 120, + "pulls": pulls, + }) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + })) + defer server.Close() + + input, _, err := FetchRepoReportInput(workflowTestContext(server), RepoReportFetchOptions{ + IncludeHealth: false, + IncludeIssues: true, + IncludePRs: true, + }) + if err != nil { + t.Fatalf("FetchRepoReportInput returned error: %v", err) + } + if len(input.Issues) != 2 { + t.Fatalf("len(Issues) = %d, want 2", len(input.Issues)) + } + if len(input.PullRequests) != 120 { + t.Fatalf("len(PullRequests) = %d, want 120", len(input.PullRequests)) + } + if prPages != 3 { + t.Fatalf("PR pages = %d, want 3", prPages) + } + if input.PullRequests[119].Number != 120 { + t.Fatalf("last PR number = %d, want 120", input.PullRequests[119].Number) + } +} + +func TestFetchRepoReportInputIncludesPRLifecycleTotals(t *testing.T) { + openCalls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" || r.URL.Path != "/v1/owner/repo/pulls.json" { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + if got := r.URL.Query().Get("limit"); got != "1" { + t.Fatalf("PR limit = %q, want 1", got) + } + switch r.URL.Query().Get("status") { + case "0": + openCalls++ + writeWorkflowJSON(t, w, map[string]interface{}{ + "total_count": 12, + "pulls": []map[string]interface{}{ + {"id": 1001, "index": 1, "title": "Open PR"}, + }, + }) + case "1": + writeWorkflowJSON(t, w, map[string]interface{}{ + "total_count": 5, + "pulls": []map[string]interface{}{}, + }) + case "2": + writeWorkflowJSON(t, w, map[string]interface{}{ + "total_count": 7, + "pulls": []map[string]interface{}{}, + }) + default: + t.Fatalf("unexpected PR status query: %q", r.URL.Query().Get("status")) + } + })) + defer server.Close() + + input, notes, err := FetchRepoReportInput(workflowTestContext(server), RepoReportFetchOptions{ + PRLimit: 1, + IncludeHealth: false, + IncludeIssues: false, + IncludePRs: true, + IncludePRLifecycle: true, + }) + if err != nil { + t.Fatalf("FetchRepoReportInput returned error: %v", err) + } + if len(input.PullRequests) != 1 { + t.Fatalf("len(PullRequests) = %d, want 1", len(input.PullRequests)) + } + if input.PRLifecycle == nil { + t.Fatal("PRLifecycle is nil, want totals") + } + if input.PRLifecycle.Open != 12 || input.PRLifecycle.Merged != 5 || input.PRLifecycle.ClosedOrRejected != 7 || input.PRLifecycle.Total != 24 { + t.Fatalf("PRLifecycle = %+v, want open=12 merged=5 closed=7 total=24", input.PRLifecycle) + } + if openCalls != 2 { + t.Fatalf("open status calls = %d, want 2 for list fetch plus lifecycle total", openCalls) + } + if !hasNote(notes, "repo_report_prs") { + t.Fatalf("notes = %+v, want repo_report_prs note", notes) + } +} + +func TestFetchRepoReportInputIncludesPRReviewAudit(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/pulls.json": + writeWorkflowJSON(t, w, map[string]interface{}{ + "total_count": 2, + "pulls": []map[string]interface{}{ + { + "id": 1001, + "index": 1, + "title": "feat: reviewed change", + "author": map[string]interface{}{"login": "alice"}, + "issue": map[string]interface{}{"id": 501}, + "updated_at": "2026-06-02T12:00:00Z", + }, + {"id": 1002, "index": 2, "title": "docs: unreviewed change", "author": map[string]interface{}{"login": "dana"}, "issue": map[string]interface{}{"id": 502}}, + }, + }) + case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/pulls/1/reviews.json": + writeWorkflowJSON(t, w, map[string]interface{}{"reviews": []map[string]interface{}{ + {"reviewer": map[string]interface{}{"login": "bob"}, "status": "approved", "content": "looks good", "created_at": "2026-06-01T10:00:00Z"}, + }}) + case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/pulls/2/reviews.json": + writeWorkflowJSON(t, w, map[string]interface{}{"reviews": []map[string]interface{}{}}) + case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/501/journals.json": + writeWorkflowJSON(t, w, map[string]interface{}{"journals": []map[string]interface{}{ + {"user": map[string]interface{}{"login": "alice"}, "notes": "I updated the branch", "operate_category": "comment", "created_at": "2026-06-02T11:00:00Z"}, + {"user": map[string]interface{}{"login": "bob"}, "notes": "Please keep this test", "operate_category": "comment", "created_at": "2026-06-01T09:30:00Z"}, + {"user": map[string]interface{}{"login": "carol"}, "notes": "I can reproduce this", "operate_category": "comment", "created_at": "2026-06-01T12:00:00Z"}, + {"user": map[string]interface{}{"login": "system"}, "operate_content": "status changed", "operate_category": "status"}, + }}) + case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/502/journals.json": + writeWorkflowJSON(t, w, map[string]interface{}{"journals": []map[string]interface{}{ + {"user": map[string]interface{}{"login": "dana"}, "notes": "Initial description", "operate_category": "comment"}, + }}) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + })) + defer server.Close() + + input, notes, err := FetchRepoReportInput(workflowTestContext(server), RepoReportFetchOptions{ + IncludeHealth: false, + IncludeIssues: false, + IncludePRs: true, + IncludePRReviewAudit: true, + }) + if err != nil { + t.Fatalf("FetchRepoReportInput returned error: %v", err) + } + if len(notes) != 1 || notes[0].Metric != "repo_report_prs" { + t.Fatalf("notes = %+v, want only list metadata note", notes) + } + audit := input.PRReviewAudit + if audit == nil { + t.Fatal("PRReviewAudit is nil") + } + if audit.Audited != 2 || audit.Reviewed != 1 || audit.Unreviewed != 1 || audit.NeedsReReview != 1 || audit.FormalReviews != 1 { + t.Fatalf("audit summary = %+v, want audited=2 reviewed=1 unreviewed=1 formal=1", audit) + } + if audit.SubmitterComments != 2 || audit.ReviewerComments != 1 || audit.ParticipantComments != 1 || audit.SystemEvents != 1 { + t.Fatalf("actor counts = submitter:%d reviewer:%d participant:%d system:%d", + audit.SubmitterComments, audit.ReviewerComments, audit.ParticipantComments, audit.SystemEvents) + } + first := audit.PullRequests[0] + if !first.Reviewed || first.ReviewStandard != reviewStandardFormal || first.FormalReviewStatus != "approved" { + t.Fatalf("first audit = %+v, want formal approved review", first) + } + if !first.NeedsReReview { + t.Fatalf("first audit = %+v, want needs_re_review after submitter update", first) + } + second := audit.PullRequests[1] + if second.Reviewed || second.ReviewStandard != reviewStandardUnreviewed { + t.Fatalf("second audit = %+v, want unreviewed despite submitter comment", second) + } +} + func hasNote(notes []ScoringNote, metric string) bool { for _, note := range notes { if note.Metric == metric { diff --git a/shortcuts/workflow/repo_report_test.go b/shortcuts/workflow/repo_report_test.go index 5b508cd..20400e9 100644 --- a/shortcuts/workflow/repo_report_test.go +++ b/shortcuts/workflow/repo_report_test.go @@ -76,6 +76,23 @@ func TestAnalyzeRepoReportPartialInput(t *testing.T) { } } +func TestAnalyzeRepoReportAggregatesPRRiskSources(t *testing.T) { + result := AnalyzeRepoReport(RepoReportInput{ + Repository: "owner/repo", + PullRequests: []PRSummaryInput{{ + Number: 1, + Title: "fix: prevent secret token leak", + Source: "remote-read-only-fetch:list-metadata", + }}, + }, "en") + if result.PRSummary.HighRisk != 1 { + t.Fatalf("HighRisk = %d, want 1", result.PRSummary.HighRisk) + } + if result.PRSummary.RiskSources["security-sensitive keyword"] != 1 { + t.Fatalf("RiskSources = %v, want security-sensitive keyword=1", result.PRSummary.RiskSources) + } +} + func TestAnalyzeRepoReportChinese(t *testing.T) { result := AnalyzeRepoReport(sampleRepoReportInput(), "zh-CN") if len(result.Recommendations) == 0 { diff --git a/shortcuts/workflow/triage_fetch.go b/shortcuts/workflow/triage_fetch.go index a509cf8..af2b47a 100644 --- a/shortcuts/workflow/triage_fetch.go +++ b/shortcuts/workflow/triage_fetch.go @@ -2,7 +2,6 @@ package workflow import ( "fmt" - "net/url" "strings" "time" @@ -28,8 +27,7 @@ func FetchIssuesForTriage(ctx *common.RuntimeContext, opts TriageFetchOptions) ( state = "open" } - query := url.Values{} - query.Set("state", state) + query := issueListQuery(state) query.Set("limit", fmt.Sprintf("%d", limit)) query.Set("page", fmt.Sprintf("%d", page)) if len(opts.Labels) > 0 { diff --git a/shortcuts/workflow/triage_fetch_test.go b/shortcuts/workflow/triage_fetch_test.go index f7af88a..782ab3c 100644 --- a/shortcuts/workflow/triage_fetch_test.go +++ b/shortcuts/workflow/triage_fetch_test.go @@ -16,8 +16,11 @@ func TestFetchIssuesForTriageNormalizesAPIResponse(t *testing.T) { if r.Method != "GET" || r.URL.Path != "/v1/owner/repo/issues.json" { t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) } - if got := r.URL.Query().Get("state"); got != "open" { - t.Fatalf("state query = %q, want open", got) + if got := r.URL.Query().Get("category"); got != "opened" { + t.Fatalf("category query = %q, want opened", got) + } + if got := r.URL.Query().Get("state"); got != "" { + t.Fatalf("state query = %q, want empty", got) } if got := r.URL.Query().Get("limit"); got != "30" { t.Fatalf("limit query = %q, want 30", got)