diff --git a/shortcuts/health/api.go b/shortcuts/health/api.go index 7692e139..2d9f81b2 100644 --- a/shortcuts/health/api.go +++ b/shortcuts/health/api.go @@ -3,15 +3,20 @@ package health import ( "fmt" "net/url" + "os" "github.com/gitlink-org/gitlink-cli/shortcuts/common" ) +// v1RepoPath constructs the v1 API path for a repository. +// Issue operations use the v1 API (/v1/{owner}/{repo}) to match shortcuts/issue conventions, +// while PR operations use the v2 API via ctx.RepoPath() (/{owner}/{repo}/pulls) to match shortcuts/pr conventions. + func v1RepoPath(owner, repo string) string { return fmt.Sprintf("/v1/%s/%s", owner, repo) } -func fetchPRListPage(ctx *common.RuntimeContext, state string, page, limit int) ([]interface{}, map[string]interface{}) { +func fetchPRListPage(ctx *common.RuntimeContext, state string, page, limit int) ([]interface{}, error) { q := url.Values{} q.Set("page", fmt.Sprintf("%d", page)) q.Set("limit", fmt.Sprintf("%d", limit)) @@ -20,22 +25,64 @@ func fetchPRListPage(ctx *common.RuntimeContext, state string, page, limit int) } env, err := ctx.CallAPIWithQuery("GET", ctx.RepoPath()+"/pulls", q) if err != nil { - fmt.Printf(" CLI error: pr +list state=%s page=%d: %v\n", state, page, err) - return nil, nil + fmt.Fprintf(os.Stderr, " CLI error: pr +list state=%s page=%d: %v\n", state, page, err) + return nil, err } if !env.OK { - fmt.Printf(" API error: pr +list state=%s page=%d\n", state, page) - return nil, nil + err := fmt.Errorf("API error: pr +list state=%s page=%d", state, page) + fmt.Fprintf(os.Stderr, " %v\n", err) + return nil, err } data, ok := env.Data.(map[string]interface{}) if !ok { - return nil, nil + return nil, fmt.Errorf("unexpected response type for pr +list") } issues, _ := data["issues"].([]interface{}) - return issues, data + return issues, nil } -func fetchIssueListPage(ctx *common.RuntimeContext, owner, repo, state string, page, limit int) ([]interface{}, map[string]interface{}) { +// fetchPRDetail retrieves full PR detail to extract merged_at timestamp. +// The list API doesn't return merged_at; only the detail endpoint does. +func fetchPRDetail(ctx *common.RuntimeContext, prNumber int) (string, error) { + path := fmt.Sprintf("%s/pulls/%d", ctx.RepoPath(), prNumber) + env, err := ctx.CallAPI("GET", path, nil) + if err != nil { + return "", fmt.Errorf("PR detail fetch #%d: %w", prNumber, err) + } + if !env.OK { + return "", fmt.Errorf("PR detail API error #%d", prNumber) + } + data, ok := env.Data.(map[string]interface{}) + if !ok { + return "", nil + } + + // Try multiple possible locations for merged_at in the detail response + for _, candidate := range []string{ + "merged_at", + "mergedAt", + } { + // data.pull_request.merged_at + if pr, _ := data["pull_request"].(map[string]interface{}); pr != nil { + if v, _ := pr[candidate].(string); v != "" { + return v, nil + } + } + // data.issue.merged_at + if issue, _ := data["issue"].(map[string]interface{}); issue != nil { + if v, _ := issue[candidate].(string); v != "" { + return v, nil + } + } + // data.merged_at (flat) + if v, _ := data[candidate].(string); v != "" { + return v, nil + } + } + return "", nil +} + +func fetchIssueListPage(ctx *common.RuntimeContext, owner, repo, state string, page, limit int) ([]interface{}, error) { q := url.Values{} q.Set("page", fmt.Sprintf("%d", page)) q.Set("limit", fmt.Sprintf("%d", limit)) @@ -44,17 +91,18 @@ func fetchIssueListPage(ctx *common.RuntimeContext, owner, repo, state string, p } env, err := ctx.CallAPIWithQuery("GET", v1RepoPath(owner, repo)+"/issues", q) if err != nil { - fmt.Printf(" CLI error: issue +list state=%s page=%d: %v\n", state, page, err) - return nil, nil + fmt.Fprintf(os.Stderr, " CLI error: issue +list state=%s page=%d: %v\n", state, page, err) + return nil, err } if !env.OK { - fmt.Printf(" API error: issue +list state=%s page=%d\n", state, page) - return nil, nil + err := fmt.Errorf("API error: issue +list state=%s page=%d", state, page) + fmt.Fprintf(os.Stderr, " %v\n", err) + return nil, err } data, ok := env.Data.(map[string]interface{}) if !ok { - return nil, nil + return nil, fmt.Errorf("unexpected response type for issue +list") } issues, _ := data["issues"].([]interface{}) - return issues, data + return issues, nil } diff --git a/shortcuts/health/db.go b/shortcuts/health/db.go index e5c0ea7a..78ea7e8d 100644 --- a/shortcuts/health/db.go +++ b/shortcuts/health/db.go @@ -4,6 +4,7 @@ import ( "database/sql" _ "embed" "fmt" + "os" _ "modernc.org/sqlite" ) @@ -103,7 +104,9 @@ func savePullTags(db *sql.DB, pullID int, tagIDs []int) { return } for _, tagID := range tagIDs { - db.Exec("INSERT OR IGNORE INTO pull_tags (pull_id, tag_id) VALUES (?, ?)", pullID, tagID) + if _, err := db.Exec("INSERT OR IGNORE INTO pull_tags (pull_id, tag_id) VALUES (?, ?)", pullID, tagID); err != nil { + fmt.Fprintf(os.Stderr, " DB error: save pull_tags (pull=%d tag=%d): %v\n", pullID, tagID, err) + } } } @@ -112,7 +115,9 @@ func saveIssueTags(db *sql.DB, issueID int, tagIDs []int) { return } for _, tagID := range tagIDs { - db.Exec("INSERT OR IGNORE INTO issue_tags (issue_id, tag_id) VALUES (?, ?)", issueID, tagID) + if _, err := db.Exec("INSERT OR IGNORE INTO issue_tags (issue_id, tag_id) VALUES (?, ?)", issueID, tagID); err != nil { + fmt.Fprintf(os.Stderr, " DB error: save issue_tags (issue=%d tag=%d): %v\n", issueID, tagID, err) + } } } @@ -134,7 +139,22 @@ func extractTagNames(data map[string]interface{}, key string) []string { return names } -func savePull(db *sql.DB, repoID int, pr map[string]interface{}) { +// mergeTimeFromList tries to extract merged_at directly from the list API response +// (preferred, avoids an extra API call per PR). +func mergeTimeFromList(pr map[string]interface{}) string { + for _, key := range []string{"merged_at", "mergedAt", "pr_merge_time", "merge_time"} { + if v, _ := pr[key].(string); v != "" { + return v + } + // Some APIs return numeric timestamps + if v, ok := pr[key].(float64); ok && v > 0 { + return fmt.Sprintf("%.0f", v) + } + } + return "" +} + +func savePull(db *sql.DB, repoID int, pr map[string]interface{}, mergedAt string) { // id: pull_request_id preferred, fallback to id var prID float64 if v, ok := pr["pull_request_id"].(float64); ok && v > 0 { @@ -171,9 +191,19 @@ func savePull(db *sql.DB, repoID int, pr map[string]interface{}) { processorID = &pid } - db.Exec(`INSERT OR REPLACE INTO pulls (id, repo_id, number, creater_id, status, processor_id, create_time, close_time) + // Priority: explicit mergedAt arg > list response field > nil + var mergedAtVal interface{} = nil + if mergedAt != "" { + mergedAtVal = mergedAt + } else if ma := mergeTimeFromList(pr); ma != "" { + mergedAtVal = ma + } + + if _, err := db.Exec(`INSERT OR REPLACE INTO pulls (id, repo_id, number, creater_id, status, processor_id, create_time, merged_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, - int(prID), repoID, int(prNumber), createrID, status, processorID, createTime, nil) + int(prID), repoID, int(prNumber), createrID, status, processorID, createTime, mergedAtVal); err != nil { + fmt.Fprintf(os.Stderr, " DB error: save pull %d: %v\n", int(prID), err) + } // Save tags tagNames := extractTagNames(pr, "issue_tags") @@ -205,7 +235,7 @@ func saveIssue(db *sql.DB, repoID int, issue map[string]interface{}, issueNumber statusName := extractStatusName(issue) var status string var closeTime interface{} - if statusName == "关闭" { + if statusName == "关闭" || statusName == "Closed" { status = "close" if v, _ := issue["closed_on"].(string); v != "" { closeTime = v @@ -218,9 +248,11 @@ func saveIssue(db *sql.DB, repoID int, issue map[string]interface{}, issueNumber status = "open" } - db.Exec(`INSERT OR REPLACE INTO issues (id, repo_id, number, creater_id, processor_id, create_time, close_time, status) + if _, err := db.Exec(`INSERT OR REPLACE INTO issues (id, repo_id, number, creater_id, processor_id, create_time, close_time, status) VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, - int(issueID), repoID, issueNumber, createrID, processorID, createTime, closeTime, status) + int(issueID), repoID, issueNumber, createrID, processorID, createTime, closeTime, status); err != nil { + fmt.Fprintf(os.Stderr, " DB error: save issue %d: %v\n", int(issueID), err) + } // Save tags tagNames := extractTagNames(issue, "tags") diff --git a/shortcuts/health/health.go b/shortcuts/health/health.go index 516e6d79..04f26d13 100644 --- a/shortcuts/health/health.go +++ b/shortcuts/health/health.go @@ -113,7 +113,10 @@ func fetchPRs(ctx *common.RuntimeContext, db *sql.DB, repoID int, state string, return nil } fmt.Fprintf(os.Stderr, " PR list: state=%s, page=%d...\n", state, page) - prs, _ := fetchPRListPage(ctx, state, page, 20) + prs, err := fetchPRListPage(ctx, state, page, 20) + if err != nil { + return err + } if len(prs) == 0 { break } @@ -140,7 +143,28 @@ func fetchPRs(ctx *common.RuntimeContext, db *sql.DB, repoID int, state string, if dup { continue } - savePull(db, repoID, pr) + + // For merged PRs: try list response first, fall back to detail API + mergedAt := "" + if state == "merged" { + mergedAt = mergeTimeFromList(pr) + if mergedAt == "" { + if prNumFloat, ok := pr["pull_request_number"].(float64); ok && prNumFloat > 0 { + prNum := int(prNumFloat) + if err := limiter.Wait(egCtx); err != nil { + return nil + } + fmt.Fprintf(os.Stderr, " PR detail #%d...\n", prNum) + if ma, err := fetchPRDetail(ctx, prNum); err != nil { + fmt.Fprintf(os.Stderr, " PR detail #%d: %v\n", prNum, err) + } else { + mergedAt = ma + } + } + } + } + + savePull(db, repoID, pr, mergedAt) } if len(prs) < 20 { break @@ -160,7 +184,10 @@ func fetchIssues(ctx *common.RuntimeContext, db *sql.DB, repoID int, state strin return nil } fmt.Fprintf(os.Stderr, " Issue list: state=%s, page=%d...\n", state, page) - issues, _ := fetchIssueListPage(ctx, ctx.Owner, ctx.Repo, state, page, 20) + issues, err := fetchIssueListPage(ctx, ctx.Owner, ctx.Repo, state, page, 20) + if err != nil { + return err + } if len(issues) == 0 { break } diff --git a/shortcuts/health/schema.sql b/shortcuts/health/schema.sql index 941b2a5c..6535d187 100644 --- a/shortcuts/health/schema.sql +++ b/shortcuts/health/schema.sql @@ -39,12 +39,13 @@ CREATE TABLE IF NOT EXISTS pulls ( status TEXT CHECK(status IN ('merged', 'closed', 'open')), processor_id INTEGER, create_time TIMESTAMP, - close_time TIMESTAMP, + merged_at TIMESTAMP, FOREIGN KEY (repo_id) REFERENCES repos(id), FOREIGN KEY (creater_id) REFERENCES users(id), FOREIGN KEY (processor_id) REFERENCES users(id) ); +CREATE INDEX IF NOT EXISTS idx_pulls_merged_at ON pulls(merged_at); CREATE INDEX IF NOT EXISTS idx_pulls_repo_id ON pulls(repo_id); CREATE INDEX IF NOT EXISTS idx_pulls_status ON pulls(status); CREATE INDEX IF NOT EXISTS idx_pulls_create_time ON pulls(create_time); diff --git a/shortcuts/register_test.go b/shortcuts/register_test.go index e3f13513..e232fbe4 100644 --- a/shortcuts/register_test.go +++ b/shortcuts/register_test.go @@ -14,6 +14,7 @@ func TestRegisterAll(t *testing.T) { "repo", "issue", "label", "pr", "release", "branch", "org", "user", "search", "ci", "workflow", "compare", "member", "milestone", "pipeline", "webhook", + "health", } groupSet := map[string]bool{} diff --git a/skills/README.md b/skills/README.md index 3ca3cf9b..f7e10ea7 100644 --- a/skills/README.md +++ b/skills/README.md @@ -114,9 +114,9 @@ skills/ │ └── SKILL.md # PM 操作指南 ├── gitlink-health/ # 项目健康度分析 │ ├── SKILL.md # 健康度分析指南 -│ ├── collector/ -│ │ ├── fetcher.py # 数据采集脚本 -│ │ └── schema.sql # 建表 SQL +│ ├── data/ +│ │ ├── .gitignore # 忽略 *.db 文件 +│ │ └── .gitkeep # 占位文件 │ ├── references/ │ │ └── queries.md # SQL 查询参考 │ └── asset/ diff --git a/skills/gitlink-health/references/queries.md b/skills/gitlink-health/references/queries.md index 7bc83372..bda9d548 100644 --- a/skills/gitlink-health/references/queries.md +++ b/skills/gitlink-health/references/queries.md @@ -58,14 +58,14 @@ CREATE TABLE pulls ( status TEXT CHECK(status IN ('merged', 'closed', 'open')), processor_id INTEGER, -- 指派人 → users.id（nullable，来自 list 的 assign_user_login） create_time TIMESTAMP, -- 创建时间，ISO 格式 - close_time TIMESTAMP, -- 始终为 NULL（API 不提供） + merged_at TIMESTAMP, -- 合并时间（从 PR 详情 API 获取） FOREIGN KEY (repo_id) REFERENCES repos(id), FOREIGN KEY (creater_id) REFERENCES users(id), FOREIGN KEY (processor_id) REFERENCES users(id) ); ``` -API 字段映射：`status` ← `pull_request_status`（0=open, 1=merged, 2=closed），`creater_id` ← `author_login`。 +API 字段映射：`status` ← `pull_request_status`（0=open, 1=merged, 2=closed），`creater_id` ← `author_login`；`merged_at` 需要调用 PR 详情 API（`GET /:owner/:repo/pulls/:number`）获取。 ### tags