Skip to content

Improve readability issues - #2

Merged
NTaylorMullen merged 2 commits into
mainfrom
core
Apr 17, 2025
Merged

NTaylorMullen merged 2 commits into
mainfrom
core

Conversation

@rakyll

@rakyll rakyll commented Apr 17, 2025

Copy link
Copy Markdown
Contributor

This is only the first change of many changes.

  • Remove redundant autogenerated comments
  • Use the recommended file name style
  • Use camelCase for variable names
  • Don't introduce submodules for relevant types
  • Don't introduce constants like modules, these are implementation details
  • Remove empty files

@rakyll
rakyll requested a review from NTaylorMullen April 17, 2025 19:03
Comment thread packages/cli/src/tools/edit.tool.ts Outdated
This is only the first change of many changes.

* Remove redundant autogenerated comments
* Use the recommended file name style
* Use camelCase for variable names
* Don't introduce submodules for relevant types
* Don't introduce constants like modules, these are implementation details
* Remove empty files
@NTaylorMullen
NTaylorMullen merged commit ece8630 into main Apr 17, 2025
@rakyll
rakyll deleted the core branch April 21, 2025 23:27
yewenchen pushed a commit to yewenchen/gemini-cli that referenced this pull request Jul 8, 2025
hymjay0 added a commit to hymjay0/gemini-cli that referenced this pull request Jul 12, 2025
bishal-codepros pushed a commit to bishal-codepros/gemini-cli that referenced this pull request Jul 22, 2025
…vely function

- Combine initial abort check with depth check for efficiency
- Remove unnecessary abort check before fs.readdir (no async operations between checks)
- Add abort check after fs.readdir async operation where signal can actually change
- Remove abort check from inside for loop (synchronous operations only)
srithreepo pushed a commit that referenced this pull request Sep 4, 2025
…look for closingIssuesReferences instead of looking for the Issue ID in the body which can lead to wrong issues and labels being pulled in.
cocosheng-g added a commit to cocosheng-g/gemini-cli that referenced this pull request Oct 24, 2025
# This is the 1st commit message:

feat: Create BYOID auth client when detecting BYOID credentials

This change introduces a BYOID auth client that is created when
BYOID credentials are detected. It also refactors the code assist
server and setup to use the new AuthClient type.

# This is the commit message google-gemini#2:

resolve comments

# This is the commit message google-gemini#3:

load credentials only once and lazy load the client

# This is the commit message google-gemini#4:

use original method names

# This is the commit message google-gemini#5:

use original method names for getOauthClient

# This is the commit message google-gemini#6:

remove redundant test mock
aka76bm referenced this pull request in aka76bm/gemini-cli Oct 29, 2025
…m_and_yarn-fd296dbd23

chore(deps): bump vite from 7.1.9 to 7.1.11 in the npm_and_yarn group across 1 directory
A1cy added a commit to A1cy/HiveCodeCli that referenced this pull request Oct 29, 2025
CRITICAL SECURITY FIX - Removed publicly exposed AWS credentials

Files Changed:
- BEDROCK_SETUP.md: Replaced real credentials with placeholder examples
- QUICK_START.md: Replaced real credentials with placeholder examples

Security Impact:
- Removed AWS_ACCESS_KEY_ID: BedrockAPIKey-mzur-at-124737196430
- Removed AWS_SECRET_ACCESS_KEY (base64 encoded)

Action Required:
⚠️ USER MUST ROTATE AWS BEDROCK API KEYS IMMEDIATELY
⚠️ These credentials were exposed in commit 3ef2c7c
⚠️ Go to AWS Console → IAM → Bedrock API Keys → Rotate Keys

This commit removes the exposed secrets from documentation files.
Users should now use placeholder examples and replace with their own keys.

Fixes: GitHub Secret Scanning Alert google-gemini#2
SeunghyunLee1982 referenced this pull request in SeunghyunLee1982/gemini-cli May 17, 2026
Adds a persistent agent swarm primitive behind the `experimental.swarm`
settings flag. The main Gemini agent can now spawn long-lived Claude
sub-agent instances, send them multiple messages across orchestrator
turns (retaining `messages` state), and release them — going beyond the
existing one-shot `kind: anthropic` AgentTool delegation.

Scope (v1.0):
- Sync only. `message` blocks until the session's turn completes.
- In-memory `SwarmManager` singleton; sessions die with the process.
- Single discriminated `swarm` tool with `action: spawn | message |
  release | list`. Zod-backed discriminated union.
- Read-only tool whitelist by default (`read_file`, `grep_search`,
  `glob`, `list_directory`, `read_many_files`); per-spawn `tools`
  override allowed.
- Session abort bound to `Config.getAppAbortSignal()` (SIGINT / process
  exit), NOT the orchestrator turn signal — ending an orchestrator turn
  does not kill the swarm.
- 30-minute idle TTL with background sweep; sessions wedged in `running`
  past `2 * TTL` are aborted, marked `error`, and kept in `list()` for
  user debugging.
- Feature-flagged off by default.

The Anthropic message loop body from `AnthropicAgentInvocation` was
extracted into `anthropic-loop.ts` so both the v1 single-shot path and
the new long-lived `SwarmSession` share the same tool-use machinery.

Design + reviews:
- Multi-turn design discussion: `design-loop/swarm-design.md` (Claude +
  Gemini, Turns 0-4 + Final Synthesis with the locked v1.0 acceptance
  E2E).
- Phase 1 reviews: `design-loop/phase1-review-opus.md`,
  `design-loop/phase1-review-gemini.md`.
- Phase 2 reviews: `design-loop/phase2-review-opus.md`,
  `design-loop/phase2-review-gemini.md`.

Phase 2 review items addressed in this commit:
- Gemini #1: TTL sweep no longer deletes stuck-RUNNING sessions —
  they're aborted + marked ERROR + kept in `list()`.
- Opus #1/#3: per-session app-abort listeners now have a disposer that
  fires from `SwarmSession.release()` and from the TTL-stuck path; no
  more unbounded listener accumulation across spawn/release cycles.
- Opus #2: `appAbortDisposers` (Config) and `appAbortDisposer`
  (SwarmManager) are nulled out after firing for clarity.
- Opus #4: `lastActiveAt` is stamped on RUNNING entry so a legitimate
  35-minute turn is not misjudged "stuck" by the 2 * TTL sweep.

Acceptance gate:
- The locked stateful-continuity E2E is in
  `packages/core/src/agents/swarm/swarm-continuity.test.ts`. It
  intercepts the Anthropic SDK at the module boundary and asserts that
  the THIRD `messages.create` call (Agent A's second turn) sees the
  prior user+assistant turns in its `messages` array. If session
  statelessness regresses (e.g. someone resets `messages = []`), the
  test fails.

Validation gates passed:
- `npm run typecheck` (all workspaces)
- `node scripts/lint.js --eslint`
- `node scripts/lint.js --prettier`
- `npm run build --workspace=packages/core`
- `npm run build --workspace=packages/cli`
- `npx vitest run packages/core/src/agents/` (753 passed)
- `npx vitest run packages/core/src/agents/swarm/` (18 passed)

Deferred to v1.1+ (per the synthesis):
- `async: true` spawn + `poll` / `await` actions.
- Shared workspace dir at `~/.gemini/swarm/<session_id>/work/`.
- Budget guardrails (`max_spend_usd`, `max_total_tokens`).
- Cross-process persistence.
- Haiku worker reintegration (user policy default stands).

Co-Authored-By: Claude Opus 4.7 <[email protected]>
SeunghyunLee1982 referenced this pull request in SeunghyunLee1982/gemini-cli May 19, 2026
…overy

Closes the gap between v1.0's "process pool of single-shot workers" and the
intended "stateful collaborator team with smart orchestrator" model. R3-locked
design after a three-round multi-model debate (see design-loop/), then
implemented and put through an Opus + Gemini reviewer loop before commit.

Area 1 — `role` / `charter`. Optional short labels (≤80 / ≤200 chars) on
`spawn` that flow through the Zod schema, JSON tool schema, session params,
`list()`, and the new `swarm_status` snapshot. They also get woven into the
session's composed system prompt so the sub-agent knows what hat it's wearing
without the orchestrator having to repeat itself in every message.

Area 2 — `message_turn_cap_reached` status. Hitting `max_turns` no longer
appends a `[Note: hit max_turns=N.]` suffix and silently returns; it's now a
first-class outcome. `runAnthropicMessageLoop` returns
`{ text, capReached: boolean }`. `SwarmResult.message` splits into:
  - `status: 'ok' | 'message_turn_cap_reached'` — message outcome
  - `session_status: SwarmSessionStatus` — session lifecycle (idle/running/
    released/error)
Cap-reached is a normal IDLE return; the session lives. The single-shot
anthropic-invocation path is bit-compatible (silently drops `capReached`).

Area 3 — Plan-mode tool filter. `SWARM_BLOCKED_TOOL_NAMES` (containing
`ENTER_PLAN_MODE_TOOL_NAME` / `EXIT_PLAN_MODE_TOOL_NAME` imported directly
from `tool-names`) is applied in the per-tool clone loop, so both inherit-all
and explicit `tools: [...]` paths drop mode-control tools. Closes the
host-CLI-state-escalation hole the user surfaced in the Phase 4 live test
where a sub-agent inherited `exit_plan_mode` and burned its turn budget
trying to escape Plan Mode.

Area 4 — `swarm_status` companion tool. New `swarm-status-tool.ts`
(`Kind.Other`, no args, no confirm) that calls
`SwarmManager.getSwarmStatusSnapshot()`. The snapshot exposes every live
session's `role` / `charter` / `status` / `turn_count` /
`seconds_since_active`, the shared `workspace_dir`, and a newest-first
`recent_events[]` ring (50 entries, fed by `publishActivity`). Sub-agents
are deliberately handed this read-only window — the verb surface
(`swarm spawn/message/release`) stays orchestrator-only behind `Kind.Agent`.

Area 5 — auto system-prompt block + default tool. `SWARM_PROTOCOL_BLOCK` is
woven into `composedSystemPrompt` between the tool advertisement and the
soft-reject suffix; it tells the sub-agent it's in a swarm, to call
`swarm_status()` for peer context, and to append a `[<agent_id> @ <ts>]`
line to `<workspace_dir>/state.md` after substantive work. `swarm_status` is
appended to `DEFAULT_SWARM_TOOLS` and auto-injected on the inherit-all path
(explicit lists are respected).

Area 6 — SKILL.md adds four sections covering self-discovery via
`swarm_status`, the `state.md` convention, "release on role-exhaustion, not
task-completion", and cap-handling. CLAUDE.md's Swarm section is updated for
the v1.0.x shape.

Tests (6 mandated, all present and exercising the real surface, not theater):
  - Plan-mode filter regression with explicit list
  - role/charter round-trip through list() + swarm_status snapshot
  - cap-reached returns IDLE session_status, turnCount still increments
  - `getSwarmStatusSnapshot` agents[] + recent_events ordering
  - composed system prompt contains protocol block + role + charter
  - anthropic-loop `{ capReached: true }` shape on max_turns

Post-review tweaks (Opus #2/#3 follow-ups, addressed before commit):
  - SWARM_BLOCKED_TOOL_NAMES now references the tool-name constants directly
    (no drift if those tools are ever renamed)
  - SWARM_STATUS_TOOL_NAME hoisted to types.ts as single SoT; swarm-manager
    and swarm-status-tool both import from there (cycle-free)

Out of scope (deferred per user):
  - agent-memory MCP integration (v1.2+; RAG-style retrieval + write-back)
  - Persistent .events.jsonl on disk (v1.1+; current ring is in-memory only,
    consistent with "sessions die with parent CLI" policy)
  - `callerAgentId` resolution from MessageBus name (v1.1; today every
    sub-agent sees `self_agent_id: undefined`, which is benign)

Rejected design directions (recorded in design-loop/swarm-model-r*.md):
  - `SwarmArchetype` registry + `catalog()` action (Gemini R1 → moved off in R2)
  - `since_last_turn` / `context_update` orchestrator-push fields (user
    redirection in R3: "공유 메모리 + self-discovery 방향으로")
  - `suggest_release` hint from sub-agent (user: "안 넣으면 문제 되는 명확한
    시나리오 안 보이면 빼자")

Validation:
  npm run typecheck                          — pass
  node scripts/lint.js --eslint              — pass
  node scripts/lint.js --prettier            — pass for staged files
  npm run build --workspace=packages/core    — pass
  npm run build --workspace=packages/cli     — pass
  npx vitest run packages/core/src/agents/   — 766 passed / 1 skipped
  npx vitest run packages/core/src/scheduler — 146 passed
  npx vitest run packages/core/src/policy    — 326 passed (2 pre-existing
    topic-policy failures unrelated to this commit; verified clean on
    f114440 before this work)

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
SeunghyunLee1982 referenced this pull request in SeunghyunLee1982/gemini-cli May 19, 2026
Closes a real UX gap surfaced in the user's live testing: the
orchestrator (Gemini model) doesn't naturally reach for the `swarm`
tool, even when registered, even with explicit prompt steering. It
falls back to `run_shell_command` and tries `gemini swarm ...`
recursively. The fork ships substantial swarm + policy machinery
already (Phases 4–6), but the model-side disposition layer was missing.

R1/R2 multi-model design debate (Opus + Gemini in parallel, two
rounds) settled the design at `design-loop/swarm-orchestrator-
disposition.md`. After implementation, a six-reviewer pass (three
angles — TS/clean-code, intent, tests/abstractions — each with one
Opus + one Gemini reviewer) ran in parallel; all six returned SHIP or
SHIP-WITH-FOLLOW-UP. Cheap follow-ups are addressed in this commit;
the rest are noted in the design doc and the agent-memory entry for
v2 pickup.

Four layers, all gated on `Config.isSwarmEnabled()` so upstream
non-swarm users see zero behavior change:

1) Tool description rewrite (~+150 tokens).
   `SWARM_TOOL_DESCRIPTION` and `SWARM_STATUS_TOOL_DESCRIPTION` now
   carry explicit "when to use" + the anti-pattern "DO NOT shell
   out to `gemini` — there is no `gemini swarm` verb." Drift-guard
   tests pin the anti-pattern phrasing so accidental rewording
   regresses behavior loudly.

2) Orchestrator system-prompt disposition block + single inline
   worked example (~+300 tokens, conditional).
   New `SwarmDispositionOptions` + `renderSwarmDisposition` in
   `prompts/snippets.ts`. Inserted between `renderSubAgents` and
   `renderAgentSkills` in `getCoreSystemPrompt`. `promptProvider.ts`
   gates on `(SWARM_TOOL_NAME ∨ SWARM_STATUS_TOOL_NAME) ∈ registered
   tools` AND `isSwarmEnabled()`. The block names the tool, flags
   the no-CLI-verb anti-pattern, references `swarm_status`, and
   embeds one `<example>` showing a `swarm spawn` call. The example
   is inline rather than separate so in-context proximity isn't
   diluted by intervening prompt sections.

3) `swarm-collaboration` skill auto-inline (~+1.6k tokens, only
   when swarm enabled). `promptProvider.ts` filters the skill out
   of the regular `<available_skills>` manifest and inlines its
   SKILL.md body via new `renderSwarmInline`. Removes the
   activate-skill indirection so the protocol (state.md
   convention, release rules, paste-verbatim discipline) lands in
   the orchestrator's context without an extra round-trip.
   Hardcoded for the single fork-builtin inline candidate; an
   `inline: boolean` metadata flag was explicitly rejected in R2
   (YAGNI; the LOCKED v2 north star reserves the capability/
   template data-model surface).

4) Runtime guard — tier-1 default-deny `PolicyRule`. Registered
   from `config.ts` inside the existing `isSwarmEnabled()` tool-
   registration block. Pattern matches the JSON-stringified args
   form (`stableStringify(toolCall.args)`), NOT raw shell text —
   that ground-truth correction was caught in R2 when both models
   independently traced `PolicyEngine.matchRule` and found the
   raw-shell prototype from R1 would silently never match. The
   correct shape is `/"command":"(gemini|gemini-fork)(\s|"|\\)/`;
   the policy engine's existing sub-command splitter
   (`policy-engine.ts:469-475`) handles `bash -c "gemini foo"` and
   `cd /tmp && gemini ...` via recursive `check()`. The deny
   message carries the redirect text. `tier-4` user policies can
   still override.

Review-cycle fixes folded in before commit:

- Legacy snippets gap (Opus angle 1 #1): `snippets.legacy.ts`
  now imports the Phase 8 option types and renderers from
  `snippets.ts` and wires them into legacy `getCoreSystemPrompt`,
  so Gemini 2.x orchestrators with swarm enabled get the
  disposition block too. The disposition fix matters more for
  older models with stronger shell-first priors, not less.
- Misleading test narration (Opus angles 2 + 3, cross-flagged):
  `policy-engine.test.ts` comment for the `bash -c "gemini help"`
  case had the wrong mechanism. The DENY actually fires via the
  sub-command splitter recursing into the inner `gemini help`,
  not the JSON-escape `(\\)` alternative at the top level.
  Corrected.
- `swarm_status`-only branch test gap (Opus angle 3): added a
  case proving the disposition AND auto-inline both fire when
  only `SWARM_STATUS_TOOL_NAME` is registered (sub-agents
  themselves get `swarm_status` without `swarm` per the
  recursion-guard filter in `swarm-manager.ts`).
- Empty-body edge case (Gemini angle 3): `renderSwarmInline`
  now returns `''` for whitespace-only bodies so a future skill
  loader returning an empty string doesn't render a dangling
  `# Skill — <name> (auto-loaded)` header with no content.
  Test added.

Non-blocking review findings deferred to follow-ups (recorded in
the design doc / agent-memory entry):

- `SWARM_TOOL_NAME` SoT split (Opus angle 1 #3, Gemini angle 1):
  `SWARM_STATUS_TOOL_NAME` lives in lightweight `agents/swarm/
  types.ts`, while `SWARM_TOOL_NAME` is still in heavy
  `swarm-tool.ts`. Consolidating both into the central
  `tools/tool-names.ts` is a separate cleanup.
- Phase 8 prompt fields bypass `withSection` (Opus angle 1 #2):
  the operator `GEMINI_PROMPT_<KEY>=0` mute knob doesn't apply
  to the new sections. Routing through `withSection` is a
  separate consistency fix.
- Layer 4 brittleness on `stableStringify` (Gemini angle 2):
  the argsPattern coupling to the policy engine's stringify
  format is a structural smell. A dedicated structured-arg
  matcher in the policy engine is a v2-level improvement.

Tests landed (11 new):
- `policy-engine.test.ts`: JSON-shape pattern denies `gemini …`,
  `gemini-fork …`, and `bash -c "gemini help"` (via recursive
  splitter); passes through `ls -la` and `echo gemini`.
- `swarm-tool.test.ts`: anti-pattern drift guards on both tool
  descriptions.
- `promptProvider.test.ts`: disposition block on/off; auto-
  inline pulled out/in manifest; only-`swarm_status` branch
  fires both layers; `renderSwarmInline` empty-body returns ''.
- `config.test.ts`: tier-1 deny rule registered iff swarm
  enabled; carries the expected `source`, `toolName`,
  `decision`, `argsPattern` shape, and `denyMessage`.

Gates:
  npm run typecheck                          — pass
  node scripts/lint.js --eslint              — pass
  node scripts/lint.js --prettier            — pass for staged
                                                files; design-loop
                                                docs auto-formatted
                                                by pre-commit hook
  npm run build --workspace=packages/core    — pass
  npm run build --workspace=packages/cli     — pass
  npx vitest run packages/core/src/policy/   — 330/332 (the 2
                                                pre-existing
                                                topic-policy.test.ts
                                                failures, unrelated,
                                                verified on parent
                                                commit f190547)
  npx vitest run packages/core/src/agents/swarm/  — 36/36
  npx vitest run packages/core/src/prompts/  — 65/65

Files committed include the LOCKED design doc
(`design-loop/swarm-orchestrator-disposition.md`) and the
`.gitignore` exception that tracks it (alongside
`swarm-north-star.md`).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@sripasg sripasg added the size/l A large sized PR label Jun 2, 2026
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 18, 2026
Resolves all 7 review comments from gemini-code-assist[bot]:

google-gemini#1 [CRITICAL] write-todos.ts - Replaced clear+recreate with reconciliation
   strategy that matches existing tasks by title, preserving IDs, types,
   parent-child relationships, and dependencies. Only status is updated
   for matched tasks. New items are created, absent items removed. Epics
   and parent tasks created by dedicated tracker tools are never deleted.

google-gemini#2 [SECURITY-HIGH] trackerService.ts - Added path traversal guard:
   deleteTask() now validates ID matches /^[0-9a-f]{6}$/i before any
   filesystem operation.

google-gemini#3 [HIGH] trackerService.ts - Moved child-task deletion guard from tool
   layer into TrackerService.deleteTask() so all callers (tools, SDK,
   tests) get the same referential integrity protection.

google-gemini#4 [HIGH] trackerService.ts - Cascade dependency cleanup now sets
   updatedAt on affected tasks so disk metadata stays consistent.

google-gemini#5 [HIGH] trackerService.ts - createTask() now retries up to 10 times
   if the generated 6-char hex ID collides with an existing task.

google-gemini#6 [HIGH] trackerService.ts - deleteTask() now clears parentId on any
   task that referenced the deleted task as parent, preventing orphaned
   invisible tasks in the UI.

google-gemini#7 [HIGH] trackerTools.ts - TrackerDeleteTaskInvocation.execute() now
   trims and validates the ID parameter with the same regex before
   calling the service, catching bad input at the tool boundary.

Tests: 49 passed (added 4 new: path traversal guard, child-task block
at service layer, updatedAt cascade, ID reconciliation preservation).
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 18, 2026
Addresses all 11 review comments from gemini-code-assist[bot]:

Security (CRITICAL + HIGH):
- Add validateToolParamValues to ASTSearchTool with path validation
  using resolveDefensiveToolPath + resolveToRealPath + validatePathAccess,
  preventing path traversal attacks (comments google-gemini#1, google-gemini#2, google-gemini#8, google-gemini#9)

Parsing correctness (HIGH):
- findClosingBrace: track parenthesis depth to ignore braces inside
  inline object types in function params (comment google-gemini#3)
- findClosingBrace: return lines.length-1 instead of startLine+50
  when brace matching fails, for honest boundary reporting (comment google-gemini#7)
- extractSymbols: track block comment state (/* ... */) to skip
  commented-out code declarations (comment google-gemini#4)
- findIndentEnd: track Python triple-quoted strings to avoid
  early termination on docstrings with low indentation (comment google-gemini#5)
- extractSymbols: fix enum body skipping, enums now advance the
  line pointer past their body like classes/functions (comment google-gemini#10)

Documentation:
- collectSourceFiles: document .gitignore/.geminiignore limitation
  and plan for FileDiscoveryService integration (comment google-gemini#6)
- findClosingBrace: document escaped-quote limitation in the
  string-literal stripping regex (comment google-gemini#11)
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 18, 2026
Fixes 5 comments from third review round:

CRITICAL/HIGH (google-gemini#1,google-gemini#2): Map scope path traversal - file_path for "map"
  scope now goes through resolveDefensiveToolPath + validatePathAccess
  before reaching getCodebaseMap. handleMapScope accepts safePath param.

HIGH (google-gemini#3): Block comment stripping rewritten from state-tracking to
  character-level replacement. New stripBlockComments() replaces comment
  content with spaces, preserving line numbers. Handles mid-line comments
  and multi-line blocks correctly.

HIGH (google-gemini#4): validateToolParamValues now trims symbol_name and file_path
  at the top before any checks, preventing whitespace-only values.

HIGH (google-gemini#5): handleSymbolScope accepts trimmed symbolName as a parameter
  instead of reading raw this.params.symbol_name, so LLM-injected
  whitespace does not cause lookup mismatches.
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 18, 2026
HIGH (google-gemini#1): extractSymbols now passes cleaned (block-comment-stripped)
  lines to findIndentEnd, findClosingBrace, and extractMembers instead
  of raw lines. Prevents commented-out braces or members from corrupting
  boundary detection.

HIGH (google-gemini#2): findClosingBrace fallback changed from lines.length-1 to
  startLine. When brace matching fails (template literals, regex literals,
  syntax errors), returning startLine localizes the failure to one symbol
  instead of skipping every subsequent declaration in the file.
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 18, 2026
google-gemini#1 [SECURITY-CRITICAL] tool-names.ts - Removed write_todos->tracker_list_tasks
   alias. write_todos is a state-modifying tool; aliasing it to a read-only
   tool would let read-only auto-approve policies bypass write protection.

google-gemini#2 [HIGH] trackerService.ts - Moved fs.unlink() AFTER reference cleanup in
   deleteTask(). If a crash occurs mid-operation, the DB stays consistent:
   orphaned dep/parent references would permanently break validateCanClose
   and validateNoCircularDependencies on the affected tasks.

google-gemini#3 [HIGH] tool-names.test.ts - Removed test for the deleted alias.

48 tests passing. Session 1+2 CLI E2E verified.
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 18, 2026
…n (round-6)

google-gemini#1 [HIGH] write-todos.ts - Replaced O(N) array scans with Map-indexed
   O(1) lookups: tasksById for getDepth(), childrenByParent for
   hasPreservedDescendants(). Added visited.delete(id) backtracking
   so sibling subtrees are traversed independently.

google-gemini#2 [HIGH] trackerService.ts - Normalized ID to lowercase in both
   getTask() and deleteTask() before filesystem operations, preventing
   ENOENT on case-sensitive filesystems (Linux) when ID contains
   uppercase hex chars (e.g. 'ABCDEF' vs 'abcdef.json').

48 tests passing. Session 1+2 CLI E2E verified.
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 18, 2026
HIGH (google-gemini#1): stripBlockComments rewritten with full state-tracking parser.
  Now correctly ignores block comment delimiters inside string literals
  (single/double/template quotes with escape handling) and single-line
  comments (// ...). Prevents false positives like const x = "/*" from
  triggering comment stripping.

HIGH (google-gemini#2): Top-level indentation limit for brace-based languages raised
  from 2 to 8 spaces. Supports 4-space indented codebases and symbols
  nested inside namespaces/modules without being silently skipped.
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 18, 2026
google-gemini#1 [HIGH] write-todos.ts:154 - createTask not wrapped in try-catch;
   a single failure (validation/disk I/O) aborts the entire write_todos
   operation. Now catches errors as warnings so reconciliation continues.

google-gemini#2 [HIGH] trackerService.ts:212 - deleteTask ID validation used !id
   instead of typeof check; a non-string value would pass the regex
   (via implicit coercion) but crash on .toLowerCase(). Now matches
   getTask's robust typeof id !== 'string' guard.

48 tests. Session 1+2 verified.
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 18, 2026
HIGH (google-gemini#1): String literal regex in findClosingBrace now handles escaped
  quotes via negated character class with backslash alternation:
  /(TICK)(?:[^(TICK)\\]|\\.)*TICK/g pattern for all three quote types.
  Resolves the documented limitation for strings like "a \" {".

HIGH (google-gemini#2): Triple-quote tracking in findIndentEnd rewritten to track
  the specific opener (""" vs TICK TICK TICK). A block started with """
  can only be closed by """, preventing cross-type toggle bugs.

HIGH (google-gemini#3): Directory walk depth limit raised from 6 to 15. Supports
  deep monorepo structures like packages/core/src/tools/definitions/
  model-family-sets/ (depth 7) without silently omitting files.
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 18, 2026
CRITICAL (google-gemini#1): getFileOutline now validates resolved path is a subpath
  of targetDir before any fs access. Traversal returns null.

CRITICAL (google-gemini#2): getCodebaseMap validates searchDir stays within targetDir.
  Traversal returns an error string instead of walking arbitrary dirs.

HIGH (google-gemini#3,google-gemini#4): stripBlockComments now accepts language parameter.
  Uses # for Python line comments, // for others. String literal
  contents are blanked (replaced with spaces) while preserving
  delimiters, preventing false keyword matches inside strings and
  incorrect brace counting from template literal contents.
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 18, 2026
Review fixes:
- HIGH (google-gemini#1): Python member regex removed leading \s+ since trimmed
  lines have no leading whitespace. Methods now extracted correctly.
- HIGH (google-gemini#2): getCodebaseMap uses resolvedTargetDir for path.relative
  instead of raw this.targetDir, preventing incorrect relative paths.
- HIGH (google-gemini#3): Python test expanded to assert class method children
  (Handler.run as method child).

Bugs found via thorough manual testing:
- extractSymbols indent check now uses original lines[i] instead of
  cleaned[i]. Block comment blanking inflates indentation of the
  cleaned line (e.g. "/* comment */ export class A" becomes 19 spaces
  of indent), causing false top-level rejection.
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 18, 2026
HIGH (google-gemini#1): getFileOutline now checks file size via fs.stat before
  reading. Files over 2MB (minified bundles, logs, db dumps) are
  rejected with null return, preventing OOM or event loop blocking.

HIGH (google-gemini#2): stripBlockComments resets single-char inString (single
  and double quotes) at end of each line. An unclosed quote from a
  syntax error or unescaped character no longer bleeds into subsequent
  lines and corrupts the rest of the file. Multi-line delimiters
  (backtick, triple quotes) intentionally persist across lines.
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 18, 2026
HIGH (google-gemini#1): Single-line comment text now blanked with spaces in
  stripBlockComments (only // or # prefix preserved). Prevents
  false declaration matches from keywords inside trailing comments
  like "const x = 1; // class MyClass".

HIGH (google-gemini#2): handleSymbolScope uses recursive findSymbolRecursive
  helper instead of shallow flatMap. Correctly locates symbols
  nested more than 1 level deep (methods inside nested classes,
  symbols inside namespace/module blocks).
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 19, 2026
HIGH (google-gemini#1): findClosingBrace now strips regex literals alongside string
  literals. Patterns like /[{}]/ and /a{1,3}/ no longer corrupt brace
  depth tracking. Added regex alternation to the existing strip regex.

HIGH (google-gemini#2): collectSourceFiles optimized for large repos. Entries sorted
  alphabetically at each directory level for deterministic traversal.
  Hard cap of 10,000 files prevents OOM in huge monorepos. Per-level
  sorting means the walk produces near-sorted output, reducing final
  sort overhead.
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 19, 2026
…e getFileOutline call

Addresses round-16 gemini-code-assist review (HIGH x 1):

handleSymbolScope previously called findSymbolBounds (which internally
calls getFileOutline) and then called getFileOutline again to retrieve
symbol metadata, resulting in the same file being read from disk and
parsed with regex heuristics twice per symbol lookup.

Fix: replace the two-call pattern with a single getFileOutline call,
then use recursive search on the returned outline to find the target
symbol. Both bounds and metadata (kind, signature) are now extracted
from the same parse result.

Call chain before (2 file reads):
  handleSymbolScope -> findSymbolBounds -> getFileOutline (read google-gemini#1)
                    -> getFileOutline (read google-gemini#2) -> findSymbolRecursive

Call chain after (1 file read):
  handleSymbolScope -> getFileOutline (read google-gemini#1) -> findSymbolRecursive

Runtime verified: gemini-cli headless session confirms ast_search in
registered tool list alongside read_file, grep_search, glob, etc.
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 19, 2026
Resolves all 7 review comments from gemini-code-assist[bot]:

google-gemini#1 [CRITICAL] write-todos.ts - Replaced clear+recreate with reconciliation
   strategy that matches existing tasks by title, preserving IDs, types,
   parent-child relationships, and dependencies. Only status is updated
   for matched tasks. New items are created, absent items removed. Epics
   and parent tasks created by dedicated tracker tools are never deleted.

google-gemini#2 [SECURITY-HIGH] trackerService.ts - Added path traversal guard:
   deleteTask() now validates ID matches /^[0-9a-f]{6}$/i before any
   filesystem operation.

google-gemini#3 [HIGH] trackerService.ts - Moved child-task deletion guard from tool
   layer into TrackerService.deleteTask() so all callers (tools, SDK,
   tests) get the same referential integrity protection.

google-gemini#4 [HIGH] trackerService.ts - Cascade dependency cleanup now sets
   updatedAt on affected tasks so disk metadata stays consistent.

google-gemini#5 [HIGH] trackerService.ts - createTask() now retries up to 10 times
   if the generated 6-char hex ID collides with an existing task.

google-gemini#6 [HIGH] trackerService.ts - deleteTask() now clears parentId on any
   task that referenced the deleted task as parent, preventing orphaned
   invisible tasks in the UI.

google-gemini#7 [HIGH] trackerTools.ts - TrackerDeleteTaskInvocation.execute() now
   trims and validates the ID parameter with the same regex before
   calling the service, catching bad input at the tool boundary.

Tests: 49 passed (added 4 new: path traversal guard, child-task block
at service layer, updatedAt cascade, ID reconciliation preservation).
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 19, 2026
google-gemini#1 [SECURITY-CRITICAL] tool-names.ts - Removed write_todos->tracker_list_tasks
   alias. write_todos is a state-modifying tool; aliasing it to a read-only
   tool would let read-only auto-approve policies bypass write protection.

google-gemini#2 [HIGH] trackerService.ts - Moved fs.unlink() AFTER reference cleanup in
   deleteTask(). If a crash occurs mid-operation, the DB stays consistent:
   orphaned dep/parent references would permanently break validateCanClose
   and validateNoCircularDependencies on the affected tasks.

google-gemini#3 [HIGH] tool-names.test.ts - Removed test for the deleted alias.

48 tests passing. Session 1+2 CLI E2E verified.
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 19, 2026
…n (round-6)

google-gemini#1 [HIGH] write-todos.ts - Replaced O(N) array scans with Map-indexed
   O(1) lookups: tasksById for getDepth(), childrenByParent for
   hasPreservedDescendants(). Added visited.delete(id) backtracking
   so sibling subtrees are traversed independently.

google-gemini#2 [HIGH] trackerService.ts - Normalized ID to lowercase in both
   getTask() and deleteTask() before filesystem operations, preventing
   ENOENT on case-sensitive filesystems (Linux) when ID contains
   uppercase hex chars (e.g. 'ABCDEF' vs 'abcdef.json').

48 tests passing. Session 1+2 CLI E2E verified.
dylanyunlon added a commit to dylanyunlon/gemini-cli that referenced this pull request Sep 19, 2026
google-gemini#1 [HIGH] write-todos.ts:154 - createTask not wrapped in try-catch;
   a single failure (validation/disk I/O) aborts the entire write_todos
   operation. Now catches errors as warnings so reconciliation continues.

google-gemini#2 [HIGH] trackerService.ts:212 - deleteTask ID validation used !id
   instead of typeof check; a non-string value would pass the regex
   (via implicit coercion) but crash on .toLowerCase(). Now matches
   getTask's robust typeof id !== 'string' guard.

48 tests. Session 1+2 verified.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/l A large sized PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants