feat(antigravity): warn when a known false-429 fingerprint leaves unmasked - #5790
yangjijian2121 wants to merge 3 commits into
Conversation
…asked
Google's Cloud Code Assist content rule answers a request whose system
instruction carries certain harness phrases with a misleading 429
RESOURCE_EXHAUSTED ("Resource has been exhausted (e.g. check quota).")
while the credential's quota is untouched. The credential then cools down
and callers only see the local "All credentials for model ... are cooling
down via provider antigravity" error, which points operators at quota
instead of at request content. Masking the phrase with the existing
antigravity.sensitive-words option is what clears it.
Detect the phrases on the payload that actually leaves the process and warn
once per phrase per process, so a permanently flagged client cannot flood
the log. The check is a diagnostic only and never rewrites the payload:
masking request content stays an explicit operator decision.
Phrases are limited to those with public reproduction evidence
(router-for-me#5751, can1357/oh-my-pi#11699).
…ields The warning carried the matched phrase and upstream condition inside an interpolated message, which cannot be filtered or aggregated. Emit a constant message with phrase / executor / component / upstream_status / suggested_config fields per the logrus structured-logging rule in AGENTS.md, and assert the fields (plus the absence of interpolation) in the test.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e0196c378
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if _, warned := antigravityFalse429Warned.LoadOrStore(phrase, struct{}{}); warned { | ||
| continue | ||
| } | ||
| log.Warnf("antigravity executor: system instruction contains %q, which Google Cloud Code Assist rejects with a misleading 429 RESOURCE_EXHAUSTED (\"Resource has been exhausted (e.g. check quota).\") while quota is intact, after which the credential cools down; mask it with antigravity.sensitive-words when the client prompt cannot change", phrase) |
There was a problem hiding this comment.
Emit the diagnostic with structured log fields
When operators need to filter or aggregate this new diagnostic, Warnf embeds the matched phrase and upstream condition in an opaque, variable message instead of stable fields. Emit a constant message with fields such as phrase, provider, and upstream_status so the warning follows the repository's structured-logging requirement and remains queryable.
AGENTS.md reference: AGENTS.md:L56-L56
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9c8384d: the warning now emits a constant message with phrase, executor, component, upstream_status (429) and suggested_config (antigravity.sensitive-words) fields, and the test asserts the fields plus the absence of interpolation in the message.
The built-in list is evidence-backed but hardcoded, so a deployment tracking a different upstream rule - or one that wants no diagnostic at all - had no lever. antigravity.false-429-phrases is tri-state: unset keeps the built-in list, an empty list disables the warning, and any other list replaces it. The config watcher reports the field so hot reloads surface the change, and config.example.yaml documents it next to antigravity.sensitive-words. Tests cover the three states: parsing (including that an explicit empty list survives parsing instead of collapsing into nil), a configured list replacing the built-in phrases, and an empty list silencing the warning.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7c7025347
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if oldFalse429Phrases != newFalse429Phrases { | ||
| changes = append(changes, fmt.Sprintf("antigravity.false-429-phrases: %s -> %s", oldFalse429Phrases, newFalse429Phrases)) |
There was a problem hiding this comment.
Compare phrase contents when reporting reloads
When an operator replaces one configured phrase with another while keeping the list length unchanged, both formatted values are identical (for example, 1 and 1), so this branch emits no change detail. If this is the only edit, config_reload.go consequently reports “no material config field changes detected” even though the diagnostic behavior has changed; compare the pointed-to slices with reflect.DeepEqual and use the counts only for the redacted message.
Useful? React with 👍 / 👎.
Makes the diagnosis half of #5751 discoverable from a running proxy.
Problem
A request whose
systemInstructioncarries one of the harness phrases below is answered byCloud Code Assist with a misleading 429 while the credential's quota is untouched:
{"error":{"code":429,"message":"Resource has been exhausted (e.g. check quota).","status":"RESOURCE_EXHAUSTED"}}The executor treats that as a rate-limit hit, the credential cools down, and the operator only
sees the local
All credentials for model ... are cooling down via provider antigravityerror,which points at quota. Masking the phrase with the existing
antigravity.sensitive-wordsoptionis what actually clears it — but nothing in the proxy says that request content is the cause, so
the search goes to quota pages, credential rotation, and restarts. The bisection work in #5751
and can1357/oh-my-pi#11699 is exactly that search.
Change
internal/runtime/executor/antigravity_false_429_fingerprint.go(new): detects the knownphrases in
request.systemInstruction.partsand logs one warning per phrase per process,with
phrase/executor/component/upstream_status/suggested_configfields.antigravity.false-429-phrases(new config key, tri-state): unset keeps the built-inlist, an explicit empty list disables the diagnostic, any other list replaces the built-in
phrases for deployments tracking a different upstream rule.
antigravity_executor.go:obfuscateSensitiveWordsnow runs the detector over its ownoutput, so every path that already obfuscates (execute, stream, countTokens) is covered and a
phrase the operator already masks is not reported as still leaving.
internal/watcher/diff: reports the new field so hot reloads surface the change.config.example.yaml: documents the symptom, the phrase list, and the new key next toantigravity.sensitive-words.Diagnostic only: no payload rewrite, no change to retry or cooldown behavior. Masking content
stays an explicit operator decision.
Why this phrase list
Limited to phrases with public reproduction evidence:
<system-conventions>/RFC 2119: MUST, REQUIRED, SHOULD, RECOMMENDED, MAY, OPTIONAL.as hardcoded by oh-my-pi / omp(can1357/oh-my-pi#11699, #11730) plus the sibling tag variants from #5751.
The trigger is request content, not a client identity: the same literal sent as raw curl through
an Antigravity-backed OpenAI-compatible endpoint reproduces the upstream failure, and replacing
only
RFCwithRF<U+200B>Creturns HTTP 200 on a 62 KB payload. A phrase added without thatkind of evidence would make this warning wrong, which is why the list is short and why operators
can override it.
Tests
go test ./internal/config/ ./internal/runtime/executor/ ./internal/watcher/... -count=1process, stays silent once the operator masks the phrases, ignores the same phrase quoted by
the user in message content, a configured list replaces the built-in one, an empty list
silences the warning, and parsing keeps an explicit empty list instead of collapsing it to nil.
Note:
TestOpenAICompatExecutorToolResultContentByInputModalitiesfails ininternal/runtime/executoron this machine, on the unmodified branch as well (verified bystashing); it is unrelated to this change.