Skip to content

Commit 19d002b

Browse files
committed
fix(cli): reject future-dated star deferral records on the version-match path
A future-dated .star-deferred with a matching version suppressed the deferral indefinitely (the version-match shortcut returned before the age check). The age is now computed first and negative age fails toward re-asking on every path. Adds the behavior-level flow test (agent deferral fires once per version, marker never written, human run still prompts) behind a gh/interactiveConfirm test seam, and pins the future-match regression.
1 parent 17ec899 commit 19d002b

3 files changed

Lines changed: 113 additions & 6 deletions

File tree

src/cli/star-prompt.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,10 @@ export function isDeferralCurrent(record: string | null, version: string, now: n
3333
if (!m) return false;
3434
const at = Date.parse(m[1]);
3535
if (Number.isNaN(at)) return false;
36-
if (version !== "?" && m[2] === version) return true;
3736
const age = now - at;
37+
// Version match suppresses for that whole version, but a future-dated
38+
// record (clock rollback) fails toward re-asking on every path.
39+
if (version !== "?" && m[2] === version) return age >= 0;
3840
return age >= 0 && age < DEFERRAL_MAX_AGE_MS;
3941
}
4042

@@ -83,6 +85,15 @@ function ghAvailable(): boolean {
8385
return !auth.error && auth.status === 0;
8486
}
8587

88+
/** Test seam: replace gh/interactiveConfirm so the full prompt flow is
89+
* drivable without a real gh login or a TTY conversation. */
90+
let depsForTests: { ghAvailable?: () => boolean; interactiveConfirm?: typeof interactiveConfirm } | null = null;
91+
export function setStarPromptDepsForTests(
92+
deps: { ghAvailable?: () => boolean; interactiveConfirm?: typeof interactiveConfirm } | null,
93+
): void {
94+
depsForTests = deps;
95+
}
96+
8697
function starRepo(): { ok: boolean; error?: string } {
8798
const star = ghInvocation(["api", "-X", "PUT", `/user/starred/${REPO}`]);
8899
const r = spawnSync(star.file, star.args,
@@ -158,7 +169,8 @@ export async function maybeShowStarPrompt(): Promise<void> {
158169
const dir = getConfigDir();
159170
const marker = join(dir, MARKER);
160171
if (existsSync(marker)) return;
161-
if (!ghAvailable()) return; // can't star without an authenticated gh — stay silent and re-check on a later start
172+
const ghOk = depsForTests?.ghAvailable ? depsForTests.ghAvailable() : ghAvailable();
173+
if (!ghOk) return; // can't star without an authenticated gh — stay silent and re-check on a later start
162174

163175
// An agent would answer this on the user's behalf, using the user's GitHub
164176
// identity. Hand the question to the agent to relay, and leave the marker
@@ -185,7 +197,8 @@ export async function maybeShowStarPrompt(): Promise<void> {
185197
writeFileSync(marker, new Date().toISOString());
186198
} catch { /* best-effort */ }
187199

188-
const yes = await interactiveConfirm({
200+
const ask = depsForTests?.interactiveConfirm ?? interactiveConfirm;
201+
const yes = await ask({
189202
question: "\n \x1b[38;5;141m⭐ Enjoying opencodex? Star it on GitHub (via gh)?\x1b[0m",
190203
defaultYes: true,
191204
});

tests/star-deferral.test.ts

Lines changed: 95 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
1-
import { describe, expect, test } from "bun:test";
2-
import { isDeferralCurrent } from "../src/cli/star-prompt";
1+
import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test";
2+
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
3+
import { tmpdir } from "node:os";
4+
import { join } from "node:path";
5+
import { existsSync } from "node:fs";
6+
import { isDeferralCurrent, maybeShowStarPrompt, setStarPromptDepsForTests } from "../src/cli/star-prompt";
37

48
const NOW = Date.parse("2026-08-02T00:00:00.000Z");
59
const DAY = 24 * 60 * 60 * 1000;
@@ -34,4 +38,93 @@ describe("isDeferralCurrent", () => {
3438
const future = `${new Date(NOW + 30 * DAY).toISOString()} 2.9.1`;
3539
expect(isDeferralCurrent(future, "2.10.0", NOW)).toBe(false);
3640
});
41+
42+
test("a future-dated record with a MATCHING version also fails toward re-asking", () => {
43+
// Clock rollback must not suppress the deferral for the version forever.
44+
const future = `${new Date(NOW + 30 * DAY).toISOString()} 2.10.0`;
45+
expect(isDeferralCurrent(future, "2.10.0", NOW)).toBe(false);
46+
});
47+
});
48+
49+
describe("maybeShowStarPrompt deferral flow (behavior)", () => {
50+
let home: string;
51+
const priorHome = process.env.OPENCODEX_HOME;
52+
const priorThread = process.env.CODEX_THREAD_ID;
53+
const stdinTTY = process.stdin.isTTY;
54+
const stdoutTTY = process.stdout.isTTY;
55+
const AGENT_ENV_VARS = [
56+
"CLAUDECODE", "CLAUDE_CODE_ENTRYPOINT", "CLAUDE_CODE_SSE_PORT",
57+
"CODEX_THREAD_ID", "CODEX_SHELL", "CODEX_CI", "CODEX_SANDBOX", "CODEX_SANDBOX_NETWORK_DISABLED",
58+
"CURSOR_TRACE_ID", "CURSOR_SESSION_TOKEN", "CURSOR_AGENT",
59+
"AIDER_CHAT", "OPENCODE_BIN_PATH", "GEMINI_CLI",
60+
"REPL_ID", "CI", "GITHUB_ACTIONS", "GITLAB_CI", "BUILDKITE", "JENKINS_URL", "TEAMCITY_VERSION", "CODESPACES",
61+
];
62+
const savedAgentEnv = new Map<string, string | undefined>();
63+
64+
beforeEach(() => {
65+
home = mkdtempSync(join(tmpdir(), "ocx-star-deferral-"));
66+
process.env.OPENCODEX_HOME = home;
67+
for (const name of AGENT_ENV_VARS) {
68+
savedAgentEnv.set(name, process.env[name]);
69+
delete process.env[name];
70+
}
71+
Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true });
72+
Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true });
73+
});
74+
75+
afterEach(() => {
76+
setStarPromptDepsForTests(null);
77+
for (const name of AGENT_ENV_VARS) {
78+
const value = savedAgentEnv.get(name);
79+
if (value === undefined) delete process.env[name];
80+
else process.env[name] = value;
81+
}
82+
Object.defineProperty(process.stdin, "isTTY", { value: stdinTTY, configurable: true });
83+
Object.defineProperty(process.stdout, "isTTY", { value: stdoutTTY, configurable: true });
84+
if (priorThread === undefined) delete process.env.CODEX_THREAD_ID;
85+
else process.env.CODEX_THREAD_ID = priorThread;
86+
if (priorHome === undefined) delete process.env.OPENCODEX_HOME;
87+
else process.env.OPENCODEX_HOME = priorHome;
88+
rmSync(home, { recursive: true, force: true });
89+
});
90+
91+
test("agent deferral fires once per version, never writes the marker, and a human run still prompts", async () => {
92+
process.env.CODEX_THREAD_ID = "agent-session";
93+
setStarPromptDepsForTests({
94+
ghAvailable: () => true,
95+
interactiveConfirm: async () => false,
96+
});
97+
const log = spyOn(console, "log").mockImplementation(() => {});
98+
try {
99+
await maybeShowStarPrompt();
100+
const firstCalls = log.mock.calls.length;
101+
expect(firstCalls).toBeGreaterThan(0);
102+
// The deferral record exists; the one-time marker does NOT.
103+
expect(existsSync(join(home, ".star-deferred"))).toBe(true);
104+
expect(existsSync(join(home, ".star-prompted"))).toBe(false);
105+
expect(readFileSync(join(home, ".star-deferred"), "utf-8")).toContain(" ");
106+
107+
// Second agent-driven start: suppressed by the record.
108+
log.mockClear();
109+
await maybeShowStarPrompt();
110+
expect(log.mock.calls.length).toBe(0);
111+
expect(existsSync(join(home, ".star-prompted"))).toBe(false);
112+
113+
// A hand-typed run still gets the real question (marker written, ask called).
114+
delete process.env.CODEX_THREAD_ID;
115+
let asked = 0;
116+
setStarPromptDepsForTests({
117+
ghAvailable: () => true,
118+
interactiveConfirm: async () => {
119+
asked += 1;
120+
return false;
121+
},
122+
});
123+
await maybeShowStarPrompt();
124+
expect(asked).toBe(1);
125+
expect(existsSync(join(home, ".star-prompted"))).toBe(true);
126+
} finally {
127+
log.mockRestore();
128+
}
129+
});
37130
});

tests/startup-prompt.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,8 @@ describe("startup star prompt", () => {
142142
// shim), so assert the arguments and the resolver rather than the literal.
143143
expect(prompt).toContain('ghInvocation(["auth", "status"])');
144144
expect(prompt).toContain('commandInvocation("gh"');
145-
expect(prompt).toContain("if (!ghAvailable()) return;");
145+
// The gh check gates the prompt via the (test-seamable) ghOk result.
146+
expect(prompt).toContain("if (!ghOk) return;");
146147
});
147148

148149
test("declining the star prompt does not steer the agent afterwards", async () => {

0 commit comments

Comments
 (0)