39e4aa1976
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
49 lines
10 KiB
Markdown
49 lines
10 KiB
Markdown
# Phase 11: Miscellaneous Bug Fixes
|
|
|
|
Standalone bug fixes that don't group neatly into other phases.
|
|
|
|
## Parser Fixes
|
|
|
|
- [x] Review PR #835 (`fix: handle nested XML tags in parser extractField and extractArrayElements` by @Glucksberg). File: `src/sdk/parser.ts`. Nested XML tags break field extraction. Steps: (1) `gh pr checkout 835` (2) Review regex/parser changes — should handle `<field><inner>content</inner></field>` patterns (3) Run `npm run build` (4) If correct: `gh pr merge 835 --rebase --delete-branch`
|
|
- **Merged.** Clean regex fix: `[^<]*` → `[\s\S]*?` (non-greedy) in `extractField()` and `extractArrayElements()`. Also adds empty element filtering. Greptile 5/5 confidence. Build failure is pre-existing (dompurify dep), not introduced by this PR. TypeScript compilation passes for parser.ts.
|
|
|
|
- [x] Review PR #862 (`fix: handle missing assistant messages gracefully in transcript parser` by @DennisHartrampf). File: `src/shared/transcript-parser.ts`. Missing assistant messages cause parser crash. Steps: (1) `gh pr checkout 862` (2) Review — should skip or handle gracefully, not crash (3) Run `npm run build` (4) If clean: `gh pr merge 862 --rebase --delete-branch`
|
|
- **Merged.** One-line fix: `throw new Error(...)` → `return ''` in `extractLastMessage()` when no message of requested role is found. Consistent with the function's existing behavior (already returns `''` at line 64 for found-but-empty cases). Fixes crash in summarize hook when user exits before assistant responds.
|
|
|
|
## Gemini Model Name
|
|
|
|
- [x] Review PR #831 (`fix: correct Gemini model name from gemini-3-flash to gemini-3-flash-preview` by @Glucksberg). Files: 12 files including GeminiAgent.ts, docs, UI. Steps: (1) `gh pr checkout 831` (2) Verify the correct model name from Gemini docs (is it `gemini-3-flash-preview` or something else as of today?) (3) If model name is correct and changes are sound: `gh pr merge 831 --rebase --delete-branch`
|
|
- **Merged.** Verified `gemini-3-flash-preview` is the correct model ID per Google's official [Gemini API docs](https://ai.google.dev/gemini-api/docs/models) — `gemini-3-flash` does not exist. Cherry-picked onto main (build artifact conflicts only, source merged cleanly). 9 files updated: type definition, RPM limits, model validation, settings, UI dropdown, docs, and tests. Greptile review noted a pre-existing unrelated naming mismatch in docs (`CLAUDE_MEM_GEMINI_BILLING_ENABLED` vs actual `CLAUDE_MEM_GEMINI_RATE_LIMITING_ENABLED`).
|
|
|
|
## Config/Environment
|
|
|
|
- [x] Review PR #634 (`fix: respect CLAUDE_CONFIG_DIR for plugin paths (#626)` by @Kuroakira). Files: 14 files across paths, hooks, and services. Steps: (1) `gh pr checkout 634` (2) Review — should use `CLAUDE_CONFIG_DIR` env var instead of hardcoded `~/.claude/` path (3) Large changeset — verify it doesn't break default behavior when env var is not set (4) Run `npm run build` (5) If clean: `gh pr merge 634 --rebase --delete-branch`
|
|
- **Cherry-picked onto main.** PR had build artifact merge conflicts (plugin/scripts/* — minified bundles), but all 6 source changes applied cleanly. Adds `MARKETPLACE_ROOT` constant to `paths.ts`, updates `HealthMonitor.ts`, `worker-utils.ts`, `BranchManager.ts`, `CursorHooksInstaller.ts`, and `ObservationCompiler.ts` to use centralized constants instead of hardcoded `homedir() + '.claude'` paths. Backwards compatible — defaults to `~/.claude` when `CLAUDE_CONFIG_DIR` is not set. TypeScript compilation passes for all modified files. Build failure is pre-existing (dompurify dep). Fixes #626.
|
|
|
|
- [x] Review PR #712 (`Fix environment variables` by @cjpeterein). Files: SettingsDefaultsManager.ts + build artifacts + tests. Steps: (1) `gh pr checkout 712` (2) Review — what env var fix? Check the diff for specifics (3) Run `npm run build` (4) If clean and focused: `gh pr merge 712 --rebase --delete-branch`
|
|
- **Cherry-picked onto main.** Adds `applyEnvOverrides()` method ensuring env vars take highest priority: `env > file > defaults`. Applied to all 3 return paths in `loadFromFile()` (missing file, normal load, error fallback). 6 test cases added and passing. Enables runtime configuration overrides for containerized deployments and CI/CD without modifying settings files. Build artifacts had merge conflicts (minified bundles) — source changes applied cleanly.
|
|
|
|
- [x] Review PR #524 (`fix: add minimum bun version check to smart-install.js` by @quicktime). File: `plugin/scripts/smart-install.js`. Steps: (1) `gh pr checkout 524` (2) Review version check logic — what minimum version? Is it still relevant? (3) If clean: `gh pr merge 524 --rebase --delete-branch`
|
|
- **Merged.** Adds `MIN_BUN_VERSION = '1.1.14'` check with auto-upgrade via `bun upgrade`. Clean semver `compareVersions()` function, `isBunVersionSufficient()` check inserted between bun install and uv install steps. Minimum version is justified: bun 1.1.14+ required for `.changes` property on SQLite `.run()` and multi-statement SQL support (1.0.26+). Falls back gracefully with manual upgrade instructions if auto-upgrade fails. Fixes #519.
|
|
|
|
- [x] Review PR #771 (`fix: handle stdin unavailability and timeout to prevent hook hangs` by @rajivsinclair). File: `src/cli/stdin-reader.ts`. Steps: (1) `gh pr checkout 771` (2) Review — stdin may not be available in all environments (CI, some Windows configs) (3) Should add timeout and graceful fallback (4) Run `npm run build` (5) If clean: `gh pr merge 771 --rebase --delete-branch`
|
|
- **Merged.** Replaces naive `stdin.on('end')` listener (which hangs when Claude Code doesn't close stdin) with JSON self-delimiting detection — tries `JSON.parse()` after each chunk and resolves immediately on valid JSON. Adds `isStdinAvailable()` guard for Bun EINVAL crash (#646), 30s safety timeout for malformed input, and graceful error handling (returns `undefined` instead of crashing). Fixes #727, addresses #646. TypeScript compilation clean.
|
|
|
|
## Cursor Integration
|
|
|
|
- [x] Review PR #721 (`fix(cursor): use bun runtime and fix hooks directory detection` by @polux0). Files: 5 cursor-related files. Steps: (1) `gh pr checkout 721` (2) Review Cursor hook changes — should use bun-runner.js pattern (consistent with v9.0.17) (3) Run `npm run build` (4) If compatible with current architecture: `gh pr merge 721 --rebase --delete-branch`
|
|
- **Cherry-picked onto main.** Source changes from `CursorHooksInstaller.ts` applied cleanly (commit 8030c44a). Fixes two Cursor standalone setup bugs: (1) `findCursorHooksDir()` now checks for `hooks.json` in unified CLI mode, not just legacy shell scripts — prevents "Could not find cursor-hooks directory" error for standalone Cursor users. (2) Hook commands now use bun instead of node, fixing `bun:sqlite` dependency crash. Added `findBunPath()` for cross-platform bun detection with PATH fallback. Build artifacts skipped (pre-existing dompurify dep issue). Dev log docs (`CURSOR-SETUP-BUGS-AND-FIXES.md`, `CURSOR-STANDALONE-SETUP-LOG.md`) not merged.
|
|
|
|
## Database
|
|
|
|
- [x] Review PR #889 (`fix(db): prevent FK constraint failures on worker restart` by @Et9797). File: `src/services/sqlite/...` (FK constraints). Steps: (1) `gh pr checkout 889` (2) Review — FK constraint failures on restart suggest orphaned references. Should the fix be proper cleanup or deferred FK checks? (3) Run `npm run build` (4) If clean: `gh pr merge 889 --rebase --delete-branch`
|
|
- **Cherry-picked onto main.** Comprehensive fix for FK constraint violations causing infinite retry loops and high CPU usage (#846). Core fix: `ensureMemorySessionIdRegistered()` guard ensures parent `sdk_sessions` row has correct `memory_session_id` before child `observations` INSERT. Schema migration 21 adds `ON UPDATE CASCADE` to FK constraints so child rows follow parent updates. Also includes claim-confirm queue pattern (prevents message loss on crash), spawn deduplication (prevents duplicate generator starts), and unrecoverable error detection (breaks infinite restart loops). Build artifacts skipped (stale base), path fixes already merged via PR #634. All 838 tests passing, 4 new FK constraint tests + 8 zombie prevention tests added. 20 source/test files changed.
|
|
|
|
- [x] Review PR #833 (`fix: add PRAGMA foreign_keys to cleanup-duplicates.ts` by @Glucksberg). Steps: (1) `gh pr checkout 833` (2) Check if the cleanup script context still exists and if PRAGMA foreign_keys is needed there (3) v8.5.6 fixed FK constraints — this may be stale. If so: `gh pr close 833 --comment "FK constraint issues were addressed in v8.5.6. If a specific scenario remains, please describe and reopen. Thank you!"`
|
|
- **Closed as stale.** The PR references non-existent `observation_files` and `observation_concepts` tables. In the current schema, `observations` is a child of `sdk_sessions` and has no children of its own — so `PRAGMA foreign_keys = ON` before deleting observations has no CASCADE effect. FK constraint issues were comprehensively addressed in v8.5.6 (PR #889: `ensureMemorySessionIdRegistered()` guard, `ON UPDATE CASCADE` schema migration, claim-confirm queue pattern).
|
|
|
|
## Session Complete Hook
|
|
|
|
- [x] Review PR #844 (`fix: add session-complete handler and hook to enable orphan reaper cleanup` by @thusdigital). Steps: (1) `gh pr checkout 844` (2) Review — does the orphan reaper need a session-complete signal to work? Check if the 5-min reaper interval is sufficient without it. (3) If the hook adds meaningful cleanup triggers: `gh pr merge 844 --rebase --delete-branch`. If reaper already handles this: close.
|
|
- **Cherry-picked onto main.** The orphan reaper was correctly implemented (5-min interval in ProcessRegistry.ts) but was ineffective because sessions were never removed from the active sessions map after summarize — so the reaper always saw all PIDs as "active" and never cleaned up. This caused zombie process accumulation (reported: 39+ processes, ~10GB on macOS after a single restart). PR adds session-complete as Stop phase 2 hook (after summarize, 30s timeout) that calls `POST /api/sessions/complete` to remove sessions from the active map via `SessionCompletionHandler.completeByDbId()`. 5 source files changed (new handler, handler registry, route, hooks.json, CLI help). Build artifacts skipped (stale base). 838 tests passing. Fixes #842.
|