feat(google): add Gemini execution guidance via prompt overlay [v3 final 2/3]#67518
feat(google): add Gemini execution guidance via prompt overlay [v3 final 2/3]#67518100yenadmin wants to merge 2 commits into
Conversation
Ported from Hermes Agent's GOOGLE_MODEL_OPERATIONAL_GUIDANCE (prompt_builder.py lines 258-276) with Hermes-verbatim text (only mechanical tool-ID substitutions and one documented deviation: "Absolute paths" bullet omitted due to sandbox path conflict at src/agents/system-prompt.ts:600). Changes: - New: extensions/google/prompt-overlay.ts with Gemini operational directives (verify first, dependency checks, parallel calls, non-interactive commands, keep going) - Wire resolveSystemPromptContribution into both google and google-gemini-cli providers (rebased on 356110c's provider-hooks refactor) - Add tool_enforcement section ID and rendering (shared with PR 1) Part of GPT 5.4 Enhancement v3 sprint. Tracking: #66345. Supersedes: #66379.
Greptile SummaryAdds a Confidence Score: 5/5Safe to merge; no runtime defects. Both findings are P2 style improvements. All remaining findings are P2: dead Set entries that are never evaluated, and a weaker-than-necessary return type that compiles correctly but loses the ProviderSystemPromptSectionId constraint. Neither affects runtime behavior or correctness. extensions/google/prompt-overlay.ts — minor typing and dead-code cleanup. Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/google/prompt-overlay.ts
Line: 2
Comment:
**Dead entries in `GOOGLE_PROVIDER_IDS`**
`"google-vertex"` and `"google-antigravity"` are registered as `hookAliases` on the `"google"` provider (see `provider-registration.ts` line 19), not as independent providers. Neither caller of `resolveGoogleSystemPromptContribution` ever passes those IDs as `modelProviderId` — both callers hardcode `"google"` or `PROVIDER_ID` (`"google-gemini-cli"`). The two alias entries in the Set are dead code and may mislead future readers into thinking they guard a real code path.
```suggestion
const GOOGLE_PROVIDER_IDS = new Set(["google", "google-gemini-cli"]);
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/google/prompt-overlay.ts
Line: 37-42
Comment:
**Weaker return type loses section-ID constraint**
The inline type `{ sectionOverrides: Record<string, string> }` accepts any string as a key. The actual contract (`ProviderSystemPromptContribution`) uses `Partial<Record<ProviderSystemPromptSectionId, string>>`, which constrains keys to the known union `"interaction_style" | "tool_call_style" | "execution_bias" | "tool_enforcement"`. With `Record<string, string>`, a typo like `"tool-enforcement"` silently produces no effect — TypeScript won't catch it inside this file.
Since `ProviderSystemPromptContribution` is not yet exported through `openclaw/plugin-sdk/*`, a local union is the simplest fix without crossing the boundary:
```ts
type SectionId = "interaction_style" | "tool_call_style" | "execution_bias" | "tool_enforcement";
export function resolveGoogleSystemPromptContribution(params: {
modelProviderId?: string;
modelId?: string;
}): { sectionOverrides: Partial<Record<SectionId, string>> } | undefined {
```
This keeps the extension boundary intact and restores compile-time safety for future edits.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(google): add Gemini execution guida..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
Adds a Google/Gemini provider prompt overlay so Gemini-family models receive Hermes-parity “execution discipline” guidance via a new overridable tool_enforcement system-prompt section.
Changes:
- Introduces
extensions/google/prompt-overlay.tswith Gemini operational directives and a resolver that returnssectionOverrides.tool_enforcement. - Wires
resolveSystemPromptContributioninto bothgoogleandgoogle-gemini-cliproviders. - Extends core system-prompt assembly to support a new provider section id:
tool_enforcement.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| extensions/google/prompt-overlay.ts | Adds Gemini execution guidance and a helper to conditionally contribute it to the system prompt. |
| extensions/google/provider-registration.ts | Hooks the new prompt contribution into the google provider registration. |
| extensions/google/gemini-cli-provider.ts | Hooks the new prompt contribution into the google-gemini-cli provider registration. |
| src/agents/system-prompt-contribution.ts | Adds tool_enforcement to the allowed provider section override ids. |
| src/agents/system-prompt.ts | Renders the new tool_enforcement overridable section in the assembled system prompt. |
Verify resolveGoogleSystemPromptContribution returns the expected tool_enforcement sectionOverrides for Google providers and returns undefined for non-Google providers.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39ca54b29c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Merge order: 3/8. See #66345 for the full merge sequence. |
| ...buildOverridablePromptSection({ | ||
| override: providerSectionOverrides.tool_enforcement, | ||
| fallback: [], | ||
| }), |
There was a problem hiding this comment.
The new tool_enforcement overridable section is now part of the core system prompt assembly, but there’s no corresponding unit test asserting that promptContribution.sectionOverrides.tool_enforcement is actually rendered (and positioned as expected) in buildAgentSystemPrompt(). Please add/extend tests in src/agents/system-prompt.test.ts similar to the existing tool_call_style override test to cover this new section ID.
| */ | ||
| export function resolveGoogleSystemPromptContribution(params: { | ||
| modelProviderId?: string; | ||
| modelId?: string; |
There was a problem hiding this comment.
resolveGoogleSystemPromptContribution accepts modelId but never uses it. Either use modelId to gate the overlay (if this is intended to apply only to Gemini/Gemma families), or drop the parameter from the function signature/callers/tests to avoid implying model-specific logic that isn't there.
| modelId?: string; |
| it("returns tool_enforcement sectionOverrides for Google provider with Gemini model", () => { | ||
| const result = resolveGoogleSystemPromptContribution({ | ||
| modelProviderId: "google", | ||
| modelId: "gemini-3.1-pro-preview", | ||
| }); | ||
| expect(result).toEqual({ | ||
| sectionOverrides: { | ||
| tool_enforcement: GOOGLE_GEMINI_EXECUTION_GUIDANCE, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
This test name implies the contribution is gated on modelId ("with Gemini model"), but resolveGoogleSystemPromptContribution currently ignores modelId and only checks the provider id. Rename the test to reflect the actual behavior (or add a separate assertion that modelId does/doesn’t affect the result) so the suite documents intent accurately.
| it("returns tool_enforcement sectionOverrides for Google provider with Gemini model", () => { | |
| const result = resolveGoogleSystemPromptContribution({ | |
| modelProviderId: "google", | |
| modelId: "gemini-3.1-pro-preview", | |
| }); | |
| expect(result).toEqual({ | |
| sectionOverrides: { | |
| tool_enforcement: GOOGLE_GEMINI_EXECUTION_GUIDANCE, | |
| }, | |
| }); | |
| it("returns tool_enforcement sectionOverrides for Google provider regardless of modelId", () => { | |
| const expected = { | |
| sectionOverrides: { | |
| tool_enforcement: GOOGLE_GEMINI_EXECUTION_GUIDANCE, | |
| }, | |
| }; | |
| const result = resolveGoogleSystemPromptContribution({ | |
| modelProviderId: "google", | |
| modelId: "gemini-3.1-pro-preview", | |
| }); | |
| expect(result).toEqual(expected); | |
| const differentModelResult = resolveGoogleSystemPromptContribution({ | |
| modelProviderId: "google", | |
| modelId: "not-a-gemini-model", | |
| }); | |
| expect(differentModelResult).toEqual(expected); |
|
Making room for other PR's. Gemini is low priority. |
GPT 5.4 Enhancement v3 — Final Sprint PR 2/3
Tracking: #66345
Supersedes: #66379 | Replaces accidentally-closed #67513
Rebased on current
main(incorporates356110c5provider-hooks refactor). Ports Hermes Agent's Google model operational directives.What this PR adds
Gemini operational directives (Hermes
prompt_builder.py:258-276)system-prompt.ts:600)Provider wiring
resolveSystemPromptContributionwired to bothgoogleandgoogle-gemini-cliproviderssectionOverrides.tool_enforcement(includestool_enforcementsection ID independently)Files changed (5)
extensions/google/prompt-overlay.ts(new)extensions/google/provider-registration.tsextensions/google/gemini-cli-provider.tssrc/agents/system-prompt-contribution.tssrc/agents/system-prompt.tsParity constraint
Hermes-verbatim text with documented tool-ID substitutions only.