golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

chore(#4727): name the tool-conversion helpers for the runtime that uses them (#4732)

Browse Source 
* chore(#4727): name the tool-conversion helpers for the runtime that uses them

GSD has had no Gemini runtime since #1928 removed it (Google sunset Gemini CLI
on 2026-06-18, shipped 1.8.0), yet two helpers were still named for it:

  claudeToGeminiTools    -> claudeToAntigravityTools
  convertGeminiToolName  -> convertAntigravityToolName

The sole consumer is convertClaudeAgentToAntigravityAgent, whose own comment
read "Map tools to Gemini equivalents (reuse existing convertGeminiToolName)".
Nothing named Gemini consumes them, because nothing named Gemini exists. The
new names follow the convention the file already sets with its neighbouring
Copilot pair, claudeToCopilotTools / convertCopilotToolName.

Zero behavior change. Every mapped VALUE is byte-identical, deliberately:

  read_file, write_file, replace, run_shell_command, glob,
  search_file_content, google_web_search, web_fetch, write_todos

Those are Gemini's built-in tool dialect and Antigravity genuinely speaks it.
This rename covers only the identifiers, which are the one part of the surface
that was GSD's choice rather than Google's contract.

Renamed in BOTH copies. CLAUDE.md labels bin/install.js "(generated)", but no
script emits it -- build:lib is tsc -p tsconfig.build.json and writes only
gsd-core/bin/lib/**. These converters are the #1099/#1173/#1182 situation: they
were extracted into src/runtime-artifact-conversion.cts while bin/install.js
kept its own working inline copies, so each symbol existed twice in two
independently hand-maintained files. Renaming one would have left two names for
one concept. Verified first that no capability descriptor resolves either by
name -- antigravity's descriptor names only convertClaudeCommandToAntigravitySkill
and convertClaudeAgentToAntigravityAgent, neither of which moved.

Comments keep their reasoning and their issue refs (#3362 AskUserQuestion,
#1394 Skill/SlashCommand); only the subject is corrected, from "Gemini CLI" to
Antigravity speaking the Gemini dialect. Those describe the dialect's behavior,
which is still Antigravity's behavior, so deleting them would destroy the record
of two real bugs.

docs/research/gemini-to-antigravity-migration.md is left unedited and carries a
dated addendum instead: it is pinned to c0b2a05d2f and quotes #1928's commit
message verbatim, so rewriting a citation to match a later tree would falsify a
primary source. ADR-1593's dimension-3 row is updated, because that table is a
present-tense index of which helper implements each dimension and would
otherwise name a symbol that no longer exists.

Coverage: the module is the only export surface -- bin/install.js exports none
of these four, its inline copies being module-private -- so the new assertion
targets gsd-core/bin/lib/runtime-artifact-conversion.cjs and checks the new keys
present, the old keys absent (no alias left behind), deepStrictEqual on the whole
map so an added or removed key fails, and each excluded input individually. The
installer's inline copy stays covered behaviorally by the existing test that
imports convertClaudeAgentToAntigravityAgent from bin/install.js.

Phase 4a of epic #4709. Refs #4727

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(#4727): fix three review blockers — ADR append-only, honest verdict, changeset

The isolated adversarial review returned BLOCK on three majors. All three were
right and all three are fixed here.

1. ADR-1593 was amended IN PLACE, violating docs/adr/README.md:5: "ADRs are
   append-only. Amendments extend existing ADRs with a dated section rather
   than replacing them." The dimension-3 row is restored to its original text
   and a dated "Amendment — 2026-09-14 (#4727)" section is appended instead.
   Worse than the policy breach: the previous commit applied OPPOSITE rules to
   two docs in one change, freezing the research doc's citations while
   rewriting the ADR's. Both now follow append-only.

2. The research-doc addendum claimed "the rename recommended in the PRESERVE
   table has landed" and that the "PRESERVE — trap" verdict "still holds".
   Neither is true. §2(c) is headed "MUST NOT be renamed" and the §6 table
   files both symbols under (c). The rename OVERTURNS that verdict, and the
   addendum now says so in those words: it is a NARROWING of (c), whose real
   subject is what Google owns -- the directories, the dialect, GEMINI.md, the
   model ids, and for these two symbols the mapped VALUES, which stay
   byte-identical. What (c) had swept in with them were two GSD-chosen
   identifiers no external contract references. Claiming endorsement from a
   verdict that forbade the change was the actual defect; the substantive
   argument was always sound.

   The addendum also now names the two superseded rows (~:55, ~:136) so a
   later reader is not misled, and discloses that §2(a)'s verbatim quotation
   (~:40) no longer matches its source: it reproduces the test docblock's old
   "convertGeminiToolName" wording, and #4727 reworded that docblock. The
   docblock had to move -- leaving it would make the #1928 guard describe a
   symbol that does not exist -- so the mismatch is disclosed rather than
   papered over by editing the quote, which is the one thing the addendum
   exists to avoid.

3. no-changelog was the wrong call. scripts/changeset/lint.cjs reports
   fail_missing_fragment because the diff touches src/ and bin/, and
   CONTRIBUTING.md:216 states src/ edits are user-facing "even though the
   generated .cjs is gitignored", with :224 adding "When unsure whether a
   change is user-facing, add the fragment." Confirmed concretely: gsd-core is
   in package.json's files array so the compiled module ships, and there is no
   exports map, so a consumer's deep require of
   gsd-core/bin/lib/runtime-artifact-conversion.cjs resolved
   .convertGeminiToolName before this change and gets undefined after. A
   Changed fragment is added. I had asserted "nothing user-facing" without
   running the repo's own changeset lint; the reviewer ran it.

Two review findings are accepted and recorded rather than fixed, both in the
research addendum's new §8: the bin/install.js half of the rename is
test-unprotected (that file exports none of the four identifiers and the export
audit asserts undefined for both spellings, so reverting it breaks no test --
its behavior is covered, its naming is not), and closing that needs the
repo-wide drift guard, which is #4729 and the last phase of the epic for
precisely this reason.

Refs #4727

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(#4727): backfill changeset pr number to 4732

Refs #4727

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#4727): assert both halves of the windsurf pre-write guard's contract

PR #4732's `conformance test (windows-latest, 24, shard 2/3)` failed on exactly
one assertion, and the base branch was green, so this is not inherited:

    tests/windsurf-hooks-bridge.test.cjs:104
    expected exit 2, got 0
    stderr: gsd-windsurf-pre-write: git probe 'git rev-parse --show-toplevel (cwd)' …
    duration_ms: 2909

Root cause, not a flake. hooks/gsd-windsurf-pre-write.js gives every git probe a
2000 ms budget (SPAWNOPT.timeout) and FAILS OPEN when a probe cannot run — its
own header states that outright, because "a hook bug must never wedge Cascade".
hooks/lib/git-probe.js (#3911) exists precisely because that budget is routinely
exceeded in CI; its header records a macOS run landing at 2084/2112/2177 ms,
just past the budget, and its reportIfUndetermined() announces the case on
stderr while deliberately changing no exit code.

So the hook has TWO documented outcomes:

  (a) probe resolved and the file's git root differs  -> BLOCK, exit 2
  (b) probe UNDETERMINED (timeout / spawn failure)   -> FAIL OPEN, exit 0,
                                                         announced on stderr

G1 and G1b asserted `status === 2` unconditionally, encoding only (a). They
therefore fail whenever the documented (b) occurs, which makes them
load-sensitive by construction. My diff did not touch that hook or its tests;
it re-packed the Windows chunks (6 -> 8 under #4737's derived cap), which raised
load enough to tip the 2 s budget and expose the latent assertion.

Both tests now branch on whether the hook ANNOUNCED an undetermined probe, and
assert the correct half in each case. This is not a loosened assertion:

  - undetermined -> exit 0 is REQUIRED. That arm still has teeth, because it
    fails if the hook ever blocks on a probe it could not determine, which is
    the dangerous direction — wedging the agent on a hook bug.
  - determined   -> the original exit 2 plus the original stderr-reason regex,
    unchanged.

The matcher is tied to reportIfUndetermined's exact message rather than a loose
/git probe/, and was proven against both strings: it matches the real
undetermined diagnostic and does NOT match a normal block message. No retry, no
sleep, no timing-dependent logic.

Deliberately NOT done: raising the hook's 2000 ms budget. That is forbidden here
without an explicit instruction, and it would only move the cliff rather than
fix the test's false premise.

Scope note: this is a test fix in a file unrelated to the rename, carried here
because it is what makes #4732 red. CLAUDE.md's no-deferral rule is explicit
that a defect found anywhere in the tree is fixed in the current change, and
that it overrides one-concern-per-PR.

Refs #4727

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-09-14 15:11:27 -04:00
committed by GitHub
parent 9b750dc00a
commit b647e28313
9 changed files with 233 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
docs/adr/1593-skill-mapping-converter-methodology.md
View File

@@ -95,3 +95,30 @@ GSD's first-party plugin/extension on every supported platform should both **pro
- **[ADR-766](766-claude-code-plugin-manifest-module.md)** (Claude plugin manifest, Accepted) — referenced for the provision methodology (the `skills` manifest field Phase B-provide / Phase D adds).
- **[ADR-1235](1235-descriptor-driven-agent-conversion-migration.md)** (descriptor-driven agent conversion) — complementary; its byte-parity transform-ordering rule is cited in Decision 3.
- **Epic [#1258](https://github.com/open-gsd/gsd-core/issues/1258)** — this is Phase A. Phase B-consume (PR #1261, merged) is the reference implementation canonized in Decision 5. Phases B-provide, C (C1–C6), D are tracked as separate issues per the epic's governance.
---
## Amendment — 2026-09-14 (#4727)
**Dimension 3's helper roster changed name, not behavior.** The table above is left as written
per `docs/adr/README.md`: *"ADRs are append-only. Amendments extend existing ADRs with a dated
section rather than replacing them."* Read the row through this amendment:
| Dimension 3 helper, as written above | now named |
|---|---|
| `convertGeminiToolName` | `convertAntigravityToolName` |
| (its backing table, `claudeToGeminiTools`) | `claudeToAntigravityTools` |
The methodology this ADR decided is unchanged: dimension 3 still maps Claude tool names to
runtime equivalents, through the same helper with the same mapping table and the same exclusion
set. Only the identifier moved, and only because GSD has had no Gemini runtime since #1928
(Google sunset Gemini CLI on 2026-06-18, removal shipped 1.8.0). The sole consumer was, and
remains, `convertClaudeAgentToAntigravityAgent`.
The mapped **values** are untouched and must stay so — `read_file`, `write_file`, `replace`,
`run_shell_command`, `glob`, `search_file_content`, `google_web_search`, `web_fetch`,
`write_todos` are Gemini's built-in tool dialect, which Antigravity genuinely speaks. That
dialect is Google's contract; the identifier was GSD's own choice. `ConverterName` (ADR-1016's
closed enum, and `VALID_CONVERTER_NAMES` in `gsd-core/bin/lib/capability-validator.cjs`) is
**not** affected: it admits only `convertClaude{Command,Agent}To*` names, and neither renamed
symbol was ever a member, so no capability descriptor's `converter` field changes.

68
docs/research/gemini-to-antigravity-migration.md
View File

@@ -310,3 +310,71 @@ The missing guard proposed in §5 was scoped to locale prose. It must also cover
runtime-loaded workflow text, and the cheapest durable form is to make the name policy
**fail loud** on a non-canonical id rather than defaulting to Claude Code — which would have
turned every site in §7.2 into a hard error on the day #1928 landed.
---
## 8. Addendum — 2026-09-14 (#4727): §2(c) narrowed, deliberately
Everything above §8 is pinned to `next` at `c0b2a05d2f` and is left byte-for-byte intact on
purpose: §1 quotes #1928's commit message verbatim and §2(a) quotes a test docblock verbatim, so
rewriting either to match a later tree would falsify a primary source. The delta is recorded here
instead.
### This overturns a verdict recorded above. Saying so plainly.
§2(c) is headed **"MUST NOT be renamed"**, and the §6 PRESERVE table files
`claudeToGeminiTools` / `convertGeminiToolName` under category (c) with the verdict
**"PRESERVE — trap"**. #4727 renamed them anyway:
| before | after |
|---|---|
| `claudeToGeminiTools` | `claudeToAntigravityTools` |
| `convertGeminiToolName` | `convertAntigravityToolName` |
That is a **narrowing of (c), not compliance with it**, and the earlier verdict was too broad
rather than wrong. (c)'s real subject is the part of the Gemini surface that Google owns — the
directories, the hook dialect, `GEMINI.md`, the model ids, and, for these two symbols
specifically, the **mapped values**:
```
read_file write_file replace run_shell_command glob
search_file_content google_web_search web_fetch write_todos
```
Those are Gemini's built-in tool names, Antigravity genuinely speaks that dialect, and they remain
byte-identical — the "trap" the table warned about is real and still stands for them. What (c) had
swept in along with them were two **GSD-chosen identifiers**, which no external contract references.
Renaming those breaks nothing and removes a name that had outlived its runtime by more than a year.
Superseded rows, named explicitly so a later reader is not misled: §2(c)'s
"Shared tool-name vocabulary" bullet (~`:55`) and the §6 PRESERVE table row for `bin/install.js`
(~`:136`) both cite `bin/install.js:1628-1720` under the old identifiers. The cited line range is
still correct; only the two names are now the Antigravity-prefixed ones.
### One citation above now reproduces superseded wording
§2(a) (~`:40`) quotes the `tests/gemini-runtime-removed.test.cjs` docblock verbatim, including the
phrase *"the shared convertGeminiToolName tool vocabulary"*. #4727 reworded that docblock to name
the live symbol, so **the quotation no longer matches its source**. Disclosed rather than silently
edited, because editing the quote is precisely what this addendum exists to avoid. The docblock had
to move: leaving it would have made the repo's #1928 guard describe a symbol that no longer exists.
### What was NOT renamed, and one coverage gap
Untouched: `~/.gemini/antigravity{,-ide,-cli}` and `~/.gemini` as their parent; `~/.gemini/config`
(#3738); `GEMINI.md` as `projectInstructionFile`; `hookEvents: "gemini"`; every `gemini-*` /
`google/gemini-*` model id and `providerPresets.google`; the `--gemini` **installer** flag, which
remains #1928's sunset redirect; the `GEMINI_CONFIG_DIR` launcher arm (epic #4632). `bin/install.js`
still carries 36 `gemini` references after the rename, which is the correct number.
The symbol existed **twice** — the extracted copy in `src/runtime-artifact-conversion.cts` (and that
module's `export =` block, added by #1182 as a dependency closure) plus a working inline copy in
`bin/install.js`. `CLAUDE.md` labels that file "(generated)", but no `package.json` script emits it;
`build:lib` is `tsc -p tsconfig.build.json` and writes `gsd-core/bin/lib/**` only. Both copies were
renamed, since renaming one would have left two names for one concept.
**Known gap:** the `bin/install.js` half is test-unprotected. That file exports none of the four
identifiers, and the export audit asserts `installer[name] === undefined` for both spellings, so
reverting that half would break no test — its behavior is covered, its *naming* is not. Closing that
requires the repo-wide drift guard (#4729), which is the last phase of this epic for exactly this
reason.