-
Notifications
You must be signed in to change notification settings - Fork 9.7k
fix(antigravity): use temp file for prompt instead of stdin (fixes #5… #6332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
aa70080
ac17db4
60098f5
872cad1
ceb2bc9
63d7f20
2c0563d
4c52284
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,10 @@ import { | |
| mkdirSync, | ||
| readFileSync, | ||
| writeFileSync, | ||
| renameSync, | ||
| statSync, | ||
| } from 'node:fs'; | ||
| import { randomBytes } from 'node:crypto'; | ||
| import { readFile as fsReadFile } from 'node:fs/promises'; | ||
| import { homedir } from 'node:os'; | ||
| import { dirname, join } from 'node:path'; | ||
|
|
@@ -49,9 +52,11 @@ export function writeAntigravityModelSelection( | |
| label: string, | ||
| settingsPath: string = ANTIGRAVITY_SETTINGS_PATH, | ||
| ): void { | ||
| let fileMode = 0o600; | ||
| let existing: Record<string, unknown> = {}; | ||
| if (existsSync(settingsPath)) { | ||
| try { | ||
| fileMode = statSync(settingsPath).mode; | ||
| const parsed = JSON.parse(readFileSync(settingsPath, 'utf8')) as unknown; | ||
| if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { | ||
| existing = parsed as Record<string, unknown>; | ||
|
|
@@ -63,7 +68,11 @@ export function writeAntigravityModelSelection( | |
| } | ||
| existing.model = label; | ||
| mkdirSync(dirname(settingsPath), { recursive: true }); | ||
| writeFileSync(settingsPath, `${JSON.stringify(existing, null, 2)}\n`); | ||
|
|
||
| // Use atomic write to prevent JSON corruption during concurrent agent spawns | ||
| const tempPath = `${settingsPath}.${randomBytes(4).toString('hex')}.tmp`; | ||
| writeFileSync(tempPath, `${JSON.stringify(existing, null, 2)}\n`, { mode: fileMode }); | ||
| renameSync(tempPath, settingsPath); | ||
| } | ||
|
|
||
| // Per-process serialization for write-settings → spawn → agy-reads | ||
|
|
@@ -215,39 +224,27 @@ export const antigravityAgentDef = { | |
| runtimeContext.antigravitySettingsPath, | ||
| ); | ||
| } | ||
| // We invoke agy via `-p -` (print mode + stdin sentinel), NOT | ||
| // `chat -`. Verified against `agy --help` on v1.0.3 — the | ||
| // `Available subcommands` list is `changelog / help / install / | ||
| // plugin / update`, and `chat` is NOT among them. `-p` is the | ||
| // documented print-mode flag (`Short alias for --print`) and | ||
| // `agy -p -` reads the prompt from stdin. The looper reviewer | ||
| // bot's environment runs a different agy build that may have | ||
| // renamed the entry point; until upstream confirms a stable | ||
| // headless subcommand (see google-antigravity/antigravity-cli#119) | ||
| // and the change actually ships in the auto-update channel that | ||
| // packaged OD users get, `-p -` is the contract that actually | ||
| // produces a print-mode reply on the installed CLI. | ||
| // We no longer use `-p -` because recent `agy` versions treat `-` as a literal | ||
| // prompt string instead of reading from stdin (see issue #5495). | ||
| // Instead, we use `promptViaFile: true` so the daemon securely prepares a | ||
| // managed temp file per-run and cleans it up after the agent exits. | ||
| if (!runtimeContext.promptFilePath) { | ||
| throw new Error('antigravity requires runtimeContext.promptFilePath when promptViaFile is true'); | ||
| } | ||
|
|
||
| const args: string[] = []; | ||
| // Always opt into `--log-file` when the daemon supplied a path so | ||
| // it can post-exit grep for the actual upstream failure shape | ||
| // (auth missing vs quota reached vs upstream error) — without it | ||
| // the chat surfaces a generic "empty response" because print mode | ||
| // never echoes those errors on stdout. See server.ts empty-output | ||
| // guard for the consumer. | ||
| // | ||
| // Flag order is load-bearing on agy v1.0.3: `agy -p --log-file | ||
| // /tmp/x -` runs successfully but leaves /tmp/x empty, while `agy | ||
| // --log-file /tmp/x -p -` captures the diagnostic log, including | ||
| // `Propagating selected model override to backend: label="<model>"` | ||
| // and auth/quota failures. | ||
| if (runtimeContext.agentLogFilePath) { | ||
| args.push('--log-file', runtimeContext.agentLogFilePath); | ||
| args.push(`--log-file=${runtimeContext.agentLogFilePath}`); | ||
| } | ||
|
|
||
| args.push(`--add-dir=${dirname(runtimeContext.promptFilePath)}`); | ||
|
|
||
| args.push('-p'); | ||
| args.push('-'); | ||
| args.push(`Read the system instructions, conversation history, and user request from the file ${runtimeContext.promptFilePath}. Follow the instructions strictly and provide the final response to the user's latest request.`); | ||
| return args; | ||
| }, | ||
| promptViaStdin: true, | ||
| promptViaStdin: false, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking — update the focused regression test with the transport contract. This line changes |
||
| promptViaFile: true, | ||
| streamFormat: 'plain', | ||
| installUrl: 'https://antigravity.google/cli', | ||
| docsUrl: 'https://antigravity.google/docs/cli-overview', | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maintainer attention — keep the silent-failure log contract covered. Changing this option to the single-token
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.--log-file=…form leaves the existing fake Antigravity CLI inapps/daemon/tests/connection-test.test.tsunable to find the requested log path: that fixture looks up the exact--log-filetoken and reads the following argument. On this head,pnpm exec vitest run -c vitest.config.ts tests/connection-test.test.tsfails 1 of 156 tests; the quota-exhaustion case receivesagent_auth_requiredinstead of the expectedrate_limitedbecause the diagnostic log is never written. This matters because that test pins the user-visible distinction between OAuth recovery and quota recovery, and the repository approval bar requires the matching package tests to pass. Please either retain the previously validated two-token form here, or update the fake CLI to accept the equals form after validating that the supportedagyversions do too, then rerun the connection-test file.