refactor(FN-1288): consolidate GitHub remote parsing in @fusion/core
- Replace dashboard calls to local getCurrentGitHubRepo helpers with getCurrentRepo from @fusion/core - Update engine scheduler PR-monitor startup paths to use shared core repo resolution - Remove duplicated remote parsing implementations from dashboard and engine packages - Mark gap analysis finding 6.3 as resolved after centralizing parsing logic
This commit is contained in:
@@ -204,13 +204,13 @@ Assessment:
|
|||||||
Cross-reference:
|
Cross-reference:
|
||||||
- Existing tasks **FN-1201** and **FN-1202** already target server-vs-client persistence boundaries.
|
- Existing tasks **FN-1201** and **FN-1202** already target server-vs-client persistence boundaries.
|
||||||
|
|
||||||
### Finding 6.3 — Duplication in GitHub remote parsing logic across packages (**Medium**)
|
### Finding 6.3 — GitHub remote parsing logic consolidation (**Resolved**)
|
||||||
|
|
||||||
- `packages/core/src/gh-cli.ts` implements `parseRepoFromRemote()` + gh execution wrappers.
|
- `packages/core/src/gh-cli.ts` is the canonical home for `parseRepoFromRemote()` and `getCurrentRepo()`.
|
||||||
- `packages/engine/src/github.ts` separately implements similar `parseGitHubRemote()` and `getCurrentGitHubRepo()` logic.
|
- Engine and dashboard consumers now import the shared `@fusion/core` helpers directly.
|
||||||
|
|
||||||
Impact:
|
Impact:
|
||||||
- Risk of drift in URL parsing behavior and edge-case handling between scheduler/engine and core CLI helpers.
|
- Eliminates drift risk in URL parsing behavior and keeps GitHub remote resolution logic centralized.
|
||||||
|
|
||||||
### Finding 6.4 — Layering is healthy: no core → engine/dashboard circular import leak (**Good / no gap**)
|
### Finding 6.4 — Layering is healthy: no core → engine/dashboard circular import leak (**Good / no gap**)
|
||||||
|
|
||||||
|
|||||||
@@ -2344,41 +2344,3 @@ export function parseGitHubBadgeUrl(url: string): { owner: string; repo: string
|
|||||||
return { owner: parsed.owner, repo: parsed.repo };
|
return { owner: parsed.owner, repo: parsed.repo };
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* Extract owner/repo from a GitHub remote URL or return null if not a GitHub remote.
|
|
||||||
* @deprecated Use parseRepoFromRemote from gh-cli.ts instead
|
|
||||||
*/
|
|
||||||
export function parseGitHubRemote(remoteUrl: string): { owner: string; repo: string } | null {
|
|
||||||
// Handle HTTPS: https://github.com/owner/repo.git or https://github.com/owner/repo
|
|
||||||
const httpsMatch = remoteUrl.match(/github\.com\/([^\/]+)\/([^\/\.]+)(?:\.git)?$/);
|
|
||||||
if (httpsMatch) {
|
|
||||||
return { owner: httpsMatch[1], repo: httpsMatch[2] };
|
|
||||||
}
|
|
||||||
|
|
||||||
// Handle SSH: git@github.com:owner/repo.git or git@github.com:owner/repo
|
|
||||||
const sshMatch = remoteUrl.match(/github\.com:([^\/]+)\/([^\/\.]+)(?:\.git)?$/);
|
|
||||||
if (sshMatch) {
|
|
||||||
return { owner: sshMatch[1], repo: sshMatch[2] };
|
|
||||||
}
|
|
||||||
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
|
||||||
* Get the current GitHub remote owner/repo from the git config.
|
|
||||||
* @deprecated Use getCurrentRepo from gh-cli.ts instead
|
|
||||||
*/
|
|
||||||
export function getCurrentGitHubRepo(cwd: string): { owner: string; repo: string } | null {
|
|
||||||
const { execFileSync } = require("node:child_process");
|
|
||||||
try {
|
|
||||||
const remoteUrl = execFileSync("git", ["remote", "get-url", "origin"], {
|
|
||||||
cwd,
|
|
||||||
encoding: "utf-8",
|
|
||||||
stdio: ["pipe", "pipe", "ignore"],
|
|
||||||
}).trim();
|
|
||||||
|
|
||||||
return parseGitHubRemote(remoteUrl);
|
|
||||||
} catch {
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -8,9 +8,9 @@ import { tmpdir } from "node:os";
|
|||||||
import * as nodeFs from "node:fs";
|
import * as nodeFs from "node:fs";
|
||||||
import * as nodeChildProcess from "node:child_process";
|
import * as nodeChildProcess from "node:child_process";
|
||||||
import type { TaskStore, Column, MergeResult, ScheduleType, ActivityEventType, ModelPreset, AutomationStep, MessageType, ParticipantType, MessageCreateInput } from "@fusion/core";
|
import type { TaskStore, Column, MergeResult, ScheduleType, ActivityEventType, ModelPreset, AutomationStep, MessageType, ParticipantType, MessageCreateInput } from "@fusion/core";
|
||||||
import { COLUMNS, VALID_TRANSITIONS, GLOBAL_SETTINGS_KEYS, type BatchStatusEntry, type BatchStatusResponse, type BatchStatusResult, type IssueInfo, type PrInfo, type Task, isGhAuthenticated, AUTOMATION_PRESETS, AutomationStore, validateBackupSchedule, validateBackupRetention, validateBackupDir, syncBackupAutomation, exportSettings, importSettings, validateImportData, MessageStore, MEMORY_FILE_PATH } from "@fusion/core";
|
import { COLUMNS, VALID_TRANSITIONS, GLOBAL_SETTINGS_KEYS, type BatchStatusEntry, type BatchStatusResponse, type BatchStatusResult, type IssueInfo, type PrInfo, type Task, getCurrentRepo, isGhAuthenticated, AUTOMATION_PRESETS, AutomationStore, validateBackupSchedule, validateBackupRetention, validateBackupDir, syncBackupAutomation, exportSettings, importSettings, validateImportData, MessageStore, MEMORY_FILE_PATH } from "@fusion/core";
|
||||||
import type { ServerOptions } from "./server.js";
|
import type { ServerOptions } from "./server.js";
|
||||||
import { GitHubClient, getCurrentGitHubRepo, parseBadgeUrl } from "./github.js";
|
import { GitHubClient, parseBadgeUrl } from "./github.js";
|
||||||
import { githubRateLimiter } from "./github-poll.js";
|
import { githubRateLimiter } from "./github-poll.js";
|
||||||
import { terminalSessionManager } from "./terminal.js";
|
import { terminalSessionManager } from "./terminal.js";
|
||||||
import { getTerminalService } from "./terminal-service.js";
|
import { getTerminalService } from "./terminal-service.js";
|
||||||
@@ -4233,7 +4233,7 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
|||||||
owner = o;
|
owner = o;
|
||||||
repo = r;
|
repo = r;
|
||||||
} else {
|
} else {
|
||||||
const gitRepo = getCurrentGitHubRepo(scopedStore.getRootDir());
|
const gitRepo = getCurrentRepo(scopedStore.getRootDir());
|
||||||
if (!gitRepo) {
|
if (!gitRepo) {
|
||||||
throw badRequest("Could not determine GitHub repository. Set GITHUB_REPOSITORY env var or configure git remote.");
|
throw badRequest("Could not determine GitHub repository. Set GITHUB_REPOSITORY env var or configure git remote.");
|
||||||
}
|
}
|
||||||
@@ -4520,7 +4520,7 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
|||||||
owner = o;
|
owner = o;
|
||||||
repo = r;
|
repo = r;
|
||||||
} else {
|
} else {
|
||||||
const gitRepo = getCurrentGitHubRepo(scopedStore.getRootDir());
|
const gitRepo = getCurrentRepo(scopedStore.getRootDir());
|
||||||
if (!gitRepo) {
|
if (!gitRepo) {
|
||||||
throw badRequest("Could not determine GitHub repository");
|
throw badRequest("Could not determine GitHub repository");
|
||||||
}
|
}
|
||||||
@@ -4644,7 +4644,7 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
|||||||
owner = o;
|
owner = o;
|
||||||
repo = r;
|
repo = r;
|
||||||
} else {
|
} else {
|
||||||
const gitRepo = getCurrentGitHubRepo(scopedStore.getRootDir());
|
const gitRepo = getCurrentRepo(scopedStore.getRootDir());
|
||||||
if (!gitRepo) {
|
if (!gitRepo) {
|
||||||
throw badRequest("Could not determine GitHub repository");
|
throw badRequest("Could not determine GitHub repository");
|
||||||
}
|
}
|
||||||
@@ -10980,7 +10980,7 @@ function getDefaultGitHubRepo(store: TaskStore): { owner: string; repo: string }
|
|||||||
}
|
}
|
||||||
|
|
||||||
const rootDir = typeof store.getRootDir === "function" ? store.getRootDir() : process.cwd();
|
const rootDir = typeof store.getRootDir === "function" ? store.getRootDir() : process.cwd();
|
||||||
return getCurrentGitHubRepo(rootDir);
|
return getCurrentRepo(rootDir);
|
||||||
}
|
}
|
||||||
|
|
||||||
function isBatchStatusStale(info: { lastCheckedAt?: string } | undefined, updatedAt?: string): boolean {
|
function isBatchStatusStale(info: { lastCheckedAt?: string } | undefined, updatedAt?: string): boolean {
|
||||||
@@ -11022,7 +11022,7 @@ async function refreshPrInBackground(store: TaskStore, taskId: string, currentPr
|
|||||||
owner = o;
|
owner = o;
|
||||||
repo = r;
|
repo = r;
|
||||||
} else {
|
} else {
|
||||||
const gitRepo = getCurrentGitHubRepo(store.getRootDir());
|
const gitRepo = getCurrentRepo(store.getRootDir());
|
||||||
if (!gitRepo) return; // Silent fail - can't determine repo
|
if (!gitRepo) return; // Silent fail - can't determine repo
|
||||||
owner = gitRepo.owner;
|
owner = gitRepo.owner;
|
||||||
repo = gitRepo.repo;
|
repo = gitRepo.repo;
|
||||||
@@ -11066,7 +11066,7 @@ async function refreshIssueInBackground(
|
|||||||
owner = o;
|
owner = o;
|
||||||
repo = r;
|
repo = r;
|
||||||
} else {
|
} else {
|
||||||
const gitRepo = getCurrentGitHubRepo(store.getRootDir());
|
const gitRepo = getCurrentRepo(store.getRootDir());
|
||||||
if (!gitRepo) return;
|
if (!gitRepo) return;
|
||||||
owner = gitRepo.owner;
|
owner = gitRepo.owner;
|
||||||
repo = gitRepo.repo;
|
repo = gitRepo.repo;
|
||||||
|
|||||||
@@ -13,7 +13,7 @@ import { getOrCreateProjectStore, evictAllProjectStores } from "./project-store-
|
|||||||
import { getTerminalService, type TerminalSession, STALE_SESSION_THRESHOLD_MS } from "./terminal-service.js";
|
import { getTerminalService, type TerminalSession, STALE_SESSION_THRESHOLD_MS } from "./terminal-service.js";
|
||||||
import { WebSocketServer, type WebSocket } from "ws";
|
import { WebSocketServer, type WebSocket } from "ws";
|
||||||
import { terminalSessionManager } from "./terminal.js";
|
import { terminalSessionManager } from "./terminal.js";
|
||||||
import { getCurrentGitHubRepo, parseBadgeUrl } from "./github.js";
|
import { parseBadgeUrl } from "./github.js";
|
||||||
import { WebSocketManager, type BadgeSnapshot } from "./websocket.js";
|
import { WebSocketManager, type BadgeSnapshot } from "./websocket.js";
|
||||||
import type { BadgePubSub } from "./badge-pubsub.js";
|
import type { BadgePubSub } from "./badge-pubsub.js";
|
||||||
import { createBadgePubSub, type BadgePubSubMessage } from "./badge-pubsub.js";
|
import { createBadgePubSub, type BadgePubSubMessage } from "./badge-pubsub.js";
|
||||||
|
|||||||
@@ -1,37 +0,0 @@
|
|||||||
import { execFileSync } from "node:child_process";
|
|
||||||
|
|
||||||
/**
|
|
||||||
* Extract owner/repo from a GitHub remote URL or return null if not a GitHub remote.
|
|
||||||
*/
|
|
||||||
export function parseGitHubRemote(remoteUrl: string): { owner: string; repo: string } | null {
|
|
||||||
// Handle HTTPS: https://github.com/owner/repo.git or https://github.com/owner/repo
|
|
||||||
const httpsMatch = remoteUrl.match(/github\.com\/([^\/]+)\/([^\/\.]+)(?:\.git)?$/);
|
|
||||||
if (httpsMatch) {
|
|
||||||
return { owner: httpsMatch[1], repo: httpsMatch[2] };
|
|
||||||
}
|
|
||||||
|
|
||||||
// Handle SSH: git@github.com:owner/repo.git or git@github.com:owner/repo
|
|
||||||
const sshMatch = remoteUrl.match(/github\.com:([^\/]+)\/([^\/\.]+)(?:\.git)?$/);
|
|
||||||
if (sshMatch) {
|
|
||||||
return { owner: sshMatch[1], repo: sshMatch[2] };
|
|
||||||
}
|
|
||||||
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
|
||||||
* Get the current GitHub remote owner/repo from the git config.
|
|
||||||
*/
|
|
||||||
export function getCurrentGitHubRepo(cwd: string): { owner: string; repo: string } | null {
|
|
||||||
try {
|
|
||||||
const remoteUrl = execFileSync("git", ["remote", "get-url", "origin"], {
|
|
||||||
cwd,
|
|
||||||
encoding: "utf-8",
|
|
||||||
stdio: ["pipe", "pipe", "ignore"],
|
|
||||||
}).trim();
|
|
||||||
|
|
||||||
return parseGitHubRemote(remoteUrl);
|
|
||||||
} catch {
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -1,4 +1,4 @@
|
|||||||
import { resolveDependencyOrder, type TaskStore, type Task, type MissionStore, type PrInfo } from "@fusion/core";
|
import { getCurrentRepo, resolveDependencyOrder, type TaskStore, type Task, type MissionStore, type PrInfo } from "@fusion/core";
|
||||||
import { existsSync } from "node:fs";
|
import { existsSync } from "node:fs";
|
||||||
import { readFile } from "node:fs/promises";
|
import { readFile } from "node:fs/promises";
|
||||||
import { join } from "node:path";
|
import { join } from "node:path";
|
||||||
@@ -6,7 +6,6 @@ import type { AgentSemaphore } from "./concurrency.js";
|
|||||||
import { generateReservedWorktreeName, slugify } from "./worktree-names.js";
|
import { generateReservedWorktreeName, slugify } from "./worktree-names.js";
|
||||||
import { schedulerLog } from "./logger.js";
|
import { schedulerLog } from "./logger.js";
|
||||||
import { type PrMonitor, type PrComment } from "./pr-monitor.js";
|
import { type PrMonitor, type PrComment } from "./pr-monitor.js";
|
||||||
import { getCurrentGitHubRepo } from "./github.js";
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Check whether two sets of file scope paths overlap.
|
* Check whether two sets of file scope paths overlap.
|
||||||
@@ -170,7 +169,7 @@ export class Scheduler {
|
|||||||
if (this.options.prMonitor) {
|
if (this.options.prMonitor) {
|
||||||
if (to === "in-review" && task.prInfo) {
|
if (to === "in-review" && task.prInfo) {
|
||||||
// Start monitoring existing PR
|
// Start monitoring existing PR
|
||||||
const repo = getCurrentGitHubRepo(this.store.getRootDir());
|
const repo = getCurrentRepo(this.store.getRootDir());
|
||||||
if (repo) {
|
if (repo) {
|
||||||
this.options.prMonitor.startMonitoring(task.id, repo.owner, repo.repo, task.prInfo);
|
this.options.prMonitor.startMonitoring(task.id, repo.owner, repo.repo, task.prInfo);
|
||||||
}
|
}
|
||||||
@@ -263,7 +262,7 @@ export class Scheduler {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const repo = getCurrentGitHubRepo(this.store.getRootDir());
|
const repo = getCurrentRepo(this.store.getRootDir());
|
||||||
if (repo) {
|
if (repo) {
|
||||||
this.options.prMonitor.startMonitoring(task.id, repo.owner, repo.repo, task.prInfo);
|
this.options.prMonitor.startMonitoring(task.id, repo.owner, repo.repo, task.prInfo);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user