Release: gcode 0.6.0, gsqz 0.4.1, gcore 0.1.0, ghook 0.1.0#1
Merged
joshwilhelmi merged 15 commits intomainfrom Apr 17, 2026
Merged
Release: gcode 0.6.0, gsqz 0.4.1, gcore 0.1.0, ghook 0.1.0#1joshwilhelmi merged 15 commits intomainfrom
joshwilhelmi merged 15 commits intomainfrom
Conversation
…grow output
When a named pipeline only trimmed a few bytes, gsqz still prepended the
~20-byte [gsqz:low-savings] marker, producing net-negative "savings" on
small outputs (e.g. short git diffs showing -1%). Now the marker is only
applied when the marked output is strictly smaller than the original;
otherwise the original output is returned unchanged with strategy_name
"{pipeline}/no-op". Existing low-savings test updated to a scenario where
the marker still fits; new test covers the no-op suppression path.
Canonical plan for making Gobby hooks work across Claude/Codex/Gemini/ QwenCode sandboxes. Introduces a new ghook crate as the fourth member of this workspace (alongside gcode, gsqz, gloc), plus runtime-detection on the Python daemon side so the two repos can ship independently. Companion task: gobby #11843 (bootstrap) and gobby #11842 (cleanup follow-up to remove the legacy hook_dispatcher.py path once ghook is universal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> EOF )
PR 1 of the sandbox-tolerant-hooks Rust migration (R2-01 + R2-04 + R2-05). Scaffolds the shared gobby-core lib and ports find_project_root and read_project_id as public API. gcode keeps its local copy until PR 4 (R2-08) to keep this diff behavior-free. See docs/plans/sandbox-tolerant-hooks-rust.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ports hook_dispatcher.py:145-175 port-resolution into `bootstrap` + a new `daemon_url` module. Reads `daemon_port` and `bind_host` from `~/.gobby/bootstrap.yaml`; falls back to 60887 and 127.0.0.1 on missing or malformed files. `daemon_url` normalizes wildcard listen addresses (`0.0.0.0`, `::`, `::0`) to `127.0.0.1` for dialing — the dispatcher and gcode both silently broke when users set `bind_host: 0.0.0.0`. 13 unit tests cover default fallback, malformed YAML, custom port/host, and wildcard normalization. No consumers yet — ghook (PR 3) is first. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…atch Port hook_dispatcher.py to a native Rust binary with enqueue-first semantics: every invocation writes an envelope to ~/.gobby/hooks/inbox/<p>-<ts13>-<uuid>.json (fsync + rename) before the daemon POST, so sandbox FS-read denials and transient network failures never drop a hook — the drain worker replays from disk. CLI: --gobby-owned, --diagnose, --version. Exit 0 on success or non-critical failure (enqueued); 2 on critical failure (enqueued, signals host CLI). Malformed stdin goes directly to inbox/quarantine/ with a .meta.json sidecar (reason, json_error, stdin_bytes_b64) — drain never replays quarantined envelopes. Headers (X-Gobby-Project-Id, X-Gobby-Session-Id) are omitted entirely when unknown — never empty strings. Walk-up for project context runs before detach on Unix (setsid). Windows detach uses FreeConsole(); the spawn-time flags in the plan cannot be self-applied to a running process, and the enqueue-first file is the source of truth regardless. Terminal-context enrichment gated by per-CLI hook set; TMUX_PANE only emits when TMUX is also set to avoid parent/child pane confusion. Schemas frozen at v1 (inbox-envelope, diagnose-output), validated in unit tests. Release workflow mirrors release-gcode with tag prefix gobby-hook-v*; publishes to crates.io as gobby-hook. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lpers Remove duplicate find_project_root and read_project_id from gcode, now importing them from gobby_core::project. Updates call sites in config.rs and commands/index.rs. Deletes redundant tests (now tested in gobby-core). Adds gobby-core as a path dependency. Validation: - crates/gcode/Cargo.toml adds gobby-core path dependency - find_project_root and read_project_id removed from crates/gcode/src/project.rs - Call sites updated to import from gobby_core::project - Tests removed (behavior tested in gobby-core) - cargo test --workspace: PASS (148 tests) - cargo clippy --workspace --all-targets -- -D warnings: PASS - cargo fmt --all --check: PASS Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Mirrors updates made to the Python-side sandbox-tolerant-hooks-rev1
plan after the gobby-cli Phase 2 review:
- Windows detach spec corrected: `FreeConsole()` is the post-spawn
analog; `DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP` are
CreateProcess parent-side flags and cannot be self-applied from the
already-spawned child. Updated in both PR 3 description and the
contract-resolution section.
- Crate & distribution: documents the gobby-core → ghook publish-order
constraint, with the `version` + `path` dep declaration pattern that
lets workspace builds and crates.io both resolve.
- PR 3 CI bullet: explicit note that `release-gobby-core.yml` must
land in the repo first and the first `ghook` release is gated on
the matching `gobby-core` version being live on the registry.
- Empty-stdin normalization: ghook treats empty as `input_data = {}`;
dispatcher exits non-zero. Documented transition-window divergence;
exit code still governed by `--critical`, no silent drop.
- POST body shape: daemon endpoint accepts both legacy bare and
schema-v1 envelope. Python plan's §2.8 adds a handler test to pin
the tolerance.
…es on crates.io success gobby-hook depends on gobby-core as a path dep with a version; the release flow requires gobby-core to exist on crates.io first, so add a library-only release workflow keyed on gobby-core-v* tags. Also tighten release-gcode and release-ghook so the GitHub Release job waits on the crates.io step — prevents the footgun where binaries ship with a version that failed upstream. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s/gobby-core → crates/gcore
Two pre-release naming fixes, landed together while nothing is on crates.io
yet so the rename is free:
(A) Package gobby-hook → gobby-hooks. Binary (ghook), crate dir (crates/ghook),
and schema IDs unchanged. Convention: plural package name for domain
families (gobby-hooks, future gobby-tasks), singular binary for the tool
(ghook, gtask). Release tag prefix becomes gobby-hooks-v*.
(B) Directory crates/gobby-core → crates/gcore to match the g-prefix short-name
pattern (gcode, gsqz, gloc, ghook). Package name stays gobby-core — only
the filesystem layout changed. Workflow renamed release-gobby-core.yml →
release-gcore.yml for consistency with the other release-<short>.yml files.
Verified: cargo fmt, cargo clippy --workspace --all-targets -- -D warnings,
cargo test --workspace (267 passing), YAML syntax on all workflow files.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n fts.rs LIKE escape Extract empty_response_for_unresolved helper in commands/graph.rs and use it from callers/usages/blast_radius. resolve_symbol_name in search/fts.rs no longer includes the resolved name in its suggestions vec — callers get only alternatives. LIKE fallback now escapes %/_/\ via new escape_like helper and uses ESCAPE '\' so symbol names containing underscores (snake_case) match literally. glob_to_like_prefix refactored to reuse escape_like; behavior unchanged for existing callers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…040) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…0.1.0, ghook 0.1.0 Bump crate versions, add new ghook user/dev guides and gcore dev guide, note gobby-core dependency in gcode dev guide and low-savings marker suppression in gsqz user guide, add four versioned CHANGELOG sections, and surface ghook + gcore in the top-level README. Adds version="0.1" to gcode's path dep on gobby-core so the manifest is valid for crates.io upload. Staged release order: tag gcore-v0.1.0 first (so gobby-core lands on the registry), then tag gcode-v0.6.0, gsqz-v0.4.1, ghook-v0.1.0 together. Verified: cargo build/test/clippy/fmt all clean across the workspace (267 tests passing). cargo package -p gobby-core succeeded; gobby-code packaging waits on gobby-core being on the registry (expected -- release ordering handles this). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…o-op strategy Follow-up to #111. The inner [gsqz:low-savings] marker is suppressed when adding it would grow output, but the outer "[Output compressed by gsqz -- STRATEGY, X% reduction]" header was still being prepended for the resulting */no-op strategy. That's noise -- no compression actually happened, so the header is just spurious. Adds CompressionResult::is_passthrough() classifying passthrough, excluded, and */no-op together. main.rs now uses it for both the outer-header decision and the daemon savings report so the two stay in sync. Adds a unit test for the classification and asserts the existing low-savings-suppressed test result also reports as passthrough. Rolls into the unreleased 0.4.1 -- no version bump. Tests: 149 in gsqz (+1 new), 268 across the workspace, all passing. Verified end-to-end: the compound-git command that previously emitted "git-mutation/low-savings, -1% reduction" now emits clean original output (strategy=git-mutation/no-op, savings=0.0%, no header). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR adds a new shared crate Changes
Sequence DiagramsequenceDiagram
actor HostCLI as Host AI CLI
participant Ghook as ghook Binary
participant Envelope as Envelope / Transport
participant FSys as Filesystem (Inbox)
participant Daemon as Gobby Daemon
HostCLI->>Ghook: Spawn with --cli=X --type=Y --gobby-owned
Ghook->>Ghook: Parse stdin JSON
Ghook->>Ghook: Resolve project root & project ID
Ghook->>Ghook: Look up CLI config (critical, terminal_context)
alt Terminal Context Enabled
Ghook->>Ghook: Capture terminal context (PID, TTY, TMUX, env)
Ghook->>Ghook: Inject context into input_data
end
Ghook->>Envelope: Build envelope (version, timestamp, input, headers)
Ghook->>FSys: Atomic write envelope (inbox/{prefix}-{ts13}-{uuid}.json)
FSys-->>Ghook: File path
Ghook->>Ghook: Optionally detach from TTY
Ghook->>Daemon: POST /api/hooks/execute (envelope JSON, headers) with 30s timeout
alt Success (2xx)
Daemon-->>Ghook: Response
Ghook->>FSys: Delete inbox file
Ghook-->>HostCLI: Exit 0
else Failure (non-2xx or timeout)
Daemon-->>Ghook: Error or timeout
FSys->>FSys: Inbox file retained
alt Critical Hook
Ghook-->>HostCLI: Exit 2
else Non-Critical
Ghook-->>HostCLI: Exit 0
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…ry_sort_by
Two pre-existing call sites in crates/gsqz/src/primitives/group.rs
trigger Rust 1.95.0's new clippy::unnecessary_sort_by lint. Local
toolchain is 1.94 so they slipped through; CI runs 1.95 and rejects
them. Mechanical replacement of
sort_by(|a, b| b.1.len().cmp(&a.1.len()))
with
sort_by_key(|b| std::cmp::Reverse(b.1.len()))
at lines 373 and 414. No behavior change. Rolls into the unreleased
gsqz 0.4.1 -- no version bump, no CHANGELOG entry (this is a CI
correctness fix, not a user-facing change).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Multi-crate release that introduces the new shared gobby-core library and the new gobby-hooks (ghook) binary, while bumping gobby-code (gcode) and gobby-squeeze (gsqz) and updating docs/CI for the new workspace layout and release processes.
Changes:
- Added
gobby-corecrate (bootstrap + daemon URL + project discovery helpers) and wired it intogcodeandghook. - Added
ghookbinary crate with inbox-envelope spooling, diagnose/version modes, and JSON schema fixtures + validation tests. - Updated
gsqzlow-savings behavior (/no-opstrategy + passthrough classification) and refreshed docs/CI/release workflows for new versions.
Reviewed changes
Copilot reviewed 40 out of 41 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/plans/sandbox-tolerant-hooks.md | Adds detailed end-to-end plan for sandbox-tolerant hooks and daemon coordination. |
| docs/plans/sandbox-tolerant-hooks-rust.md | Rust-side handoff plan aligning ghook/gcore work with migration epic. |
| docs/guides/gsqz-user-guide.md | Documents low-savings suppression and /no-op strategy behavior. |
| docs/guides/ghook-user-guide.md | Introduces ghook user-facing behavior, wiring, diagnose, inbox/replay semantics. |
| docs/guides/ghook-development-guide.md | Documents ghook internals, schemas, transport guarantees, and testing approach. |
| docs/guides/gcore-development-guide.md | Documents gobby-core API, module responsibilities, and versioning/consumption guidance. |
| docs/guides/gcode-development-guide.md | Updates gcode architecture notes to reference shared gobby-core helpers. |
| crates/gsqz/src/main.rs | Uses CompressionResult::is_passthrough() to unify header/report suppression behavior. |
| crates/gsqz/src/compressor.rs | Implements low-savings marker suppression + /no-op strategy; adds passthrough classifier + tests. |
| crates/gsqz/Cargo.toml | Bumps gobby-squeeze to 0.4.1. |
| crates/ghook/src/transport.rs | Implements enqueue-first atomic spool + best-effort POST + quarantine writing utilities. |
| crates/ghook/src/terminal_context.rs | Captures/injects terminal context with tmux inheritance guard. |
| crates/ghook/src/main.rs | Implements clap CLI, dispatch flow, compatibility stamp writing. |
| crates/ghook/src/envelope.rs | Defines envelope v1 and validates serialization against JSON schema in tests. |
| crates/ghook/src/diagnose.rs | Implements --diagnose output and schema validation tests. |
| crates/ghook/src/detach.rs | Adds Unix setsid() / Windows FreeConsole() best-effort detach primitive. |
| crates/ghook/src/cli_config.rs | Adds per-CLI registry for critical hooks and terminal-context enrichment. |
| crates/ghook/schemas/inbox-envelope.v1.schema.json | Defines v1 inbox envelope schema shared across components. |
| crates/ghook/schemas/diagnose-output.v1.schema.json | Defines v1 diagnose output schema. |
| crates/ghook/README.md | Adds crate-level README with CLI surface and schema pointers. |
| crates/ghook/Cargo.toml | Adds gobby-hooks crate metadata and dependencies. |
| crates/gcore/src/project.rs | Adds shared project root discovery + project id reading helpers. |
| crates/gcore/src/lib.rs | Exposes gcore modules as public API surface. |
| crates/gcore/src/daemon_url.rs | Adds daemon URL composition with wildcard normalization + tests. |
| crates/gcore/src/bootstrap.rs | Adds bootstrap.yaml parsing with infallible defaults + tests. |
| crates/gcore/Cargo.toml | Adds gobby-core crate metadata and dependencies. |
| crates/gcode/src/search/fts.rs | Hardens LIKE escaping and refines suggestion list behavior for symbol resolution. |
| crates/gcode/src/project.rs | Removes duplicated project id logic now sourced from gobby-core. |
| crates/gcode/src/config.rs | Migrates project root/id helpers to gobby-core imports. |
| crates/gcode/src/commands/index.rs | Uses gobby-core project helpers when re-resolving alternate roots. |
| crates/gcode/src/commands/graph.rs | Deduplicates unresolved-response building and simplifies suggestion printing. |
| crates/gcode/Cargo.toml | Bumps gobby-code to 0.6.0 and adds gobby-core dependency. |
| README.md | Updates workspace overview to include ghook + gobby-core and install examples. |
| Cargo.toml | Adds gcore/ghook to workspace members and sets release opt-level for gobby-hooks. |
| Cargo.lock | Updates lockfile for new crates and dependencies (ghook/jsonschema/etc). |
| CHANGELOG.md | Adds release sections for gcode 0.6.0, gsqz 0.4.1, gobby-hooks 0.1.0, gobby-core 0.1.0. |
| .github/workflows/release-ghook.yml | Adds ghook release workflow (build artifacts + publish + GitHub release). |
| .github/workflows/release-gcore.yml | Adds gobby-core publish workflow to crates.io. |
| .github/workflows/release-gcode.yml | Gates GitHub release on successful crates.io publish. |
| .github/workflows/ci.yml | Expands CI to include clippy/tests for new crates (gobby-core, gobby-hooks). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+26
to
+29
| fn endpoint_to_url(endpoint: &DaemonEndpoint) -> String { | ||
| let host = dial_host(&endpoint.host); | ||
| format!("http://{host}:{}", endpoint.port) | ||
| } |
There was a problem hiding this comment.
endpoint_to_url will generate invalid URLs for non-wildcard IPv6 literals (e.g. bind_host: "::1" becomes http://::1:60887). Even if rare, this helper is now the shared implementation, so it should correctly bracket IPv6 hosts (http://[::1]:60887) or otherwise validate/normalize the host string before formatting.
Comment on lines
+8
to
+12
| /// Escape LIKE wildcards (`%`, `_`) and the backslash escape char itself. | ||
| /// Must be paired with `ESCAPE '\'` in the SQL for SQLite to honor it. | ||
| fn escape_like(s: &str) -> String { | ||
| let mut out = String::with_capacity(s.len()); | ||
| for c in s.chars() { |
There was a problem hiding this comment.
escape_like is documented as requiring ESCAPE '\\' in SQL, but glob_to_like_prefix is used to build ... file_path LIKE ? predicates without an ESCAPE clause (e.g. in search_symbols_fts). As written, the backslash escapes may not be honored, which can break literal %/_/\\ matching and reduce prefilter correctness. Either add ESCAPE '\\' to those LIKE conditions or avoid emitting escapes there.
| 3. Otherwise, walk up from cwd looking for `.gobby/project.json` (Gobby-managed) or `.gobby/gcode.json` (standalone) | ||
| 4. Fall back to VCS root markers (`.git`, `.hg`, `.svn`) or cwd | ||
|
|
||
| The walk-up and `project.json` reading steps use `gobby_core::project::find_project_root` and `gobby_core::project::read_project_id` (extracted from `gcode/src/project.rs` so `ghook` and other binaries can share the same logic). Bootstrap-config and daemon-URL helpers also come from `gobby-core`. See [gobby-core Development Guide](gcore-development-guide.md) for the shared API. |
There was a problem hiding this comment.
This guide now states that gcode uses gobby-core for “bootstrap-config and daemon-URL helpers”, but crates/gcode/src/config.rs still has its own resolve_daemon_url() implementation that reads bootstrap.yaml directly. Please either update the guide to match the current code, or complete the migration so daemon URL resolution (including wildcard-host normalization) actually comes from gobby-core.
|
|
||
| use std::collections::HashSet; | ||
|
|
||
| /// Per-CLI dispatcher knobs. Frozen at compile time — CLIs are a closed set. |
There was a problem hiding this comment.
The doc comment says “CLIs are a closed set”, but the runtime behavior intentionally tolerates unknown --cli values (main falls back to using the literal CLI string as source and skips enrichment). Please update this comment to reflect the actual behavior so future changes don’t accidentally remove the “unknown CLI tolerated” contract.
Suggested change
| /// Per-CLI dispatcher knobs. Frozen at compile time — CLIs are a closed set. | |
| /// Per-CLI dispatcher knobs for known CLIs in the compile-time registry. | |
| /// Unknown CLI values are tolerated by callers: they can fall back to using | |
| /// the literal CLI string as `source` and skip CLI-specific enrichment. |
Comment on lines
+52
to
+55
| /// Print version and write ~/.gobby/bin/.ghook-compatibility stamp. | ||
| #[arg(long)] | ||
| version: bool, | ||
|
|
There was a problem hiding this comment.
Mode selection isn’t enforced as mutually exclusive: --version can currently be combined with --diagnose/--gobby-owned and will silently take precedence. Consider using a clap ArgGroup/conflicts so exactly one mode flag is set, matching the documented CLI surface and avoiding surprising precedence rules.
Comment on lines
+9
to
+13
| /// Walk up from `start` looking for a `.gobby` directory containing either | ||
| /// `project.json` or `gcode.json`. Returns the project root (the directory | ||
| /// containing `.gobby`) or `None` if no project is found before hitting the | ||
| /// filesystem root. | ||
| pub fn find_project_root(start: &Path) -> Option<PathBuf> { |
There was a problem hiding this comment.
gobby-core::project is now the shared implementation, but there are currently no unit tests in this module (the equivalent tests were removed from gcode). Since bootstrap/daemon_url already have tests in gobby-core, please port the prior find_project_root and read_project_id test cases here (including the legacy project_id fallback) to keep behavior pinned.
Comment on lines
+3
to
+7
| on: | ||
| push: | ||
| tags: | ||
| - "gobby-hooks-v*" | ||
|
|
There was a problem hiding this comment.
The workflow triggers on gobby-hooks-v* tags, but the PR description/release instructions refer to a ghook-v* prefix. This mismatch will cause releases to not run when following the documented tag naming; please align either the workflow trigger or the documented release tag prefix.
Comment on lines
+3
to
+7
| on: | ||
| push: | ||
| tags: | ||
| - "gobby-core-v*" | ||
|
|
There was a problem hiding this comment.
The workflow triggers on gobby-core-v* tags, but the PR description/release instructions refer to a gcore-v* prefix. This mismatch will cause publishes to not run when following the documented tag naming; please align either the workflow trigger or the documented release tag prefix.
Comment on lines
+74
to
+78
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "ghook --gobby-owned --cli=claude --type=session-start --critical" | ||
| } |
There was a problem hiding this comment.
The manual hook wiring examples use "command": "ghook ...". In strict sandbox profiles, exec allowlists typically require an absolute path (and may not allow PATH lookup), which is also called out in the sandbox plan. Consider updating these examples to use the resolved absolute path (e.g. expanded ~/.gobby/bin/ghook) and note that PATH-based invocation may fail under sandboxing.
Comment on lines
+86
to
+90
| .with_context(|| format!("fsync tmp {}", tmp.display()))?; | ||
| } | ||
| fs::rename(&tmp, final_path) | ||
| .with_context(|| format!("rename {} -> {}", tmp.display(), final_path.display()))?; | ||
| Ok(()) |
There was a problem hiding this comment.
atomic_write fsyncs the temp file but doesn’t fsync the containing directory after rename(). On many filesystems this can still lose the directory entry on power loss/kernel crash, undermining the “durable spool” guarantee. Consider fsyncing the parent directory after rename (best-effort, Unix-only if needed).
…ble_match
crates/gcode/src/commands/init.rs:34 trips clippy::collapsible_match
on a nested `match Err(e) => { if !quiet { ... } }`. Convert to a
match guard `Err(e) if !quiet => { ... }`; the `_ => {}` arm catches
the quiet=true case as before, so behavior is preserved.
Updated local toolchain to 1.95 (was 1.94) so all 1.95 lints fire
locally going forward. Verified all five CI clippy commands pass on
1.95: gobby-core, gobby-hooks, gobby-squeeze, gobby-local,
gobby-code (no embeddings). Rolls into unreleased gcode 0.6.0 -- no
version bump.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (1)
crates/ghook/src/diagnose.rs (1)
39-40: Derivedaemon_urlfrom the same endpoint snapshot.These two calls read
bootstrap.yamlindependently, sodaemon_host/daemon_portcan disagree withdaemon_urlif the file changes between reads.--diagnoseis supposed to explain the current config, so it should build all three fields from one resolved endpoint.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/ghook/src/diagnose.rs` around lines 39 - 40, The code currently calls bootstrap::read_daemon_endpoint() and then independently calls daemon_url::daemon_url(), risking inconsistent values if bootstrap.yaml changes between reads; instead, capture a single endpoint snapshot via bootstrap::read_daemon_endpoint() into endpoint and derive the daemon_url from that same snapshot (e.g., construct the URL using endpoint.daemon_host and endpoint.daemon_port or call a constructor/helper that accepts the endpoint) so daemon_host, daemon_port and daemon_url all come from the same endpoint object rather than a second daemon_url::daemon_url() read.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release-gcore.yml:
- Around line 4-7: The workflow's tag trigger uses the wrong pattern
"gobby-core-v*" in the push.tags entry; update the push.tags value in
.github/workflows/release-gcore.yml (the push: tags: block) to the documented
release prefix "gcore-v*" so the workflow runs for the intended release tags.
In @.github/workflows/release-ghook.yml:
- Around line 4-7: The workflow push tag pattern uses "gobby-hooks-v*" but docs
expect "ghook-v*", so update the push -> tags pattern (the tag value currently
"gobby-hooks-v*") to the correct release prefix "ghook-v*" so pushes of tags
like ghook-v... will trigger the workflow; locate the tags entry in the release
workflow and replace "gobby-hooks-v*" with "ghook-v*".
In `@crates/gcode/src/search/fts.rs`:
- Around line 8-19: Several LIKE usages still overmatch because patterns and
file_path matches aren't escaped: update search_symbols_by_name,
search_symbols_fts, count_symbols, count_content and all fallback LIKE call
sites (including the fallback paths around the ~230–405 region) to use
escape_like(...) for any pattern construction (e.g. replace raw
format!("%{query}%") with formatting that calls escape_like on the query and on
any glob_to_like_prefix output) and append " ESCAPE '\\'" to the SQL fragments
that use LIKE (including "file_path LIKE ?") so SQLite honors the backslash
escapes; ensure any helper that returns a LIKE prefix (glob_to_like_prefix) has
its output run through escape_like before use.
In `@crates/gcore/src/bootstrap.rs`:
- Around line 83-87: The current bind_host extraction lets an empty string
through causing an invalid host; update the logic that builds host (the
yaml.get("bind_host") chain in bootstrap.rs) to treat empty strings as absent by
filtering out empty &str values (e.g., check for !s.is_empty() before mapping)
so that when bind_host is missing or empty it falls back to
DEFAULT_BIND_HOST.to_string(); ensure you modify the same expression that
produces host so downstream code (e.g., daemon_url builder) always receives a
usable host.
In `@crates/gcore/src/daemon_url.rs`:
- Around line 26-29: endpoint_to_url currently concatenates the host from
dial_host into the URL without IPv6 brackets; update endpoint_to_url (which
takes &DaemonEndpoint and calls dial_host) to detect IPv6 literals and wrap them
in square brackets before formatting the URL (e.g., if the host contains ':' and
is not already bracketed, produce "[host]"). Ensure you only bracket literal
IPv6 forms (avoid bracketing already-bracketed hosts or wildcard/empty values)
and then use the bracketed host in the existing format!("http://{host}:{}",
endpoint.port).
In `@crates/gcore/src/project.rs`:
- Around line 17-18: find_project_root() currently treats a directory containing
either .gobby/project.json or .gobby/gcode.json as a project root, but
read_project_id() only opens .gobby/project.json causing legacy repos to break;
update read_project_id() to mirror the legacy fallback by attempting to open
.gobby/project.json first and if it does not exist, fall back to
.gobby/gcode.json (parsing whichever exists) and return the project id, or
alternatively change find_project_root() to require project.json—prefer the
backward-compatible fix by adding the gcode.json fallback in read_project_id()
so both functions accept the same set of legacy roots.
In `@crates/ghook/schemas/inbox-envelope.v1.schema.json`:
- Around line 45-61: The schema for the "headers" object currently allows
arbitrary header names because "additionalProperties" is a schema (type:
"string", minLength: 1); change "additionalProperties" to false to restrict keys
to only the defined properties ("X-Gobby-Project-Id" and "X-Gobby-Session-Id")
so only those two headers validate; update the "headers" object definition by
replacing the existing additionalProperties schema with false in the
inbox-envelope.v1 schema to enforce the intended contract.
In `@crates/ghook/src/detach.rs`:
- Around line 35-38: The extern FFI block declaring FreeConsole must be marked
unsafe for Rust 2024 compatibility; replace the bare extern "system" block with
an unsafe extern "system" block (e.g., change the declaration that contains fn
FreeConsole() -> i32 to reside inside an unsafe extern "system" { ... } block),
and apply the same unsafe extern change to any other extern blocks in this
module if present so the Windows FFI compiles under Rust 2024.
In `@crates/ghook/src/main.rs`:
- Around line 117-123: The current flow in main.rs (around argument handling and
event ingestion) returns ExitCode::SUCCESS for non-critical hooks before the
hook is durably stored, and it swallows stdin read errors by converting them to
"{}"; update the logic so non-critical mode only returns success after a
successful durable action (successful enqueue into the inbox or successful
quarantine write), and propagate or surface failures from CLI/type resolution,
inbox-dir resolution, stdin().read_to_end(), enqueue (e.g., enqueue_event /
enqueue_to_inbox), and quarantine write (e.g., write_quarantine) instead of
silently treating them as recoverable; specifically, stop converting read errors
into an empty JSON payload, check the result of inbox resolution and
enqueue/quarantine functions (return non-zero or log and return failure when
they fail), and ensure functions handling CLI/type (args.cli, args.hook_type)
and stdin reading return an Err/ExitCode on unrecoverable errors so success is
only returned once durability is confirmed.
- Around line 76-95: The current main.rs branches on args.version,
args.diagnose, and args.gobby_owned independently which lets multiple mode flags
be mixed; change the startup to validate that exactly one of args.version,
args.diagnose, args.gobby_owned is true (e.g., compute a count of selected
modes) and if the count != 1 print the same error message and return
ExitCode::from(2). After that guard, keep the existing logic that calls
write_compatibility_stamp()/run_diagnose()/run_gobby_owned() as before so each
mode still behaves the same when chosen alone.
In `@crates/ghook/src/terminal_context.rs`:
- Around line 75-91: The tty_name_or_null function uses the non-thread-safe
libc::ttyname; replace its body to call libc::ttyname_r with a caller-owned
buffer: allocate a sufficiently large Vec<u8> (e.g. PATH_MAX or 1024), call
unsafe { libc::ttyname_r(0, buf.as_mut_ptr() as *mut _, buf.len()) }, check the
returned errno/result for errors and for a null/empty result, then construct a
CStr from the buffer slice up to the written length and convert to Rust str to
return Value::String or Value::Null on errors; keep the unsafe block but avoid
relying on any shared static buffer so concurrent calls are safe.
In `@crates/ghook/src/transport.rs`:
- Around line 128-131: The match arm handling req.send_string(&body) currently
ignores fs::remove_file(enqueued_path) and always returns
DeliveryOutcome::Delivered; change it so you only return
DeliveryOutcome::Delivered if fs::remove_file(enqueued_path) returns Ok. If
remove_file returns Err, log or propagate that error and return a non-delivered
outcome instead (i.e., do not return DeliveryOutcome::Delivered), ensuring the
code paths around req.send_string, enqueued_path, fs::remove_file, and
DeliveryOutcome::Delivered reflect this check.
- Around line 64-90: The atomic_write function writes and fsyncs the temp file
but doesn't fsync the containing directory after fs::rename, which can lose the
entry on crash; after the fs::rename(&tmp, final_path) call open the parent
directory (from final_path.parent() or tmp.parent()), call sync_all on that
directory file handle (e.g., open with File::open or std::fs::OpenOptions) and
propagate errors with context similar to the existing with_context calls to
ensure the directory metadata is persisted.
In `@crates/gsqz/src/compressor.rs`:
- Around line 29-33: is_passthrough currently infers passthrough from
strategy_name strings and can misclassify user-defined pipelines like
"foo/no-op"; change the design to use explicit state: add a boolean field (e.g.,
passthrough) to the struct that is set to true only in the actual passthrough
paths (the true passthrough return path, the excluded path, and the low-savings
suppression branch) and false everywhere else, then modify the is_passthrough()
method to return that boolean instead of matching strategy_name, and ensure any
code paths that previously relied on strategy_name text are updated to set the
new passthrough flag where appropriate.
In `@docs/guides/gcore-development-guide.md`:
- Around line 68-79: The docs are contradictory about IPv6: update the paragraph
around gobby_core::daemon_url::daemon_url to explicitly state that raw IPv6
literals are not safe for URL composition because daemon_url does not perform
bracketting for IPv6 addresses; change the text that currently says "Hostnames,
named interfaces, and explicit IPv4/IPv6 literals pass through unchanged" to
clarify that only hostnames, named interfaces, and explicit IPv4 literals pass
through unchanged, and add a sentence that "explicit IPv6 literals are passed
through verbatim by daemon_url and therefore must be bracketed by the caller
(e.g. [::1]) or converted to a hostname before embedding in a URL; daemon_url
does not add brackets."
In `@docs/guides/gsqz-user-guide.md`:
- Line 203: The overview still states that sub-5% compression results are
returned unchanged; update that earlier summary to match the new behavior
described later by saying sub-5% compression may be annotated with the
`[gsqz:low-savings]` marker unless prepending the annotation would increase the
output size, in which case the result is left unchanged; ensure the overview
references the `[gsqz:low-savings]` marker and the conditional suppression rule
so both sections are consistent.
In `@docs/plans/sandbox-tolerant-hooks.md`:
- Around line 72-80: This plan is out of date and must be marked superseded or
reconciled with the shipped implementation: update the document to reference the
actual modules and contracts used (replace mentions of
crates/ghook/src/config.rs, top-level gobby-cli/schemas/*, env/current.json
header sourcing, and double-fork detach with the current
crates/gcore/src/bootstrap.rs, crates/ghook/schemas/*, stdin + project walk-up
input contract, and the single post-enqueue detach flow), and either remove or
clearly annotate the superseded sections (including the other affected ranges
around 208-216 and 293-321) so future work targets the correct files and APIs.
---
Nitpick comments:
In `@crates/ghook/src/diagnose.rs`:
- Around line 39-40: The code currently calls bootstrap::read_daemon_endpoint()
and then independently calls daemon_url::daemon_url(), risking inconsistent
values if bootstrap.yaml changes between reads; instead, capture a single
endpoint snapshot via bootstrap::read_daemon_endpoint() into endpoint and derive
the daemon_url from that same snapshot (e.g., construct the URL using
endpoint.daemon_host and endpoint.daemon_port or call a constructor/helper that
accepts the endpoint) so daemon_host, daemon_port and daemon_url all come from
the same endpoint object rather than a second daemon_url::daemon_url() read.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b1c873e3-8215-4c50-9dd2-124e8e45b316
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
.github/workflows/ci.yml.github/workflows/release-gcode.yml.github/workflows/release-gcore.yml.github/workflows/release-ghook.ymlCHANGELOG.mdCargo.tomlREADME.mdcrates/gcode/Cargo.tomlcrates/gcode/src/commands/graph.rscrates/gcode/src/commands/index.rscrates/gcode/src/config.rscrates/gcode/src/project.rscrates/gcode/src/search/fts.rscrates/gcore/Cargo.tomlcrates/gcore/src/bootstrap.rscrates/gcore/src/daemon_url.rscrates/gcore/src/lib.rscrates/gcore/src/project.rscrates/ghook/Cargo.tomlcrates/ghook/README.mdcrates/ghook/schemas/diagnose-output.v1.schema.jsoncrates/ghook/schemas/inbox-envelope.v1.schema.jsoncrates/ghook/src/cli_config.rscrates/ghook/src/detach.rscrates/ghook/src/diagnose.rscrates/ghook/src/envelope.rscrates/ghook/src/main.rscrates/ghook/src/terminal_context.rscrates/ghook/src/transport.rscrates/gsqz/Cargo.tomlcrates/gsqz/src/compressor.rscrates/gsqz/src/main.rsdocs/guides/gcode-development-guide.mddocs/guides/gcore-development-guide.mddocs/guides/ghook-development-guide.mddocs/guides/ghook-user-guide.mddocs/guides/gsqz-user-guide.mddocs/plans/sandbox-tolerant-hooks-rust.mddocs/plans/sandbox-tolerant-hooks.md
Comment on lines
+4
to
+7
| push: | ||
| tags: | ||
| - "gobby-core-v*" | ||
|
|
There was a problem hiding this comment.
Tag trigger does not match the documented release tag prefix.
Line 6 uses gobby-core-v*, but the release instructions/changelog reference gcore-v*. Following the documented tag flow would skip this workflow.
Suggested fix
on:
push:
tags:
- - "gobby-core-v*"
+ - "gcore-v*"📝 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.
Suggested change
| push: | |
| tags: | |
| - "gobby-core-v*" | |
| on: | |
| push: | |
| tags: | |
| - "gcore-v*" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release-gcore.yml around lines 4 - 7, The workflow's tag
trigger uses the wrong pattern "gobby-core-v*" in the push.tags entry; update
the push.tags value in .github/workflows/release-gcore.yml (the push: tags:
block) to the documented release prefix "gcore-v*" so the workflow runs for the
intended release tags.
Comment on lines
+4
to
+7
| push: | ||
| tags: | ||
| - "gobby-hooks-v*" | ||
|
|
There was a problem hiding this comment.
Tag trigger likely mismatches the intended release tag prefix.
Line 6 listens for gobby-hooks-v*, while release docs/changelog describe ghook-v*. If ghook-v... is pushed, this workflow won’t fire.
Suggested fix
on:
push:
tags:
- - "gobby-hooks-v*"
+ - "ghook-v*"📝 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.
Suggested change
| push: | |
| tags: | |
| - "gobby-hooks-v*" | |
| push: | |
| tags: | |
| - "ghook-v*" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release-ghook.yml around lines 4 - 7, The workflow push
tag pattern uses "gobby-hooks-v*" but docs expect "ghook-v*", so update the push
-> tags pattern (the tag value currently "gobby-hooks-v*") to the correct
release prefix "ghook-v*" so pushes of tags like ghook-v... will trigger the
workflow; locate the tags entry in the release workflow and replace
"gobby-hooks-v*" with "ghook-v*".
Comment on lines
+8
to
+19
| /// Escape LIKE wildcards (`%`, `_`) and the backslash escape char itself. | ||
| /// Must be paired with `ESCAPE '\'` in the SQL for SQLite to honor it. | ||
| fn escape_like(s: &str) -> String { | ||
| let mut out = String::with_capacity(s.len()); | ||
| for c in s.chars() { | ||
| if matches!(c, '\\' | '%' | '_') { | ||
| out.push('\\'); | ||
| } | ||
| out.push(c); | ||
| } | ||
| out | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, read the file to see the current state
cat -n crates/gcode/src/search/fts.rs | head -200Repository: GobbyAI/gobby-cli
Length of output: 8199
🏁 Script executed:
# Also search for all LIKE patterns and ESCAPE usage in the file
rg "LIKE|ESCAPE|escape_like|%\{" crates/gcode/src/search/fts.rs -nRepository: GobbyAI/gobby-cli
Length of output: 2139
🏁 Script executed:
# Get the full file to understand context better
wc -l crates/gcode/src/search/fts.rsRepository: GobbyAI/gobby-cli
Length of output: 95
The LIKE hardening is incomplete across multiple fallback functions.
Only resolve_symbol_name() (line 169–172) applies both escape_like() and ESCAPE '\\'. Other callsites still need fixes:
-
search_symbols_by_name()(line 111): Uses rawformat!("%{query}%")without escaping. Lines 114 and 126 havefile_path LIKE ?withoutESCAPE '\\'. -
search_symbols_fts()(line 78): Uses escaped prefix fromglob_to_like_prefix()but omitsESCAPE '\\', breaking the protection. -
Additional functions (
count_symbols,count_content, and other fallback paths around lines 230–405): Same issues—raw patterns withoutescape_like()andfile_path LIKE ?withoutESCAPE '\\'.
Queries or paths containing _, %, or \ will continue to overmatch in all these locations.
Pattern to apply across remaining LIKE call sites
- let pattern = format!("%{query}%");
+ let pattern = format!("%{}%", escape_like(query));
- conditions.push("file_path LIKE ?".to_string());
+ conditions.push("file_path LIKE ? ESCAPE '\\'".to_string());
- conditions.push("(name LIKE ? OR qualified_name LIKE ?)".to_string());
+ conditions.push("(name LIKE ? ESCAPE '\\' OR qualified_name LIKE ? ESCAPE '\\')".to_string());
- conditions.push("content LIKE ?".to_string());
+ conditions.push("content LIKE ? ESCAPE '\\'".to_string());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/gcode/src/search/fts.rs` around lines 8 - 19, Several LIKE usages
still overmatch because patterns and file_path matches aren't escaped: update
search_symbols_by_name, search_symbols_fts, count_symbols, count_content and all
fallback LIKE call sites (including the fallback paths around the ~230–405
region) to use escape_like(...) for any pattern construction (e.g. replace raw
format!("%{query}%") with formatting that calls escape_like on the query and on
any glob_to_like_prefix output) and append " ESCAPE '\\'" to the SQL fragments
that use LIKE (including "file_path LIKE ?") so SQLite honors the backslash
escapes; ensure any helper that returns a LIKE prefix (glob_to_like_prefix) has
its output run through escape_like before use.
Comment on lines
+83
to
+87
| let host = yaml | ||
| .get("bind_host") | ||
| .and_then(|v| v.as_str()) | ||
| .map(str::to_owned) | ||
| .unwrap_or_else(|| DEFAULT_BIND_HOST.to_string()); |
There was a problem hiding this comment.
Treat empty bind_host as invalid and fall back to loopback.
bind_host: "" currently passes as_str() and is returned verbatim, but crates/gcore/src/daemon_url.rs:13-30 will then build a dial URL from that empty host. That breaks the “always get something usable” contract for malformed bootstrap config.
Proposed fix
let host = yaml
.get("bind_host")
.and_then(|v| v.as_str())
+ .filter(|s| !s.trim().is_empty())
.map(str::to_owned)
.unwrap_or_else(|| DEFAULT_BIND_HOST.to_string());📝 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.
Suggested change
| let host = yaml | |
| .get("bind_host") | |
| .and_then(|v| v.as_str()) | |
| .map(str::to_owned) | |
| .unwrap_or_else(|| DEFAULT_BIND_HOST.to_string()); | |
| let host = yaml | |
| .get("bind_host") | |
| .and_then(|v| v.as_str()) | |
| .filter(|s| !s.trim().is_empty()) | |
| .map(str::to_owned) | |
| .unwrap_or_else(|| DEFAULT_BIND_HOST.to_string()); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/gcore/src/bootstrap.rs` around lines 83 - 87, The current bind_host
extraction lets an empty string through causing an invalid host; update the
logic that builds host (the yaml.get("bind_host") chain in bootstrap.rs) to
treat empty strings as absent by filtering out empty &str values (e.g., check
for !s.is_empty() before mapping) so that when bind_host is missing or empty it
falls back to DEFAULT_BIND_HOST.to_string(); ensure you modify the same
expression that produces host so downstream code (e.g., daemon_url builder)
always receives a usable host.
Comment on lines
+26
to
+29
| fn endpoint_to_url(endpoint: &DaemonEndpoint) -> String { | ||
| let host = dial_host(&endpoint.host); | ||
| format!("http://{host}:{}", endpoint.port) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify URL composition currently lacks IPv6 bracketing and test coverage for ::1 formatting.
rg -n -C2 'fn endpoint_to_url|format!\("http://\{host\}:\{\}"|fn dial_host|::1|\[::1\]' crates/gcore/src/daemon_url.rsRepository: GobbyAI/gobby-cli
Length of output: 489
🏁 Script executed:
# Get the full daemon_url.rs file to understand the complete context
wc -l crates/gcore/src/daemon_url.rsRepository: GobbyAI/gobby-cli
Length of output: 95
🏁 Script executed:
# Read the complete daemon_url.rs file
cat -n crates/gcore/src/daemon_url.rsRepository: GobbyAI/gobby-cli
Length of output: 4137
🏁 Script executed:
# Search for usages of endpoint_to_url to see how it's called
rg -n 'endpoint_to_url' crates/gcore/src/Repository: GobbyAI/gobby-cli
Length of output: 317
🏁 Script executed:
# Search for tests related to daemon_url
fd -n '*.rs' | xargs rg -l 'daemon_url|endpoint_to_url' | grep -i testRepository: GobbyAI/gobby-cli
Length of output: 291
🏁 Script executed:
# Search for bootstrap configuration to understand what values are actually used
rg -n 'bootstrap' crates/gcore/src/ -i --max-count 20Repository: GobbyAI/gobby-cli
Length of output: 3163
🏁 Script executed:
# Search for any test or code using ::1 or other explicit IPv6 addresses
rg -n '::1|fe80::|2001:' crates/gcore/src/Repository: GobbyAI/gobby-cli
Length of output: 43
🏁 Script executed:
# Check bootstrap.rs to understand what values are actually allowed/expected
cat -n crates/gcore/src/bootstrap.rs | head -100Repository: GobbyAI/gobby-cli
Length of output: 4073
🏁 Script executed:
# Search for any documentation on valid bind_host values
rg -n 'bind_host|listen' crates/gcore/src/ -A 2 -B 2 | head -50Repository: GobbyAI/gobby-cli
Length of output: 3289
Add IPv6 literal bracketing to prevent malformed URLs in daemon address composition.
The endpoint_to_url function currently formats IPv6 literals without brackets. If bind_host is set to an explicit IPv6 address like ::1, the resulting URL http://::1:60887 is invalid; IPv6 addresses require bracketed form http://[::1]:60887 in URLs per RFC 3986.
While the dial_host docstring states that IPv6 bracketing "is not handled here" and suggests this case is out of scope, the implementation accepts any arbitrary host string (line 27 returns the result of dial_host, which passes through non-wildcard values unchanged). This inconsistency creates a latent correctness issue for users who configure explicit IPv6 addresses.
The suggested fix is correct: detect IPv6 literals and wrap them in brackets.
Suggested fix
fn endpoint_to_url(endpoint: &DaemonEndpoint) -> String {
let host = dial_host(&endpoint.host);
- format!("http://{host}:{}", endpoint.port)
+ if host.parse::<std::net::Ipv6Addr>().is_ok() {
+ format!("http://[{host}]:{}", endpoint.port)
+ } else {
+ format!("http://{host}:{}", endpoint.port)
+ }
}📝 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.
Suggested change
| fn endpoint_to_url(endpoint: &DaemonEndpoint) -> String { | |
| let host = dial_host(&endpoint.host); | |
| format!("http://{host}:{}", endpoint.port) | |
| } | |
| fn endpoint_to_url(endpoint: &DaemonEndpoint) -> String { | |
| let host = dial_host(&endpoint.host); | |
| if host.parse::<std::net::Ipv6Addr>().is_ok() { | |
| format!("http://[{host}]:{}", endpoint.port) | |
| } else { | |
| format!("http://{host}:{}", endpoint.port) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/gcore/src/daemon_url.rs` around lines 26 - 29, endpoint_to_url
currently concatenates the host from dial_host into the URL without IPv6
brackets; update endpoint_to_url (which takes &DaemonEndpoint and calls
dial_host) to detect IPv6 literals and wrap them in square brackets before
formatting the URL (e.g., if the host contains ':' and is not already bracketed,
produce "[host]"). Ensure you only bracket literal IPv6 forms (avoid bracketing
already-bracketed hosts or wildcard/empty values) and then use the bracketed
host in the existing format!("http://{host}:{}", endpoint.port).
Comment on lines
+128
to
+131
| match req.send_string(&body) { | ||
| Ok(resp) if (200..300).contains(&resp.status()) => { | ||
| let _ = fs::remove_file(enqueued_path); | ||
| DeliveryOutcome::Delivered |
There was a problem hiding this comment.
Don't report success if cleanup failed.
Line 130 drops the remove_file error and still returns Delivered. If the POST succeeded but the inbox file remains, the drain path can replay the same hook and duplicate its side effects.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/ghook/src/transport.rs` around lines 128 - 131, The match arm handling
req.send_string(&body) currently ignores fs::remove_file(enqueued_path) and
always returns DeliveryOutcome::Delivered; change it so you only return
DeliveryOutcome::Delivered if fs::remove_file(enqueued_path) returns Ok. If
remove_file returns Err, log or propagate that error and return a non-delivered
outcome instead (i.e., do not return DeliveryOutcome::Delivered), ensuring the
code paths around req.send_string, enqueued_path, fs::remove_file, and
DeliveryOutcome::Delivered reflect this check.
Comment on lines
+29
to
+33
| pub fn is_passthrough(&self) -> bool { | ||
| self.strategy_name == "passthrough" | ||
| || self.strategy_name == "excluded" | ||
| || self.strategy_name.ends_with("/no-op") | ||
| } |
There was a problem hiding this comment.
is_passthrough() can misclassify real compression for user-defined pipeline names.
Any pipeline named like foo/no-op will be treated as passthrough even when compression actually occurred, because classification is derived from strategy_name text instead of explicit state.
Suggested refactor (state-based classification)
pub struct CompressionResult {
pub compressed: String,
pub original_chars: usize,
pub compressed_chars: usize,
pub strategy_name: String,
+ pub passthrough: bool,
}
@@
pub fn is_passthrough(&self) -> bool {
- self.strategy_name == "passthrough"
- || self.strategy_name == "excluded"
- || self.strategy_name.ends_with("/no-op")
+ self.passthrough
}
}Then set passthrough: true only in the actual passthrough return paths (passthrough, excluded, and the explicit low-savings suppression branch), and false everywhere else.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/gsqz/src/compressor.rs` around lines 29 - 33, is_passthrough currently
infers passthrough from strategy_name strings and can misclassify user-defined
pipelines like "foo/no-op"; change the design to use explicit state: add a
boolean field (e.g., passthrough) to the struct that is set to true only in the
actual passthrough paths (the true passthrough return path, the excluded path,
and the low-savings suppression branch) and false everywhere else, then modify
the is_passthrough() method to return that boolean instead of matching
strategy_name, and ensure any code paths that previously relied on strategy_name
text are updated to set the new passthrough flag where appropriate.
Comment on lines
+68
to
+79
| Composes `http://{host}:{port}` from a bootstrap-derived endpoint, with one rewrite: wildcard listen hosts (`0.0.0.0`, `::`, `::0`) become `127.0.0.1`. Hostnames, named interfaces, and explicit IPv4/IPv6 literals pass through unchanged. | ||
|
|
||
| ```rust | ||
| let url = gobby_core::daemon_url::daemon_url(); | ||
| // "http://127.0.0.1:60887" for default bootstrap | ||
| // "http://10.0.0.5:61234" if bootstrap has bind_host: 10.0.0.5 | ||
| // "http://127.0.0.1:60887" if bootstrap has bind_host: 0.0.0.0 | ||
| ureq::post(&format!("{url}/api/hooks/execute")).send_string(body)?; | ||
| ``` | ||
|
|
||
| Bracketing IPv6 literals for URL embedding is **not** handled here — in practice `bootstrap.yaml` is always `localhost`, an IPv4 literal, or a wildcard. If that ever stops being true, this is the place to add it. | ||
|
|
There was a problem hiding this comment.
Clarify the IPv6 behavior; current wording is internally contradictory.
Line 68 implies explicit IPv6 literals are supported, while Line 78 says IPv6 URL bracketing is not handled. Please state explicitly that raw IPv6 literals are currently not safe for URL composition without bracket handling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/guides/gcore-development-guide.md` around lines 68 - 79, The docs are
contradictory about IPv6: update the paragraph around
gobby_core::daemon_url::daemon_url to explicitly state that raw IPv6 literals
are not safe for URL composition because daemon_url does not perform bracketting
for IPv6 addresses; change the text that currently says "Hostnames, named
interfaces, and explicit IPv4/IPv6 literals pass through unchanged" to clarify
that only hostnames, named interfaces, and explicit IPv4 literals pass through
unchanged, and add a sentence that "explicit IPv6 literals are passed through
verbatim by daemon_url and therefore must be bracketed by the caller (e.g.
[::1]) or converted to a hostname before embedding in a URL; daemon_url does not
add brackets."
|
|
||
| - `[gsqz:passthrough]` — no pipeline matched, fallback truncation applied | ||
| - `[gsqz:low-savings]` — a pipeline matched but achieved less than 5% compression | ||
| - `[gsqz:low-savings]` — a pipeline matched but achieved less than 5% compression. The marker is suppressed when prepending it would actually grow the output beyond the original — gsqz will only annotate when the annotation isn't itself making things worse. |
There was a problem hiding this comment.
Update the earlier overview text too.
This section documents the new low-savings behavior, but Line 61 still says sub-5% results are returned unchanged. The guide now describes two different outcomes for the same case.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/guides/gsqz-user-guide.md` at line 203, The overview still states that
sub-5% compression results are returned unchanged; update that earlier summary
to match the new behavior described later by saying sub-5% compression may be
annotated with the `[gsqz:low-savings]` marker unless prepending the annotation
would increase the output size, in which case the result is left unchanged;
ensure the overview references the `[gsqz:low-savings]` marker and the
conditional suppression rule so both sections are consistent.
Comment on lines
+72
to
+80
| **New in `~/Projects/gobby-cli`:** | ||
| - `crates/ghook/Cargo.toml` | ||
| - `crates/ghook/src/main.rs` — `clap` entry point | ||
| - `crates/ghook/src/envelope.rs` — replay envelope struct + serde | ||
| - `crates/ghook/src/transport.rs` — enqueue-first flow (file write + HTTP) | ||
| - `crates/ghook/src/diagnose.rs` — sandbox probe for `--diagnose` | ||
| - `crates/ghook/src/config.rs` — reads `~/.gobby/bootstrap.yaml` for port | ||
| - `.github/workflows/release-ghook.yml` — mirrors `release-gcode.yml` | ||
| - Root `Cargo.toml` — add `ghook` to workspace members, opt level "z" |
There was a problem hiding this comment.
Mark this plan as superseded or reconcile it with the shipped architecture.
Several load-bearing details here no longer match the code in this PR: the plan still points to crates/ghook/src/config.rs, top-level gobby-cli/schemas/*, env/current.json header sourcing, and double-fork detach, while the implementation now uses crates/gcore/src/bootstrap.rs, crates/ghook/schemas/*, stdin + project walk-up, and a single post-enqueue detach path. Leaving both versions in-tree will send follow-up work toward the wrong modules and contracts.
Also applies to: 208-216, 293-321
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/sandbox-tolerant-hooks.md` around lines 72 - 80, This plan is out
of date and must be marked superseded or reconciled with the shipped
implementation: update the document to reference the actual modules and
contracts used (replace mentions of crates/ghook/src/config.rs, top-level
gobby-cli/schemas/*, env/current.json header sourcing, and double-fork detach
with the current crates/gcore/src/bootstrap.rs, crates/ghook/schemas/*, stdin +
project walk-up input contract, and the single post-enqueue detach flow), and
either remove or clearly annotate the superseded sections (including the other
affected ranges around 208-216 and 293-321) so future work targets the correct
files and APIs.
This was referenced Apr 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Multi-crate release: introduces the
gobby-coreandgobby-hookscrates,bumps
gcodeto 0.6.0 andgsqzto 0.4.1, and updates docs + CIaccordingly.
glocis unchanged.gobby-coregobby-hooksgobby-codegobby-squeezeWhat's in the release
gobby-core— shared primitives (project root walk-up, bootstrap config, daemon URL) consumed by every binary. (#112, #113, #117)gobby-hooks/ghook— sandbox-tolerant hook dispatcher with spool-then-POST semantics, v1 envelope schema, diagnose mode, and per-CLI registry for claude/codex/gemini/qwen. (#114, #117)gobby-corehelpers; FTS5 LIKE escape hardened;graph.rsunresolved-response building deduped. (#115, #118)[gsqz:low-savings]marker suppressed when adding it would grow output; outer[Output compressed by gsqz — …]header also suppressed for the resulting*/no-opstrategy;CompressionResult::is_passthrough()classifies passthrough/excluded/*-no-op together. (#111, #121)gcode-v*,gsqz-v*,gloc-v*,gcore-v*,ghook-v*); addedrelease-gobby-coreworkflow; GitHub releases for binary crates gated on successful crates.io publish. (#110, #116)ghook-user-guide.md,ghook-development-guide.md,gcore-development-guide.md; touched upgcode-development-guide.mdandgsqz-user-guide.md; README now surfaces ghook + gobby-core; CHANGELOG has four versioned sections. (#119, #120)Release order after merge
Tag
gcore-v0.1.0first sogobby-corelands on crates.io before thegcode publish step needs it. Then tag the other three together.
Test plan
cargo build --workspace --no-default-featurescleancargo test --workspace --no-default-features— 268 tests passing (gcore 13, ghook 27, gcode 44, gsqz 149, gloc 35)cargo clippy --workspace -- -D warningscleancargo fmt --all --checkcleancargo package -p gobby-coresucceedscargo package -p gobby-codepending ongobby-corebeing on crates.io (expected — staged release order handles this)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores