Skip to content

Commit fdfd06e

Browse files
committed
fix: stop reporting the override difference as uncommitted work
Once per-machine overrides are in use the config file on disk is meant to differ from the committed version — that is the entire mechanism. But status asked git, so it reported one modified file forever: a warning the user could never clear, and one that invites a pointless push. The config is now compared with override-owned keys subtracted. Identical means clean; anything else is still reported. Fixing that surfaced a second bug. The porcelain parser sliced at a fixed column offset, but the git wrapper trims its output, which eats the leading column of an unstaged entry and shifts every field after it. ' M file' arrived as 'M file' and the parser returned 'ile'. Paths are now matched by pattern, which also handles quoted paths and renames.
1 parent fd51cea commit fdfd06e

8 files changed

Lines changed: 199 additions & 7 deletions

File tree

CHANGELOG.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,18 @@
11
# Changelog
22

3+
## 3.0.1
4+
5+
### Fixed
6+
- `status` no longer reports the config file as uncommitted work on machines
7+
that use per-machine overrides. The file on disk is *supposed* to differ from
8+
the committed version once overrides are in play, so plain `git status`
9+
flagged it as modified forever — a warning the user could never clear. The
10+
config is now compared with overrides subtracted, so only genuine edits count.
11+
- `git status --porcelain` output is parsed by pattern rather than at a fixed
12+
column offset. Output is trimmed before parsing, which removed the leading
13+
column of an unstaged entry and shifted every field after it, silently
14+
yielding a truncated filename.
15+
316
## 3.0.0
417

518
Complete rewrite. The previous release was a set of shell-wrapped Node scripts;

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "opencode-github-sync",
3-
"version": "3.0.0",
3+
"version": "3.0.1",
44
"description": "Sync your OpenCode config, skills, and selected sessions across machines via a private GitHub repo — as a plugin or a CLI.",
55
"type": "module",
66
"license": "MIT",

src/core/git.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,33 @@ export function toGitError(operation: string, result: GitResult): Error {
134134
: new GitError(operation, result);
135135
}
136136

137+
/**
138+
* Extract the file paths from `git status --porcelain` output.
139+
*
140+
* The format is fixed-width — two status columns, a space, then the path — but
141+
* this deliberately does not slice at a fixed offset. Output here is trimmed
142+
* before parsing, which eats the leading space of an unstaged entry (` M file`)
143+
* and shifts every subsequent column. Matching the status field by pattern
144+
* instead survives that, as well as the quoting git applies to paths with
145+
* unusual characters.
146+
*
147+
* Renames appear as `old -> new`; the destination is reported, since that is
148+
* what exists afterwards.
149+
*/
150+
export function parsePorcelainPaths(output: string): string[] {
151+
const paths: string[] = [];
152+
for (const line of String(output ?? "").split("\n")) {
153+
if (!line.trim()) continue;
154+
const withoutStatus = line.replace(/^\s*[MADRCU?!]{1,2}\s+/, "");
155+
if (withoutStatus === line.trim() && !/^\s*[MADRCU?!]/.test(line)) continue;
156+
const arrow = withoutStatus.lastIndexOf(" -> ");
157+
const raw = arrow === -1 ? withoutStatus : withoutStatus.slice(arrow + 4);
158+
const unquoted = raw.trim().replace(/^"(.*)"$/, "$1");
159+
if (unquoted) paths.push(unquoted);
160+
}
161+
return paths;
162+
}
163+
137164
export interface NameStatusEntry {
138165
kind: "added" | "modified" | "deleted" | "renamed";
139166
path: string;

src/core/overrides.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,48 @@ function isPlainObject(value: unknown): value is Record<string, any> {
114114
return typeof value === "object" && value !== null && !Array.isArray(value);
115115
}
116116

117+
/** Serialise with sorted keys, so comparisons ignore key order. */
118+
function canonical(value: unknown): string {
119+
if (Array.isArray(value)) return `[${value.map(canonical).join(",")}]`;
120+
if (isPlainObject(value)) {
121+
const entries = Object.keys(value)
122+
.sort()
123+
.map((key) => `${JSON.stringify(key)}:${canonical(value[key])}`);
124+
return `{${entries.join(",")}}`;
125+
}
126+
return JSON.stringify(value) ?? "null";
127+
}
128+
129+
/**
130+
* True when the config on disk is, apart from override-owned keys, exactly what
131+
* the repository already holds.
132+
*
133+
* Once overrides are in use the file on disk permanently differs from the
134+
* committed version — that is the whole point. Plain `git status` therefore
135+
* reports it as modified forever, which looks like pending work that never goes
136+
* away. This lets the status command tell a real edit apart from the expected
137+
* override difference.
138+
*
139+
* Returns `false` when there are no overrides, so callers fall back to git.
140+
*/
141+
export function configMatchesRepo(configRoot: string, previousText: string | undefined): boolean {
142+
const overrides = loadOverrides(configRoot);
143+
if (Object.keys(overrides).length === 0) return false;
144+
if (!previousText) return false;
145+
146+
const configFile = findConfigFile(configRoot);
147+
if (!configFile) return false;
148+
149+
try {
150+
const effective = parseJsonc<Record<string, any>>(fs.readFileSync(configFile, "utf8"));
151+
const previous = parseJsonc<Record<string, any>>(previousText);
152+
return canonical(stripOverrides(effective, overrides, previous)) === canonical(previous);
153+
} catch {
154+
// Unparseable on either side: treat it as a genuine difference.
155+
return false;
156+
}
157+
}
158+
117159
/**
118160
* Produce the text that should be committed for the config file.
119161
*

src/core/sync.ts

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
11
import fs from "node:fs";
22
import path from "node:path";
3-
import { type GitResult, git, isGitRepo, parseNameStatus, toGitError } from "./git.js";
3+
import {
4+
type GitResult,
5+
git,
6+
isGitRepo,
7+
parseNameStatus,
8+
parsePorcelainPaths,
9+
toGitError,
10+
} from "./git.js";
411
import { commitMessage, hostAlias } from "./host.js";
5-
import { applyOverrides } from "./overrides.js";
12+
import { applyOverrides, configMatchesRepo, findConfigFile } from "./overrides.js";
613
import { type Roots, getDatabasePath, getRoots } from "./paths.js";
714
import {
815
LOCAL_RUNTIME_PATHS,
@@ -493,7 +500,7 @@ export function status(options: SyncOptions = {}): StatusResult {
493500
base.behind = countCommits(root, `HEAD..origin/${branch}`);
494501

495502
const dirty = git(["status", "--porcelain", "--", ".", ...excludePathspecs()], { cwd: root });
496-
base.dirty = dirty.ok ? dirty.stdout.split("\n").filter(Boolean).length : 0;
503+
base.dirty = dirty.ok ? countRealChanges(root, dirty.stdout) : 0;
497504

498505
const log = git(["log", "-1", "--format=%s%n%cI"], { cwd: root });
499506
if (log.ok) {
@@ -510,6 +517,27 @@ function countShards(root: string): number {
510517
return fs.readdirSync(dir).filter((f) => f.endsWith(".json.gz")).length;
511518
}
512519

520+
/**
521+
* Count genuinely uncommitted files.
522+
*
523+
* With overrides in use the config file always differs from the committed
524+
* version, so a raw `git status` would report one modified file forever. That
525+
* entry is dropped when the only difference is the overrides themselves.
526+
*/
527+
function countRealChanges(root: string, porcelain: string): number {
528+
const paths = parsePorcelainPaths(porcelain);
529+
if (paths.length === 0) return 0;
530+
531+
const configFile = findConfigFile(root);
532+
if (!configFile) return paths.length;
533+
534+
const relative = path.relative(root, configFile).split(path.sep).join("/");
535+
const head = git(["show", `HEAD:${relative}`], { cwd: root });
536+
if (!configMatchesRepo(root, head.ok ? head.stdout : undefined)) return paths.length;
537+
538+
return paths.filter((file) => file !== relative).length;
539+
}
540+
513541
// ── Helpers ─────────────────────────────────────────────────────────────────
514542

515543
function countCommits(root: string, range: string): number {

tests/config.test.ts

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from "vitest";
2-
import { parseNameStatus } from "../src/core/git.js";
2+
import { parseNameStatus, parsePorcelainPaths } from "../src/core/git.js";
33
import { commitMessage, hostAlias, sanitizeAlias } from "../src/core/host.js";
44
import { deepMerge, parseJsonc } from "../src/core/jsonc.js";
55
import { stripOverrides } from "../src/core/overrides.js";
@@ -156,6 +156,47 @@ describe("isLocalRuntimePath", () => {
156156
});
157157
});
158158

159+
describe("parsePorcelainPaths", () => {
160+
it("reads a staged entry", () => {
161+
expect(parsePorcelainPaths("M opencode.jsonc")).toEqual(["opencode.jsonc"]);
162+
});
163+
164+
it("reads an unstaged entry that still has its leading space", () => {
165+
expect(parsePorcelainPaths(" M opencode.jsonc")).toEqual(["opencode.jsonc"]);
166+
});
167+
168+
it("reads an unstaged entry whose leading space was trimmed away", () => {
169+
// The git wrapper trims its output, which eats the leading column of the
170+
// first line. Slicing at a fixed offset silently returned "pencode.jsonc"
171+
// here and made the caller miscount.
172+
expect(parsePorcelainPaths("M opencode.jsonc")).toEqual(["opencode.jsonc"]);
173+
});
174+
175+
it("reads untracked entries", () => {
176+
expect(parsePorcelainPaths("?? command/new.md")).toEqual(["command/new.md"]);
177+
});
178+
179+
it("reports the destination of a rename", () => {
180+
expect(parsePorcelainPaths('R "old name.md" -> "new name.md"')).toEqual(["new name.md"]);
181+
});
182+
183+
it("unquotes a path containing spaces", () => {
184+
// git always reports forward slashes, and quotes a path only when it holds
185+
// unusual characters.
186+
expect(parsePorcelainPaths('A "skills/my skill/SKILL.md"')).toEqual([
187+
"skills/my skill/SKILL.md",
188+
]);
189+
});
190+
191+
it("handles several entries and ignores blank lines", () => {
192+
expect(parsePorcelainPaths("M a.md\n\n?? b.md\n D c.md\n")).toEqual(["a.md", "b.md", "c.md"]);
193+
});
194+
195+
it("returns nothing for empty input", () => {
196+
expect(parsePorcelainPaths("")).toEqual([]);
197+
});
198+
});
199+
159200
describe("parseNameStatus", () => {
160201
it("maps status characters to change kinds", () => {
161202
const entries = parseNameStatus("A\tnew.txt\nM\tmod.txt\nD\tgone.txt");

tests/sync.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,47 @@ describe("status", () => {
349349
expect(result.configured).toBe(false);
350350
});
351351

352+
it("does not report the override difference as uncommitted work", async () => {
353+
// The config on disk always differs from the committed version once
354+
// overrides are in use. Reporting that as pending work would show a warning
355+
// that the user can never clear.
356+
const a = makeMachine("a");
357+
useMachine(a);
358+
write(path.join(a.config, "opencode.jsonc"), '{ "model": "shared", "theme": "dark" }\n');
359+
await push({ settings: settings(), roots: getRoots() });
360+
361+
const b = makeMachine("b");
362+
useMachine(b);
363+
write(path.join(b.config, "opencode-sync.overrides.jsonc"), '{ "model": "mine" }\n');
364+
await pull({ settings: settings(), roots: getRoots() });
365+
366+
// git itself still sees a modified file — that is expected.
367+
const porcelain = execFileSync("git", ["status", "--porcelain"], {
368+
cwd: b.config,
369+
encoding: "utf8",
370+
});
371+
expect(porcelain).toMatch(/opencode\.jsonc/);
372+
373+
// But the reported state is clean, because nothing is actually pending.
374+
expect(status({ settings: settings(), roots: getRoots() }).dirty).toBe(0);
375+
});
376+
377+
it("still reports a real edit made alongside an override", async () => {
378+
const a = makeMachine("a");
379+
useMachine(a);
380+
write(path.join(a.config, "opencode.jsonc"), '{ "model": "shared", "theme": "dark" }\n');
381+
await push({ settings: settings(), roots: getRoots() });
382+
383+
const b = makeMachine("b");
384+
useMachine(b);
385+
write(path.join(b.config, "opencode-sync.overrides.jsonc"), '{ "model": "mine" }\n');
386+
await pull({ settings: settings(), roots: getRoots() });
387+
388+
// A change to a key the overrides do not claim is genuine pending work.
389+
write(path.join(b.config, "opencode.jsonc"), '{ "model": "mine", "theme": "light" }\n');
390+
expect(status({ settings: settings(), roots: getRoots() }).dirty).toBeGreaterThan(0);
391+
});
392+
352393
it("reports pending incoming commits", async () => {
353394
const a = makeMachine("a");
354395
useMachine(a);

0 commit comments

Comments
 (0)