smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(coding-agent): address PR comments and improve reftable support

Browse Source
This commit is contained in:
Matt Anger
2026-06-06 18:29:25 -07:00
committed by can1357
parent 65c56db2f5
commit 2e31abcc16
4 changed files with 170 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/src/modes/components/footer.ts
+3 -1
View File
@@ -1,4 +1,5 @@
import * as fs from "node:fs";
import * as path from "node:path";
import { stripVTControlCharacters } from "node:util";
import { ThinkingLevel } from "@oh-my-pi/pi-agent-core";
import { type Component, padding, truncateToWidth, visibleWidth } from "@oh-my-pi/pi-tui";
@@ -65,7 +66,8 @@ export class FooterComponent implements Component {
}
try {
this.#gitWatcher = fs.watch(head.headPath, () => {
const watchPath = head.isReftable ? path.join(head.commonDir, "reftable", "tables.list") : head.headPath;
this.#gitWatcher = fs.watch(watchPath, () => {
this.#cachedBranch = undefined; // Invalidate cache
if (this.#onBranchChange) {
this.#onBranchChange();
packages/coding-agent/src/modes/components/status-line/component.ts
+8 -3
View File
@@ -1,4 +1,5 @@
import * as fs from "node:fs";
import * as path from "node:path";
import type { AgentMessage } from "@oh-my-pi/pi-agent-core";
import { estimateTokens } from "@oh-my-pi/pi-agent-core/compaction";
import { type Component, truncateToWidth, visibleWidth } from "@oh-my-pi/pi-tui";
@@ -238,11 +239,15 @@ export class StatusLineComponent implements Component {
this.#gitWatcher = null;
}
const gitHeadPath = git.repo.resolveSync(getProjectDir())?.headPath ?? null;
if (!gitHeadPath) return;
const repository = git.repo.resolveSync(getProjectDir());
if (!repository) return;
const watchPath = git.repo.isReftableSync(repository)
? path.join(repository.commonDir, "reftable", "tables.list")
: repository.headPath;
try {
this.#gitWatcher = fs.watch(gitHeadPath, () => {
this.#gitWatcher = fs.watch(watchPath, () => {
this.#invalidateGitCaches();
if (this.#onBranchChange) {
this.#onBranchChange();
packages/coding-agent/src/utils/git.ts
+93 -33
View File
@@ -27,6 +27,7 @@ export interface GitRepository {
gitEntryPath: string;
headPath: string;
repoRoot: string;
isReftable?: boolean;
}
export interface GitStatusSummary {
@@ -560,8 +561,6 @@ function parsePackedRefs(content: string | null, targetRef: string): string | nu
return null;
}
const reftableCache = new Map<string, boolean>();
function parseGitConfigHasReftable(content: string): boolean {
let inExtensions = false;
for (const line of content.split("\n")) {
@@ -570,12 +569,35 @@ function parseGitConfigHasReftable(content: string): boolean {
const section = trimmed.slice(1, -1).trim().toLowerCase();
inExtensions = section === "extensions";
} else if (inExtensions) {
const parts = trimmed.split("=");
if (parts.length >= 2) {
const key = parts[0].trim().toLowerCase();
const value = parts.slice(1).join("=").trim().toLowerCase();
if (key === "refstorage" && value === "reftable") {
return true;
const eqIndex = trimmed.indexOf("=");
if (eqIndex !== -1) {
const key = trimmed.slice(0, eqIndex).trim().toLowerCase();
const value = trimmed.slice(eqIndex + 1).trim();
if (key === "refstorage") {
// Strip trailing comments per git-config(5)
let cleanValue = "";
let inQuotes = false;
for (let i = 0; i < value.length; i++) {
const char = value[i];
if (char === '"') {
inQuotes = !inQuotes;
cleanValue += char;
} else if (!inQuotes && (char === ";" || char === "#")) {
if (i === 0 || /\s/.test(value[i - 1])) {
break;
}
cleanValue += char;
} else {
cleanValue += char;
}
}
cleanValue = cleanValue.trim().toLowerCase();
if (cleanValue.startsWith('"') && cleanValue.endsWith('"')) {
cleanValue = cleanValue.slice(1, -1).trim();
}
if (cleanValue === "reftable") {
return true;
}
}
}
}
@@ -584,28 +606,36 @@ function parseGitConfigHasReftable(content: string): boolean {
}
function isReftableRepoSync(repository: GitRepository): boolean {
const cached = reftableCache.get(repository.commonDir);
if (cached !== undefined) return cached;
if (repository.isReftable !== undefined) return repository.isReftable;
const configPath = path.join(repository.commonDir, "config");
const content = readOptionalTextSync(configPath);
const hasReftable = content ? parseGitConfigHasReftable(content) : false;
reftableCache.set(repository.commonDir, hasReftable);
return hasReftable;
repository.isReftable = content ? parseGitConfigHasReftable(content) : false;
return repository.isReftable;
}
async function isReftableRepo(repository: GitRepository): Promise<boolean> {
const cached = reftableCache.get(repository.commonDir);
if (cached !== undefined) return cached;
if (repository.isReftable !== undefined) return repository.isReftable;
const configPath = path.join(repository.commonDir, "config");
const content = await readOptionalText(configPath);
const hasReftable = content ? parseGitConfigHasReftable(content) : false;
reftableCache.set(repository.commonDir, hasReftable);
return hasReftable;
repository.isReftable = content ? parseGitConfigHasReftable(content) : false;
return repository.isReftable;
}
async function resolveHeadStateReftable(repository: GitRepository): Promise<GitHeadState | null> {
const symResult = await git(repository.repoRoot, ["symbolic-ref", "HEAD"], { readOnly: true }).catch(() => null);
const revResult = await git(repository.repoRoot, ["rev-parse", "HEAD"], { readOnly: true }).catch(() => null);
async function resolveHeadStateReftable(repository: GitRepository, signal?: AbortSignal): Promise<GitHeadState | null> {
throwIfAborted(signal);
const symResult = await git(repository.repoRoot, ["symbolic-ref", "HEAD"], { readOnly: true, signal }).catch(err => {
if (signal?.aborted || (err instanceof Error && (err.name === "AbortError" || err.name === "ToolAbortError"))) {
throw err;
}
return null;
});
throwIfAborted(signal);
const revResult = await git(repository.repoRoot, ["rev-parse", "HEAD"], { readOnly: true, signal }).catch(err => {
if (signal?.aborted || (err instanceof Error && (err.name === "AbortError" || err.name === "ToolAbortError"))) {
throw err;
}
return null;
});
const commit = revResult && revResult.exitCode === 0 ? revResult.stdout.trim() || null : null;
if (symResult && symResult.exitCode === 0) {
@@ -707,15 +737,35 @@ function readRefSync(repository: GitRepository, targetRef: string): string | nul
return null;
}
async function readRef(repository: GitRepository, targetRef: string): Promise<string | null> {
async function readRef(repository: GitRepository, targetRef: string, signal?: AbortSignal): Promise<string | null> {
if (await isReftableRepo(repository)) {
const symResult = await git(repository.repoRoot, ["symbolic-ref", targetRef], { readOnly: true }).catch(
() => null,
throwIfAborted(signal);
const symResult = await git(repository.repoRoot, ["symbolic-ref", targetRef], { readOnly: true, signal }).catch(
err => {
if (
signal?.aborted ||
(err instanceof Error && (err.name === "AbortError" || err.name === "ToolAbortError"))
) {
throw err;
}
return null;
},
);
if (symResult && symResult.exitCode === 0) {
return `${HEAD_REF_PREFIX} ${symResult.stdout.trim()}`;
}
const revResult = await git(repository.repoRoot, ["rev-parse", targetRef], { readOnly: true }).catch(() => null);
throwIfAborted(signal);
const revResult = await git(repository.repoRoot, ["rev-parse", targetRef], { readOnly: true, signal }).catch(
err => {
if (
signal?.aborted ||
(err instanceof Error && (err.name === "AbortError" || err.name === "ToolAbortError"))
) {
throw err;
}
return null;
},
);
if (revResult && revResult.exitCode === 0) {
return revResult.stdout.trim() || null;
}
@@ -1146,7 +1196,7 @@ export const branch = {
const repository = await resolveRepository(cwd);
if (repository) {
for (const refPath of DEFAULT_BRANCH_REFS) {
const target = await readRef(repository, refPath);
const target = await readRef(repository, refPath, signal);
const branchName = parseDefaultBranchRef(refPath, target);
if (branchName) return branchName;
}
@@ -1244,7 +1294,7 @@ export const ref = {
async exists(cwd: string, refName: string, signal?: AbortSignal): Promise<boolean> {
if (refName === "HEAD") return (await head.sha(cwd, signal)) !== null;
const repository = await resolveRepository(cwd);
if (repository && refName.startsWith("refs/")) return (await readRef(repository, refName)) !== null;
if (repository && refName.startsWith("refs/")) return (await readRef(repository, refName, signal)) !== null;
const result = await git(cwd, ["show-ref", "--verify", "--quiet", refName], { readOnly: true, signal });
return result.exitCode === 0;
},
@@ -1253,7 +1303,7 @@ export const ref = {
async resolve(cwd: string, refName: string, signal?: AbortSignal): Promise<string | null> {
if (refName === "HEAD") return head.sha(cwd, signal);
const repository = await resolveRepository(cwd);
if (repository && refName.startsWith("refs/")) return readRef(repository, refName);
if (repository && refName.startsWith("refs/")) return readRef(repository, refName, signal);
const result = await git(cwd, ["rev-parse", refName], { readOnly: true, signal });
if (result.exitCode !== 0) return null;
return result.stdout.trim() || null;
@@ -1546,11 +1596,11 @@ export const ls = {
export const head = {
/** Full HEAD state (branch, commit, repo info). */
async resolve(cwd: string): Promise<GitHeadState | null> {
async resolve(cwd: string, signal?: AbortSignal): Promise<GitHeadState | null> {
const repository = await resolveRepository(cwd);
if (!repository) return null;
if (await isReftableRepo(repository)) {
return resolveHeadStateReftable(repository);
return resolveHeadStateReftable(repository, signal);
}
const content = await readOptionalText(repository.headPath);
if (content === null) return null;
@@ -1571,7 +1621,7 @@ export const head = {
/** Current HEAD commit SHA. */
async sha(cwd: string, signal?: AbortSignal): Promise<string | null> {
const headState = await head.resolve(cwd);
const headState = await head.resolve(cwd, signal);
if (headState?.commit) return headState.commit;
const result = await git(cwd, ["rev-parse", "HEAD"], { readOnly: true, signal });
if (result.exitCode !== 0) return null;
@@ -1626,11 +1676,21 @@ export const repo = {
resolve(cwd: string): Promise<GitRepository | null> {
return resolveRepository(cwd);
},
/** Check if the repository uses the reftable reference storage format (sync). */
isReftableSync(repository: GitRepository): boolean {
return isReftableRepoSync(repository);
},
/** Check if the repository uses the reftable reference storage format. */
isReftable(repository: GitRepository): Promise<boolean> {
return isReftableRepo(repository);
},
};
// Helper used during head resolution — defined here to reference `head` namespace.
async function resolveHead(cwd: string): Promise<GitHeadState | null> {
return head.resolve(cwd);
async function resolveHead(cwd: string, signal?: AbortSignal): Promise<GitHeadState | null> {
return head.resolve(cwd, signal);
}
// ════════════════════════════════════════════════════════════════════════════
packages/coding-agent/test/git-reftable.test.ts
+66 -18
View File
@@ -1,15 +1,18 @@
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
import * as fs from "node:fs/promises";
import * as os from "node:os";
import * as path from "node:path";
import { $ } from "bun";
import * as git from "../src/utils/git";
describe("git reftable support", () => {
const gitInitHelp = await $`git init -h`.quiet().nothrow().text();
const supportsReftable = gitInitHelp.includes("--ref-format");
describe.skipIf(!supportsReftable)("git reftable support", () => {
let testRepoDir: string;
beforeEach(async () => {
testRepoDir = path.join(import.meta.dir, `tmp-reftable-test-${Date.now()}`);
await fs.mkdir(testRepoDir, { recursive: true });
testRepoDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-reftable-"));
});
afterEach(async () => {
@@ -18,15 +21,8 @@ describe("git reftable support", () => {
test("resolves references in a reftable repository", async () => {
// Initialize the repository with reftable format
const initResult = await $`git init --ref-format=reftable --initial-branch=main`
.cwd(testRepoDir)
.quiet()
.nothrow();
if (initResult.exitCode !== 0) {
// If the installed git doesn't support --ref-format=reftable, skip the test
console.warn("Skipping reftable test: Git does not support --ref-format=reftable");
return;
}
const initResult = await $`git init --ref-format=reftable --initial-branch=main`.cwd(testRepoDir).quiet();
expect(initResult.exitCode).toBe(0);
// Configure basic user details so we can commit
await $`git config user.name "Test User"`.cwd(testRepoDir).quiet();
@@ -66,16 +62,16 @@ describe("git reftable support", () => {
// Test HEAD resolution (object shape)
const headState = await git.head.resolve(testRepoDir);
expect(headState).not.toBeNull();
expect(headState?.kind).toBe("ref");
expect((headState as any).branchName).toBe("feature-branch");
expect(headState?.commit).toBe(headSha);
if (headState?.kind !== "ref") throw new Error("expected ref head");
expect(headState.branchName).toBe("feature-branch");
expect(headState.commit).toBe(headSha);
// Test HEAD resolution sync
const headStateSync = git.head.resolveSync(testRepoDir);
expect(headStateSync).not.toBeNull();
expect(headStateSync?.kind).toBe("ref");
expect((headStateSync as any).branchName).toBe("feature-branch");
expect(headStateSync?.commit).toBe(headSha);
if (headStateSync?.kind !== "ref") throw new Error("expected ref head sync");
expect(headStateSync.branchName).toBe("feature-branch");
expect(headStateSync.commit).toBe(headSha);
// Test exists check
const mainExists = await git.ref.exists(testRepoDir, "refs/heads/main");
@@ -83,4 +79,56 @@ describe("git reftable support", () => {
expect(mainExists).toBe(true);
expect(nonexistentExists).toBe(false);
});
test("handles git config trailing comments correctly", async () => {
// Initialize the repository with reftable format
const initResult = await $`git init --ref-format=reftable --initial-branch=main`.cwd(testRepoDir).quiet();
expect(initResult.exitCode).toBe(0);
const repository = await git.repo.resolve(testRepoDir);
expect(repository).not.toBeNull();
if (!repository) return;
expect(await git.repo.isReftable(repository)).toBe(true);
// Now let's manually write to .git/config with comments and test
const configPath = path.join(repository.commonDir, "config");
const baseConfig = await fs.readFile(configPath, "utf8");
// Test trailing semicolon comment
const newConfigWithSemicolon = baseConfig.replace(
"refstorage = reftable",
"refstorage = reftable ; trailing comment",
);
await fs.writeFile(configPath, newConfigWithSemicolon);
const repository2 = await git.repo.resolve(testRepoDir);
expect(repository2).not.toBeNull();
if (repository2) {
expect(await git.repo.isReftable(repository2)).toBe(true);
expect(git.repo.isReftableSync(repository2)).toBe(true);
}
// Test trailing hash comment
const newConfigWithHash = baseConfig.replace("refstorage = reftable", "refstorage = reftable # trailing hash");
await fs.writeFile(configPath, newConfigWithHash);
const repository3 = await git.repo.resolve(testRepoDir);
expect(repository3).not.toBeNull();
if (repository3) {
expect(await git.repo.isReftable(repository3)).toBe(true);
expect(git.repo.isReftableSync(repository3)).toBe(true);
}
// Test double-quoted value containing semicolon (not a comment)
const newConfigWithQuotes = baseConfig.replace("refstorage = reftable", 'refstorage = "reftable ; not comment"');
await fs.writeFile(configPath, newConfigWithQuotes);
const repository4 = await git.repo.resolve(testRepoDir);
expect(repository4).not.toBeNull();
if (repository4) {
// This value would be "reftable ; not comment", which shouldn't match "reftable"
expect(await git.repo.isReftable(repository4)).toBe(false);
expect(git.repo.isReftableSync(repository4)).toBe(false);
}
});
});