Release v0.4.2#12
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Voice tests run in a dedicated Voice Extra job (ci.yml:174) with the voice extra installed. The main Test (Python 3.13/3.14) and release test jobs were re-running them without the extra, which caused 3.13 to spend ~20+ minutes loading TTS/STT models late in the run. Add --ignore=tests/voice and --ignore=tests/servers/routes/test_voice_routes.py to the main pytest invocations. Coverage on voice code is still collected through the Voice Extra job. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fixes GH #10. `uv tool install gobby` previously left the web UI unreachable: `:60887/` 404'd because the daemon's `_mount_production_ui` couldn't find UI assets, and `:60889` was never listening because no production process serves it. The root cause was that the wheel's `package-data` glob did not include `web/dist/` and `find_web_dir()` required `package.json` (which the installed wheel never ships). A second bug surfaced alongside: `gobby ui *` CLI commands loaded config via `load_config()` without a `ConfigStore`, so flipping `ui.enabled=true` in the DB was invisible to the CLI - only the daemon saw it. Changes: - New `build_backend/` PEP 517 wrapper that runs `npm ci && npm run build` in `web/` and copies `web/dist/` into `src/gobby/ui/web/dist/` before wheel packaging. `GOBBY_SKIP_UI_BUILD=1` escape hatch for CI; falls back to pre-staged dist if npm is unavailable. - `pyproject.toml`: register the wrapper and add `ui/web/dist/**/*` to the `gobby` package-data glob. - `find_web_dir()` now accepts dist-only install-mode layouts; a new `require_source=True` flag keeps `gobby ui dev/build/install-deps` on the source-mode (package.json) path. - `load_full_config_from_db()` takes an optional `config_file`; the CLI root uses it so every subcommand sees DB-resolved config. - `cli/ui.py`: drop the broken `WEB_UI_DIR` constant; resolve via `find_web_dir(config, require_source=True)`. - Tests: production-mount integration tests, install-mode `find_web_dir` coverage, build-backend staging tests, end-to-end regression test for `ui.enabled` read-through, and updated the CLI test suite for the patch-target rename. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
setuptools is a build-time dependency, not a runtime one. The earlier eager `from setuptools import build_meta as _orig` broke any code that imports the `build_backend` module at runtime - including the wrapper's own focused unit tests - in an env where setuptools isn't installed. Move the setuptools import behind a small `_orig()` accessor and a module-level `__getattr__` that forwards the PEP 517 hook attributes (`get_requires_for_*`, `prepare_metadata_for_*`) on demand. The actual build hooks (`build_wheel`, `build_sdist`, `build_editable`) call `_orig()` only when invoked, which is the only time setuptools is guaranteed to be present anyway. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fixes GH #11. Gobby's home-dir MCP server registry collided with Claude Code's project-level `.mcp.json` schema; running Claude Code from `~` (or any tool that walks for `.mcp.json` with the Claude Code parser) would emit a parse error against Gobby's `{"servers": [...]}` shape. - Rename the file to `~/.gobby/mcp-servers.json`. New module-level constants `DEFAULT_MCP_CONFIG_PATH` and `LEGACY_MCP_CONFIG_PATH` in `gobby.config.mcp` hold the canonical paths. - Add idempotent `migrate_legacy_mcp_config()` that renames the legacy file when the new one is missing, logs a warning when both exist, and is a no-op otherwise. - Call the migrator from `MCPConfigManager.__init__`, from `install_default_mcp_servers()`, and once early in daemon startup (`run_daemon`) so existing installs upgrade transparently. - Update the config docs to reference the new filename. - The in-isolation `.mcp.json` written by `agents/isolation.py` intentionally uses Claude Code's schema and is left alone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…stall Fixes GH #9. The embedding installer hardcoded `api_base`, `model`, and `dim` per provider, so users running LM Studio on a non-default host/port - or wanting a stronger model like Qwen3-Embedding-4B - had to hand-edit `~/.gobby/gobby-hub.db` to override `embeddings.api_base`, `embeddings.model`, and `embeddings.dim`. - `gobby install` now accepts `--embedding-url`, `--embedding-model`, and `--embedding-dim`. Each overrides the matching field on whichever provider was selected. - When the user supplies a custom URL or model but omits `--embedding-dim`, the installer probes `/v1/embeddings` with a 1-token request and reads `len(data[0].embedding)` to detect the dim automatically. - When the user supplies `model_override`, the LM Studio / Ollama bundled-model setup is skipped - they're bringing their own model (e.g. a Qwen3-Embedding GGUF served by LM Studio), and we should not try to `lms get nomic` underneath them. - The setup wizard exposes the same three knobs: after picking a provider it asks "Customize endpoint URL, model id, or embedding dim?" and collects whichever fields the user wants to override. Blank dim triggers the probe path. - The default remains nomic-embed-text-v1.5@f16 (768-dim) when no flags are passed. - Docs add a Qwen3-Embedding-4B recommendation with the storage and latency tradeoff note. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Build backend wrapper and UI staging build_backend/__init__.py |
Implements PEP 517 backend wrapper that stages web UI before wheel/sdist builds; runs npm build in web/, copies dist/ into src/gobby/ui/web/dist/, and validates wheel contains gobby/ui/web/dist/index.html; uses lazy setuptools import and delegates non-wrapped hooks. |
Build system configuration and manifest pyproject.toml, MANIFEST.in |
Updates build backend from setuptools.build_meta to custom build_backend with backend-path=".", includes ui/web/dist/**/* in package data, and adds build_backend/__init__.py to source distribution manifest. |
CI workflow wheel and package validation .github/workflows/ci.yml, .github/workflows/release.yml |
Validates built wheels contain gobby/ui/web/dist/index.html, inspects both source and wheel distributions, filters out voice tests from pytest runs. |
Build backend unit tests and production UI mounting tests tests/test_build_backend.py, tests/servers/test_app_factory_production_ui.py |
Unit tests covering UI staging from web/dist, env-var skip, pre-staged reuse, wheel verification; separate tests for production UI mounting covering index serve, asset serving, SPA fallback, and no-op behavior. |
Embedding Endpoint Customization
| Layer / File(s) | Summary |
|---|---|
CLI install flags and prompt infrastructure src/gobby/cli/install.py, src/gobby/cli/_install_prompts.py |
Adds --embedding-url, --embedding-model, --embedding-dim CLI options; implements _infer_embedding_provider_from_url helper; extends embedding prompts with interactive customization flow for URL/model/dimension overrides. |
Embedding installer with override support and dimension probing src/gobby/cli/installers/embedding.py |
Updates install_embedding to accept override parameters; skips bundled setup when overrides provided; adds _probe_embedding_dim helper to auto-detect dimension via short embedding request when not explicitly specified. |
Embedding installer and wizard override tests tests/cli/installers/test_embedding_installer.py, tests/cli/test_install_embedding_wizard.py |
Tests override behavior covering explicit overrides, API-base-only with probing, combined model+endpoint, failed probe with actionable error; wizard tests for CLI override flow, custom URL provider inference, and interactive override collection. |
MCP Server Config Migration and Daemon Startup
| Layer / File(s) | Summary |
|---|---|
MCP config module with legacy migration src/gobby/config/mcp.py |
Adds DEFAULT_MCP_CONFIG_PATH, LEGACY_MCP_CONFIG_PATH constants and migrate_legacy_mcp_config function for idempotent file-rename migration; updates MCPConfigManager to call migration and default to new path. |
MCP installer and daemon startup migration hooks src/gobby/cli/installers/mcp_config.py, src/gobby/runner_lifecycle.py |
Updates install_default_mcp_servers to use new path and invoke migration; adds startup migration in run_daemon with try/except error handling. |
MCP config migration tests tests/config/test_mcp_config.py |
Tests for migrate_legacy_mcp_config covering rename scenarios, no-op behavior, and parent directory creation; updates default path expectation to mcp-servers.json. |
CLI Config Loading Refactor and Web Directory Discovery
| Layer / File(s) | Summary |
|---|---|
CLI entry point and config loading refactor src/gobby/cli/__init__.py |
Switches from load_config to load_full_config_from_db for layered config resolution; stores resolved config in ctx.obj["config"] for subcommand access. |
Web directory discovery and UI command refactoring src/gobby/cli/ui.py, src/gobby/cli/utils.py |
Adds _resolve_source_web_dir helper; updates dev, build, install_deps commands to use improved find_web_dir with require_source flag; find_web_dir now supports both source (package.json) and production (dist/index.html) layouts. |
UI command tests with web directory mocking tests/cli/test_ui_coverage.py |
Updates dev, build, install_deps test suites to mock find_web_dir instead of WEB_UI_DIR; creates temporary web directories with package.json for testing. |
Web directory discovery and config loading regression tests tests/cli/test_utils_coverage.py |
Enhanced find_web_dir tests preventing accidental real-directory matches; tests for dist-only layout acceptance/rejection; regression test for load_full_config_from_db with DB-stored ui.enabled. |
CLI test mocking updates for load_full_config_from_db tests/cli/test_cli.py, tests/cli/test_cli_daemon.py, tests/cli/test_cli_extensions.py, tests/cli/test_cli_init.py, tests/cli/test_cli_install.py, tests/cli/test_install_prompts.py |
Systematically updates all CLI command tests to patch gobby.cli.load_full_config_from_db instead of gobby.cli.load_config across status, init, install, uninstall, daemon, extension, and hook test cases. |
Documentation and Version Updates
| Layer / File(s) | Summary |
|---|---|
Version bump and release notes pyproject.toml, src/gobby/__init__.py, CHANGELOG.md |
Bumps version 0.4.1 → 0.4.2; comprehensive changelog documenting embedding customization, wheel UI packaging, MCP config migration, and test filtering. |
Web UI port clarification in documentation README.md, docs/guides/observability.md, docs/guides/providers-and-models.md, docs/guides/system-requirements.md, docs/guides/web-ui.md |
Clarifies installed web UI runs on :60887 (with HTTP API and WebSocket), separate from dev UI on :60889; updates example URLs and quick-start instructions. |
MCP config path and build asset docs, gitignore update docs/guides/configuration.md, .gitignore |
Documents new MCP path ~/.gobby/mcp-servers.json; documents new embedding install flags; adds .gitignore entry for built web UI assets. |
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly Related Issues
- Allow custom embedding endpoint, model, and dim in
gobby install#9: Directly implements the embedding endpoint customization feature with CLI flags, interactive prompting, and dimension probing logic described in the issue.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 73.48% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title 'Release v0.4.2' directly matches the main objective of this PR, which is to cut and release version 0.4.2 into main. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
0.4.2
Comment @coderabbitai help to get the list of available commands and usage tips.
Code Review: Release v0.4.2Overall impression: A solid release with meaningful additions — custom embedding endpoints, web UI packaging, and MCP config migration. The implementation is generally clean and the changelog/README updates are comprehensive. A few issues worth addressing before merge, mostly around error-handling robustness and test coverage. 🔴 High Priority1. Broad exception swallowing in
|
| Area | Status |
|---|---|
| Error handling | |
| Type hints | ✅ Mostly good, minor gaps in build_backend |
| Security | ✅ No significant concerns |
| Test coverage | |
| Documentation | ✅ Good, minor gaps (GOBBY_SKIP_UI_BUILD, voice exclusion rationale) |
| Project conventions | ✅ Mostly followed |
The high-priority items (#1–#3) are the most important to address. The rest are polish items that could be handled in follow-up tasks if needed.
Review by Claude Sonnet 4.6
There was a problem hiding this comment.
Pull request overview
Release merge for v0.4.2, focused on (1) shipping production Web UI assets inside built wheels, (2) adding custom embedding endpoint/model/dim install support, and (3) renaming/migrating the global MCP server registry file to avoid .mcp.json schema collisions.
Changes:
- Add a custom PEP 517 build backend to stage
web/distinto the Python package and verify wheels containgobby/ui/web/dist/index.html. - Extend
gobby install+ installer prompts to support--embedding-url,--embedding-model,--embedding-dim(including dim probing). - Rename
~/.gobby/.mcp.json→~/.gobby/mcp-servers.jsonwith idempotent migration; update CLI config loading to honor DB config_store values.
Reviewed changes
Copilot reviewed 35 out of 37 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Bump local workspace version to 0.4.2. |
| pyproject.toml | Version bump; switch to build_backend; include UI dist in package-data. |
| build_backend/init.py | Custom PEP 517 backend to build/stage UI and verify wheel contents. |
| MANIFEST.in | Ensure build_backend is included in sdists. |
| .gitignore | Ignore staged UI assets under src/gobby/ui/. |
| src/gobby/init.py | Bump __version__ to 0.4.2. |
| src/gobby/cli/init.py | Load CLI config via load_full_config_from_db (DB-aware). |
| src/gobby/cli/utils.py | Add load_full_config_from_db(config_file=...); expand find_web_dir() to accept dist-only installs. |
| src/gobby/cli/ui.py | Resolve web/ via find_web_dir() for dev/build/install-deps flows. |
| src/gobby/cli/install.py | Add embedding override CLI flags and pass overrides into embedding installer prompts. |
| src/gobby/cli/_install_prompts.py | Add embedding override prompting + provider inference from custom URL. |
| src/gobby/cli/installers/embedding.py | Support URL/model/dim overrides; add dim probing helper. |
| src/gobby/cli/installers/mcp_config.py | Write default MCP servers to mcp-servers.json and run legacy migration. |
| src/gobby/config/mcp.py | New default MCP config path, constants, and migration helper. |
| src/gobby/runner_lifecycle.py | Run MCP legacy migration on daemon startup. |
| README.md | Update UI port docs (installed UI on :60887; dev UI on :60889). |
| docs/guides/web-ui.md | Document installed UI served from daemon port and dev UI behavior. |
| docs/guides/system-requirements.md | Update port ownership and UI requirements. |
| docs/guides/providers-and-models.md | Update UI URLs to :60887. |
| docs/guides/observability.md | Update dashboard/traces URLs to :60887. |
| docs/guides/configuration.md | Document MCP path rename + embedding override flags and probing behavior. |
| CHANGELOG.md | Add 0.4.2 release notes. |
| .github/workflows/ci.yml | Exclude voice tests from main run; add wheel UI index assertion in package check. |
| .github/workflows/release.yml | Exclude voice tests from pre-release test run. |
| tests/test_build_backend.py | Unit tests for staging UI and wheel verification behavior. |
| tests/servers/test_app_factory_production_ui.py | Regression tests for production UI mounting with dist-only layout. |
| tests/config/test_mcp_config.py | Update MCP default path expectations + migration tests. |
| tests/cli/test_utils_coverage.py | Update find_web_dir tests + add regression test for DB-backed ui.enabled. |
| tests/cli/test_ui_coverage.py | Update UI CLI tests to use find_web_dir and Click context config. |
| tests/cli/test_install_prompts.py | Add embedding override args to install prompt test expectations. |
| tests/cli/test_install_embedding_wizard.py | Add coverage for embedding overrides and interactive customize flow. |
| tests/cli/installers/test_embedding_installer.py | Add tests for override behavior, dim probing, and setup skipping. |
| tests/cli/test_cli.py | Update CLI tests to patch load_full_config_from_db. |
| tests/cli/test_cli_install.py | Update install/uninstall CLI tests to patch load_full_config_from_db. |
| tests/cli/test_cli_init.py | Update init CLI tests to patch load_full_config_from_db. |
| tests/cli/test_cli_extensions.py | Update extensions CLI tests to patch load_full_config_from_db. |
| tests/cli/test_cli_daemon.py | Update daemon CLI tests to patch load_full_config_from_db. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _qualifies(p: Path) -> bool: | ||
| if not p.exists(): | ||
| return False | ||
| if (p / "package.json").exists(): | ||
| return True | ||
| if not require_source and (p / "dist" / "index.html").exists(): | ||
| return True | ||
| return False |
| @click.option( | ||
| "--embedding-dim", | ||
| "embedding_dim", | ||
| type=int, |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build_backend/__init__.py`:
- Around line 73-74: In _stage_ui(), the two subprocess.run calls that invoke
npm (the lines calling subprocess.run(["npm", "ci"], ...) and
subprocess.run(["npm", "run", "build"], ...)) lack timeouts; add a timeout
argument (e.g. timeout=600) to each call and wrap them in a try/except that
catches subprocess.TimeoutExpired to log or raise a clear error (propagate a
RuntimeError or re-raise with context) so CI won't hang indefinitely if npm
stalls. Ensure you update both calls in the _stage_ui() function and include
contextual information (command and timeout) in the error handling.
In `@pyproject.toml`:
- Around line 113-115: Update pyproject.toml to raise the setuptools minimum
requirement from 61.0 to 64.0 because build_backend.build_editable() delegates
to setuptools.build_meta.build_editable which was added in setuptools 64.0.
Locate the requires = ["setuptools>=61.0", "wheel"] entry and change the
setuptools version to >=64.0 so editable installs using
build_backend.build_editable() will work in constrained build environments.
In `@src/gobby/cli/install.py`:
- Around line 165-170: The CLI currently accepts non-positive values for the
"--embedding-dim" argument; modify the add_argument calls that define
"--embedding-dim" (the entries setting dest "embedding_dim") to validate input
at the argparse boundary by using a custom type/validator (e.g., positive_int)
that raises argparse.ArgumentTypeError for values <= 0 so users get immediate
feedback; apply the same change to the other add_argument defining
"--embedding-dim" referenced in the file (the second occurrence around the block
at 197-202) so both CLI entry points reject zero/negative integers.
In `@src/gobby/config/mcp.py`:
- Around line 63-66: Wrap the call to legacy.replace(new) in an OSError handler
so migration is best-effort: attempt the replace inside try/except OSError, and
on exception log a warning with the exception (using logger.warning or
logger.exception) and fall back to a copy+unlink move strategy (e.g.,
shutil.copy2(legacy, new) then legacy.unlink()) before proceeding; after a
successful move (either replace or fallback) set restrictive permissions via
new.chmod(0o600) and only return True on success, otherwise return False so
initialization can continue safely. Ensure you reference and update the existing
new.parent.mkdir(new.parent.mkdir(...)), legacy.replace(new), logger.info(...)
locations and add new.chmod(0o600) after the move and appropriate exception
logging and fallback logic.
In `@tests/cli/test_ui_coverage.py`:
- Line 328: The current positional-only assertion is brittle; instead read the
recorded call from mock_find.call_args and pick up the config either from
call_args.kwargs['config'] if present or from call_args.args[0] otherwise, then
assert the call used require_source=True by invoking
mock_find.assert_called_with(config, require_source=True) (use the symbols
mock_find, call_args and assert_called_with to locate and implement this
change).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 45ef7d2d-d782-4b0d-94e0-2bed3d2b1797
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (36)
.github/workflows/ci.yml.github/workflows/release.yml.gitignoreCHANGELOG.mdMANIFEST.inREADME.mdbuild_backend/__init__.pydocs/guides/configuration.mddocs/guides/observability.mddocs/guides/providers-and-models.mddocs/guides/system-requirements.mddocs/guides/web-ui.mdpyproject.tomlsrc/gobby/__init__.pysrc/gobby/cli/__init__.pysrc/gobby/cli/_install_prompts.pysrc/gobby/cli/install.pysrc/gobby/cli/installers/embedding.pysrc/gobby/cli/installers/mcp_config.pysrc/gobby/cli/ui.pysrc/gobby/cli/utils.pysrc/gobby/config/mcp.pysrc/gobby/runner_lifecycle.pytests/cli/installers/test_embedding_installer.pytests/cli/test_cli.pytests/cli/test_cli_daemon.pytests/cli/test_cli_extensions.pytests/cli/test_cli_init.pytests/cli/test_cli_install.pytests/cli/test_install_embedding_wizard.pytests/cli/test_install_prompts.pytests/cli/test_ui_coverage.pytests/cli/test_utils_coverage.pytests/config/test_mcp_config.pytests/servers/test_app_factory_production_ui.pytests/test_build_backend.py
| subprocess.run(["npm", "ci"], cwd=_WEB_SRC, check=True) # nosec B603 B607 | ||
| subprocess.run(["npm", "run", "build"], cwd=_WEB_SRC, check=True) # nosec B603 B607 |
There was a problem hiding this comment.
Add timeouts to npm subprocess calls in _stage_ui().
These build-critical external calls currently have no timeout, so a network/npm stall can block CI/release indefinitely.
Suggested fix
- subprocess.run(["npm", "ci"], cwd=_WEB_SRC, check=True) # nosec B603 B607
- subprocess.run(["npm", "run", "build"], cwd=_WEB_SRC, check=True) # nosec B603 B607
+ subprocess.run(["npm", "ci"], cwd=_WEB_SRC, check=True, timeout=600) # nosec B603 B607
+ subprocess.run(
+ ["npm", "run", "build"], cwd=_WEB_SRC, check=True, timeout=600
+ ) # nosec B603 B607📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| subprocess.run(["npm", "ci"], cwd=_WEB_SRC, check=True) # nosec B603 B607 | |
| subprocess.run(["npm", "run", "build"], cwd=_WEB_SRC, check=True) # nosec B603 B607 | |
| subprocess.run(["npm", "ci"], cwd=_WEB_SRC, check=True, timeout=600) # nosec B603 B607 | |
| subprocess.run( | |
| ["npm", "run", "build"], cwd=_WEB_SRC, check=True, timeout=600 | |
| ) # nosec B603 B607 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build_backend/__init__.py` around lines 73 - 74, In _stage_ui(), the two
subprocess.run calls that invoke npm (the lines calling subprocess.run(["npm",
"ci"], ...) and subprocess.run(["npm", "run", "build"], ...)) lack timeouts; add
a timeout argument (e.g. timeout=600) to each call and wrap them in a try/except
that catches subprocess.TimeoutExpired to log or raise a clear error (propagate
a RuntimeError or re-raise with context) so CI won't hang indefinitely if npm
stalls. Ensure you update both calls in the _stage_ui() function and include
contextual information (command and timeout) in the error handling.
| requires = ["setuptools>=61.0", "wheel"] | ||
| build-backend = "setuptools.build_meta" | ||
| build-backend = "build_backend" | ||
| backend-path = ["."] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the first setuptools version that provides setuptools.build_meta.build_editable (PEP 660)?
💡 Result:
The first setuptools version that provides the PEP 660 hook setuptools.build_meta.build_editable is setuptools 64.0.0.[1][2] Setuptools’ NEWS notes that “#3265: Added implementation for editable install hooks (PEP 660)”.[2] That change was included starting with the 64.0.0 release line (hence first version with build_editable support is 64.0.0).[2][3]
Citations:
- 1: https://peps.python.org/pep-0660/
- 2: https://github.com/pypa/setuptools/blob/main/NEWS.rst
- 3: https://setuptools.pypa.io/en/latest/history.html
🏁 Script executed:
fd -name "build_backend.py" -o -name "build_backend" -type fRepository: GobbyAI/gobby
Length of output: 288
🏁 Script executed:
rg "build_editable|build_meta" --type pyRepository: GobbyAI/gobby
Length of output: 744
🏁 Script executed:
cat -n build_backend/__init__.pyRepository: GobbyAI/gobby
Length of output: 5173
🏁 Script executed:
head -120 pyproject.toml | tail -10Repository: GobbyAI/gobby
Length of output: 248
Raise the setuptools minimum to 64.0 to support the PEP 660 editable build hook.
build_backend.build_editable() (lines 122-127) delegates to setuptools.build_meta.build_editable, which was first provided in setuptools 64.0.0. The current requirement of setuptools>=61.0 is insufficient and will cause editable installs to fail in constrained build environments.
Suggested change
[build-system]
-requires = ["setuptools>=61.0", "wheel"]
+requires = ["setuptools>=64.0", "wheel"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| requires = ["setuptools>=61.0", "wheel"] | |
| build-backend = "setuptools.build_meta" | |
| build-backend = "build_backend" | |
| backend-path = ["."] | |
| requires = ["setuptools>=64.0", "wheel"] | |
| build-backend = "build_backend" | |
| backend-path = ["."] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyproject.toml` around lines 113 - 115, Update pyproject.toml to raise the
setuptools minimum requirement from 61.0 to 64.0 because
build_backend.build_editable() delegates to setuptools.build_meta.build_editable
which was added in setuptools 64.0. Locate the requires = ["setuptools>=61.0",
"wheel"] entry and change the setuptools version to >=64.0 so editable installs
using build_backend.build_editable() will work in constrained build
environments.
| "--embedding-dim", | ||
| "embedding_dim", | ||
| type=int, | ||
| default=None, | ||
| help="Override the embedding dimension. Omit to auto-detect via /v1/embeddings probe.", | ||
| ) |
There was a problem hiding this comment.
Validate --embedding-dim as a positive integer at the CLI boundary.
Right now 0 or negative values are accepted and only fail downstream (or produce unclear behavior). Rejecting invalid values here gives immediate, actionable feedback.
Suggested fix
def install(
@@
) -> None:
@@
+ if embedding_dim is not None and embedding_dim <= 0:
+ click.echo("--embedding-dim must be a positive integer", err=True)
+ sys.exit(2)
+
if (Also applies to: 197-202
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gobby/cli/install.py` around lines 165 - 170, The CLI currently accepts
non-positive values for the "--embedding-dim" argument; modify the add_argument
calls that define "--embedding-dim" (the entries setting dest "embedding_dim")
to validate input at the argparse boundary by using a custom type/validator
(e.g., positive_int) that raises argparse.ArgumentTypeError for values <= 0 so
users get immediate feedback; apply the same change to the other add_argument
defining "--embedding-dim" referenced in the file (the second occurrence around
the block at 197-202) so both CLI entry points reject zero/negative integers.
| new.parent.mkdir(parents=True, exist_ok=True) | ||
| legacy.replace(new) | ||
| logger.info("Migrated MCP config %s -> %s", legacy, new) | ||
| return True |
There was a problem hiding this comment.
Harden migration failure path and enforce restrictive permissions.
On Lines 64-65, legacy.replace(new) can raise OSError (permissions, FS issues) and currently aborts initialization. Also, migrated file mode may remain too permissive. Make migration best-effort and set mode to 0o600 after move.
Suggested patch
new.parent.mkdir(parents=True, exist_ok=True)
- legacy.replace(new)
- logger.info("Migrated MCP config %s -> %s", legacy, new)
- return True
+ try:
+ legacy.replace(new)
+ new.chmod(0o600)
+ except OSError as e:
+ logger.warning("Failed to migrate MCP config %s -> %s: %s", legacy, new, e)
+ return False
+
+ logger.info("Migrated MCP config %s -> %s", legacy, new)
+ return True🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gobby/config/mcp.py` around lines 63 - 66, Wrap the call to
legacy.replace(new) in an OSError handler so migration is best-effort: attempt
the replace inside try/except OSError, and on exception log a warning with the
exception (using logger.warning or logger.exception) and fall back to a
copy+unlink move strategy (e.g., shutil.copy2(legacy, new) then legacy.unlink())
before proceeding; after a successful move (either replace or fallback) set
restrictive permissions via new.chmod(0o600) and only return True on success,
otherwise return False so initialization can continue safely. Ensure you
reference and update the existing new.parent.mkdir(new.parent.mkdir(...)),
legacy.replace(new), logger.info(...) locations and add new.chmod(0o600) after
the move and appropriate exception logging and fallback logic.
| result = runner.invoke(ui, ["dev"], obj={"config": MagicMock()}, catch_exceptions=False) | ||
| assert result.exit_code == 0 | ||
| assert "Starting dev server" in result.output | ||
| mock_find.assert_called_with(mock_find.call_args.args[0], require_source=True) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Make the find_web_dir assertion resilient to arg-style changes.
The current check depends on positional args and can break if config is passed as a keyword while behavior remains correct.
Proposed tweak
- mock_find.assert_called_with(mock_find.call_args.args[0], require_source=True)
+ mock_find.assert_called()
+ assert mock_find.call_args is not None
+ assert mock_find.call_args.kwargs.get("require_source") is True🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/cli/test_ui_coverage.py` at line 328, The current positional-only
assertion is brittle; instead read the recorded call from mock_find.call_args
and pick up the config either from call_args.kwargs['config'] if present or from
call_args.args[0] otherwise, then assert the call used require_source=True by
invoking mock_find.assert_called_with(config, require_source=True) (use the
symbols mock_find, call_args and assert_called_with to locate and implement this
change).
Summary
gobby install#9.http://localhost:60889and daemon catch-all both 404 afteruv tool install gobby#10.~/.gobby/.mcp.jsonuses different schema than Claude Code's project.mcp.json#11 and release changelog updates.Validation
Summary by CodeRabbit
Release Notes v0.4.2
New Features
--embedding-url,--embedding-model,--embedding-dim) to configure custom OpenAI-compatible embedding endpoints during installation with automatic dimension detection.Bug Fixes
Documentation
:60887, dev mode on:60889.mcp-servers.json.