Skip to content

feat(antigravity): warn when a known false-429 fingerprint leaves unmasked - #5790

Open
yangjijian2121 wants to merge 3 commits into
router-for-me:devfrom
yangjijian2121:feat/antigravity-false-429-fingerprint-warning
Open

yangjijian2121 wants to merge 3 commits into
router-for-me:devfrom
yangjijian2121:feat/antigravity-false-429-fingerprint-warning

Conversation

@yangjijian2121

@yangjijian2121 yangjijian2121 commented Sep 13, 2026

Copy link
Copy Markdown

Makes the diagnosis half of #5751 discoverable from a running proxy.

Problem

A request whose systemInstruction carries one of the harness phrases below is answered by
Cloud 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 antigravity error,
which points at quota. Masking the phrase with the existing antigravity.sensitive-words option
is 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 known
    phrases in request.systemInstruction.parts and logs one warning per phrase per process,
    with phrase / executor / component / upstream_status / suggested_config fields.
  • antigravity.false-429-phrases (new config key, tri-state): unset keeps the built-in
    list, 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: obfuscateSensitiveWords now runs the detector over its own
    output, 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 to
    antigravity.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 RFC with RF<U+200B>C returns HTTP 200 on a 62 KB payload. A phrase added without that
kind 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=1
  • New cases: warns on each flagged phrase with structured fields, warns once per phrase per
    process, 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: TestOpenAICompatExecutorToolResultContentByInputModalities fails in
internal/runtime/executor on this machine, on the unmodified branch as well (verified by
stashing); it is unrelated to this change.

…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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +119 to +120
if oldFalse429Phrases != newFalse429Phrases {
changes = append(changes, fmt.Sprintf("antigravity.false-429-phrases: %s -> %s", oldFalse429Phrases, newFalse429Phrases))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant