Release v0.4.9

[joshwilhelmi]

Summary

• Prepare the 0.4.9 release changelog
• Include pre-push validation fixes and FastAPI 0.136.3 exclusion
• Release branch includes PostgreSQL-only runtime, build/verification gate, memory recall helper, and agent/session reliability work

Validation

• ./pre-push-test.sh exited 0
• pytest report: 14476 passed, 4869 skipped, 3 xfailed
• pip-audit: no known vulnerabilities found
• test-quality audit: no high-severity new issues

Summary by CodeRabbit

• New Features

  • PostgreSQL runtime hub and FalkorDB graph backend; Grok CLI support; new Postgres management CLI and manual release gate.

• Changes

  • Search now uses pg_search (BM25); memory/search stack moved to PostgreSQL + Qdrant + FalkorDB.
  • Build command adds planning-seed, completed-review-rounds, coordinator and dry-run; new build observability endpoints.

• Bug Fixes

  • Improved agent, session, terminal and task-recovery reliability.

• Tests / CI

  • Local Postgres test harness, CI workflow refinements and a release verification tool.

[claude]

Code Review: Release v0.4.9

Note: This PR spans 1,565 changed files (115k additions, 45k deletions). The GitHub diff API truncates at 300 files, so source code changes are only reviewable by fetching the branch locally. This review covers what is visible via the API: CI/CD workflows, documentation, changelog, and project config.

---

Overview

A substantial release PR bundling the PostgreSQL-only runtime migration, FalkorDB (replacing Neo4j), a new memory-recall-helper agent, build/review completion gates, Grok CLI provider support, and broad agent/session reliability work. Validation reported: 14,476 passed, 4,869 skipped, 3 xfailed, pip-audit clean.

---

Issues

🔴 Critical — Duplicate workflow name "CI"

Both ci.yml and postgres-pgsearch-smoke.yml declare name: CI. The release.yml verify-ci gate calls:

python scripts/ci/verify_ci_gate.py --workflow "CI"

With two workflows sharing the same name, the gate's behavior against the GitHub API is non-deterministic — it may match the wrong run, find multiple matches, or silently succeed/fail based on whichever run is returned first. Rename postgres-pgsearch-smoke.yml to something distinct (e.g., name: Release Branch CI) before merging.

🟡 Medium — postgres-pgsearch-smoke.yml fires on every push to every branch

on:
  push:
  workflow_dispatch:

No branch filter is set, so this workflow runs the full pre-push-test.sh suite for every commit on every branch in the repo. The concurrency group (release-branch-ci-${{ github.ref }}) prevents pile-up per branch but does not prevent unnecessary runs on short-lived feature branches. Add a branch filter (e.g., branches: ["0.4.*", "main"] or tags: ["v*"]) to limit execution scope.

🟡 Medium — CHANGELOG skips 0.4.7 and 0.4.8

The changelog jumps from [0.4.6] directly to [0.4.9]. If 0.4.7 and 0.4.8 were internal or skipped releases, add a note in the changelog explaining this so future contributors don't assume missing entries are a mistake.

🟠 Minor — Inconsistent --build-arg quoting across jobs

The test job in ci.yml quotes only the value:

--build-arg PG_SEARCH_VERSION="${GOBBY_PG_SEARCH_VERSION}"

The build job quotes the entire argument:

--build-arg "PG_SEARCH_VERSION=${GOBBY_PG_SEARCH_SHA256}"

postgres-pgsearch-smoke.yml matches the build job style. These are functionally equivalent but should be consistent. The whole-argument quoting style is marginally safer when values contain spaces. Align all three to the same style.

---

Security

Positive:

• FastAPI 0.136.3 correctly excluded for MAL-2026-4750; locked to 0.136.1. ✅
• release.yml verify-ci step uses a pinned SHA for actions/checkout (34e114876...) rather than a floating @v4 tag — good supply-chain hygiene. ✅
• PyPI publishing uses OIDC trusted publishing (id-token: write, no stored token). ✅
• PostgreSQL test container uses --tmpfs /var/lib/postgresql so test data is ephemeral and never persisted to disk. ✅

Informational — two CVEs suppressed in pip-audit with explanatory comments:

• CVE-2025-69872: diskcache 5.6.3 (transitive via litellm) — no fix available
• CVE-2026-4539: pygments 2.19.2 (latest) — no fix available

These are documented and acceptable, but worth re-evaluating when upstream fixes land.

---

CI Quality

Positive:

• GOBBY_TEST_PROTECT=1 is set in all relevant test jobs, preventing daemon interference. ✅
• Python matrix covers ["3.13", "3.14"] with fail-fast: false. ✅
• pgaudit is enabled in the test container configuration with appropriate log rotation and file permissions (0640). ✅
• Docker image build includes SHA256 pinning for the pg_search extension, verified at build time. ✅
• docker-compose.test.yml provides a local equivalent of the CI test container setup. ✅

Minor:

• The test job in ci.yml and the postgres-pgsearch-smoke.yml both rebuild the Docker image from scratch on every run with no --cache-from or layer caching. Consider publishing/pulling the test image to a registry or using GitHub's Docker layer cache to reduce build time.
• The build job in ci.yml defensively runs docker rm -f before starting the container; the test job does not. Low risk (ephemeral runners), but worth aligning for consistency.

---

Changelog Quality

The 0.4.9 entry is well-structured and follows Keep a Changelog conventions. The security section clearly documents the FastAPI exclusion with the MAL reference. The release section documents the version bump and lockfile regeneration — both important callouts for release PRs.

---

Overall Assessment

The work in this PR looks well-validated (passing test suite with explicit report, pip-audit clean). The duplicate "CI" workflow name is the one issue that warrants fixing before merge, as it directly affects the release gate. The unfiltered push trigger in postgres-pgsearch-smoke.yml is an operational cost concern worth addressing. Everything else is low-risk polish.

🤖 Generated with Claude Code

[claude]

Code Review: Release v0.4.9

This is a large release PR (1565 files, +115K/-45K lines) covering a PostgreSQL-only runtime migration, FalkorDB replacing Neo4j, Grok CLI provider support, build/review completion gates, and significant agent/session reliability work. Given the scope, I focused on the key new modules rather than every touched file.

---

Overview

The PR is well-scoped for a release branch. The changelog accurately describes the changes. Pre-push validation passed (14,476 tests, no known vulnerabilities). The architectural direction — completing the SQLite→PostgreSQL migration and consolidating adapter patterns — is sound.

---

Bugs

version.json / Dockerfile arch-mismatch (src/gobby/data/postgres-pgsearch/)

The top-level pg_search_sha256 field in version.json contains only the amd64 hash. Both the CI workflow (ci.yml, postgres-pgsearch-smoke.yml) and the Dockerfile use this single value via jq -r '.pg_search_sha256'. On an arm64 builder or runner, the SHA256 verification step will either pass the wrong hash (silently accepting the wrong binary) or fail the build — depending on which hash ends up in PG_SEARCH_SHA256. The pg_search_sha256_by_arch map is present in version.json but never read.

# ci.yml "Load Postgres test image metadata" step should read:
- pg_search_sha256="$(jq -r '.pg_search_sha256' src/gobby/data/postgres-pgsearch/version.json)"
+ arch="$(dpkg --print-architecture 2>/dev/null || uname -m | sed 's/x86_64/amd64/;s/aarch64/arm64/')"
+ pg_search_sha256="$(jq -r --arg arch "$arch" '.pg_search_sha256_by_arch[$arch]' src/gobby/data/postgres-pgsearch/version.json)"

The same change is needed in the Dockerfile ARG/RUN chain, or the Dockerfile should detect arch internally.

post_tool_use_failure collapses into AFTER_TOOL (src/gobby/adapters/capabilities.py)

In GROK_EVENT_MAP, both post_tool_use and post_tool_use_failure map to HookEventType.AFTER_TOOL. Any rule or workflow that checks event_type == AFTER_TOOL cannot distinguish a tool failure from a success for Grok sessions. The other providers don't expose a failure variant, so this may be intentional, but it should be called out explicitly (a comment, or a dedicated AFTER_TOOL_FAILURE event type) to prevent silent information loss.

---

Code Quality

sql_dialect.py — dead-code stubs (src/gobby/storage/sql_dialect.py)

The migration to PostgreSQL-only is complete, but these helpers carry vestigial branches:

def dialect_of(db: object) -> StorageDialect:
    dialect = getattr(db, "dialect", "postgres")
    if dialect == "postgres":          # always true, branch below unreachable
        return cast(StorageDialect, dialect)
    return "postgres"                  # unreachable

def is_postgres(db: object) -> bool:
    return True                        # always True, callers can inline this

dialect_of() should be removed entirely (or simplified to just return "postgres"), and is_postgres() should be deleted — call sites should drop the guard condition outright. Keeping them risks confusion about whether multi-dialect support is a future goal.

dispatch/merge_recovery.py — one-liner module

WORKSPACE_MERGE_CONFLICT_LABEL = "merge-recovery:workspace-conflict"

A file with a single string constant is unusual. If this is scaffolding for future work, it should carry a # TODO comment pointing to the relevant task. If the constant has callsites, fold it into the module that uses it; if not, it should be removed.

acp_hook_adapter.py — stale docstring

The class docstring opens with "Translates between Gemini CLI's native hook format" but ACPHookAdapter is now the base class for both Gemini and Grok adapters (and potentially others). Update the docstring to describe its ACP-style generic role.

---

Security

postgres-pgsearch-smoke.yml — missing branch filter on push trigger

on:
  push:              # triggers on ALL pushes, including forked PRs
  workflow_dispatch:

Without a branch filter, this workflow fires on every push to every branch, including contributors' fork branches that run in the full repository context. This could expose secrets. Add:

on:
  push:
    branches: [main, "0.4.9"]
  workflow_dispatch:

SQL injection guards — correctly applied throughout. The nosec B608 annotations in write_set_guard.py and claim_recovery.py are accompanied by accurate comments explaining why the code is safe. Good practice.

Subprocess in claim_recovery.py — ["git", "status", "--porcelain"] with fixed args and a cwd from an internal database field is fine; the nosec B603 annotation is accurate.

---

Performance

DispatchWriteSetGuard._files_for_task — each call issues a SELECT DISTINCT file_path FROM task_affected_files WHERE task_id = ? query per candidate task in a heartbeat. The result is cached in _task_files, so per-heartbeat this is bounded to one query per unique task. This is acceptable. The load() classmethod fetches all active write sets in one query, so the overall query count per heartbeat is 1 (load) + N (lazy per-task lookups on conflict checks). Worth noting if the dispatcher ever processes large backlogs.

---

Tests

The PR includes new test files covering FalkorDB memory, the setup wizard, chat transport hooks, CSS token integrity, and tmux text injection. The validation report (14,476 passed, 4,869 skipped) and pip-audit clean pass are reassuring.

One gap: there are no tests for DispatchWriteSetGuard.load() with an overlapping write set across two tasks (the conflict detection path). A unit test asserting that conflict_for(task_b) returns an overlap when task_a already holds a shared file would protect against regressions in the most critical logic in that module.

---

Minor

• migration 261 backfills implementation_domain using a heuristic regex on task title/description. This is a best-effort one-time pass, which is fine — but tasks already closed before the migration runs will be excluded (AND closed_at IS NULL), leaving historical data unclassified. If analytics need complete coverage, a follow-up pass over closed tasks would be needed.
• pg_audit_export.sh includes macOS-specific date -j fallback paths that are dead code inside the Linux Docker image. Not a problem, but worth noting if the script is also used on developer machines for local audit exports.

---

Summary

Category	Severity	Count
Bug	High	1 (arch SHA mismatch)
Bug	Medium	1 (Grok failure event collapse)
Security	Medium	1 (smoke workflow branch filter)
Code quality	Low	3 (dead stubs, thin module, stale docstring)
Test gap	Low	1 (write-set conflict path)

The arch SHA mismatch is the only item I'd consider a blocker — it will silently use the wrong hash or break arm64 builds. The rest are improvements for follow-up tasks.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (2)

src/gobby/autonomous/stuck_detector.py (1)

58-60: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use the hub transaction boundary for these writes.

These paths still call db.execute(...) directly. Please wrap them in with db.transaction() as conn: and run conn.execute(...) so the migration, insert, and delete all follow the Hub DB contract consistently.

🛠️ Suggested pattern

-        db.execute(
-            "UPDATE task_selection_history SET context = $1 WHERE id = $2",
-            (context_json, row["id"]),
-        )
+        with db.transaction() as conn:
+            conn.execute(
+                "UPDATE task_selection_history SET context = $1 WHERE id = $2",
+                (context_json, row["id"]),
+            )

As per coding guidelines: "Use the hub database transaction boundary and project-standard $N placeholders for database access in Python code."

Also applies to: 185-198, 389-393

🤖 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/autonomous/stuck_detector.py` around lines 58 - 60, Replace direct
db.execute(...) calls in src/gobby/autonomous/stuck_detector.py with the hub
transaction boundary: wrap each write in "with db.transaction() as conn:" and
call conn.execute(...) so the update/insert/delete participate in the Hub DB
contract; specifically change the db.execute at the shown UPDATE (and the
similar blocks around lines 185-198 and 389-393) to use the transaction context
and conn.execute, and ensure you continue to use the project-standard $N
parameter placeholders (e.g., $1, $2) when passing parameters.

src/gobby/agents/idle_check_handler.py (1)

289-301: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Redact the sibling Codex diagnostic log too.

This warning is safe now, but _log_recent_codex_response_items() below still logs json.dumps(items, ...), and each item still carries the raw transcript payload. The reprompt/fail paths will keep leaking sensitive transcript content until the same redaction is applied there as well.

🤖 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/agents/idle_check_handler.py` around lines 289 - 301, The
_log_recent_codex_response_items function still logs full items including the
sensitive 'raw' transcript; update that function to mirror the redaction used
above by mapping items to redacted_items (keeping only line_num, timestamp,
payload_type or whichever safe fields you used in the watchdog) and call
json.dumps(redacted_items, ensure_ascii=True) in the logger call instead of
json.dumps(items, ...); locate the _log_recent_codex_response_items method and
replace the direct use of items with the sanitized list (omitting the 'raw'
payload) so reprompt/fail paths no longer leak transcripts.

🤖 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 `@src/gobby/agents/agent_health.py`:
- Around line 126-133: The call to self._clear_tmux_session_name(run) can raise
and abort the outer handler, skipping cleanup_agent(run) for an
already-terminated run; wrap the await self._clear_tmux_session_name(run) call
in a try/except that catches Exception, logs the failure (e.g. logger.exception
or logger.warning with run.id and error), and then continues so that
cleanup_agent(...) is still executed for that run; update the block around
result.get("success") in agent_health.py to perform this best-effort clearing
without letting exceptions propagate.

---

Duplicate comments:
In `@src/gobby/agents/idle_check_handler.py`:
- Around line 289-301: The _log_recent_codex_response_items function still logs
full items including the sensitive 'raw' transcript; update that function to
mirror the redaction used above by mapping items to redacted_items (keeping only
line_num, timestamp, payload_type or whichever safe fields you used in the
watchdog) and call json.dumps(redacted_items, ensure_ascii=True) in the logger
call instead of json.dumps(items, ...); locate the
_log_recent_codex_response_items method and replace the direct use of items with
the sanitized list (omitting the 'raw' payload) so reprompt/fail paths no longer
leak transcripts.

In `@src/gobby/autonomous/stuck_detector.py`:
- Around line 58-60: Replace direct db.execute(...) calls in
src/gobby/autonomous/stuck_detector.py with the hub transaction boundary: wrap
each write in "with db.transaction() as conn:" and call conn.execute(...) so the
update/insert/delete participate in the Hub DB contract; specifically change the
db.execute at the shown UPDATE (and the similar blocks around lines 185-198 and
389-393) to use the transaction context and conn.execute, and ensure you
continue to use the project-standard $N parameter placeholders (e.g., $1, $2)
when passing parameters.

🪄 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: 2eaa24b9-a92e-4dad-bbe2-d844246e3048

📥 Commits

Reviewing files that changed from the base of the PR and between 9a1d793 and 1df2674.

⛔ Files ignored due to path filters (1)

• .gobby/tasks.jsonl is excluded by !**/.gobby/tasks.jsonl

📒 Files selected for processing (32)

• .coderabbit.yaml
• .github/coderabbit.yaml
• .github/workflows/ci.yml
• .github/workflows/postgres-pgsearch-smoke.yml
• .github/workflows/release.yml
• .gobby/memories.jsonl
• QWEN.md
• docs/architecture/source-tree.md
• docs/contracts/plan-coverage.md
• docs/research/daemon-enhancements.md
• pre-push-test.sh
• scripts/ci/verify_ci_gate.py
• scripts/migrate_index_to_plans_table.py
• src/gobby/adapters/acp_client_requests.py
• src/gobby/adapters/acp_hook_adapter.py
• src/gobby/adapters/codex_impl/app_server_adapter.py
• src/gobby/agents/agent_cleanup.py
• src/gobby/agents/agent_health.py
• src/gobby/agents/code_index.py
• src/gobby/agents/constants.py
• src/gobby/agents/idle_check_handler.py
• src/gobby/agents/isolation.py
• src/gobby/agents/isolation_git_hygiene.py
• src/gobby/agents/python_env_seed.py
• src/gobby/agents/sandbox.py
• src/gobby/agents/session.py
• src/gobby/agents/spawn_executor.py
• src/gobby/agents/step_workflow.py
• src/gobby/agents/task_recovery.py
• src/gobby/agents/tmux/spawner.py
• src/gobby/agents/tmux/text_injection.py
• src/gobby/autonomous/stuck_detector.py

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make tmux-session-name clearing best-effort after a successful kill.

If _clear_tmux_session_name() raises here, the outer handler skips cleanup_agent(...) for a run that was already terminated, leaving stale agent state behind. Catch/log that bookkeeping failure locally and continue with the main cleanup path.

Suggested fix

                     if not result.get("success"):
                         logger.warning(
                             "Skipping cleanup for run %s after failed terminal kill: %s",
                             run.id,
                             result.get("error") or result.get("message"),
                         )
                         continue
-                    await self._clear_tmux_session_name(run)
+                    try:
+                        await self._clear_tmux_session_name(run)
+                    except Exception:
+                        logger.warning(
+                            "Failed clearing tmux session name for run %s after kill",
+                            run.id,
+                            exc_info=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/agents/agent_health.py` around lines 126 - 133, The call to
self._clear_tmux_session_name(run) can raise and abort the outer handler,
skipping cleanup_agent(run) for an already-terminated run; wrap the await
self._clear_tmux_session_name(run) call in a try/except that catches Exception,
logs the failure (e.g. logger.exception or logger.warning with run.id and
error), and then continues so that cleanup_agent(...) is still executed for that
run; update the block around result.get("success") in agent_health.py to perform
this best-effort clearing without letting exceptions propagate.

[claude]

Code Review — Release v0.4.9

Note: This PR contains 1,575 changed files (+116,589 / -45,397 lines), exceeding GitHub's 300-file diff API limit. This review covers the CI/CD workflows, CHANGELOG, and docker-compose configuration that are accessible. Source code changes in `src/` and `web/` could not be reviewed in full — additional per-module reviews are recommended before merge.

---

Overview

This is a substantial release PR covering:

• PostgreSQL-only runtime (SQLite paths removed)
• Memory recall helper agent
• Build/review completion gates and coordinator
• Agent/session lifecycle hardening
• FastAPI 0.136.3 exclusion (MAL-2026-4750)
• New CI: postgres-pgsearch smoke workflow

The CHANGELOG is well-structured and follows Keep a Changelog conventions. Validation evidence (pre-push exit 0, 14476 tests passed, pip-audit clean) is noted in the PR description — good practice for a release PR.

---

CI/CD — .github/workflows/

🔴 Critical: Workflow name collision breaks the release gate

postgres-pgsearch-smoke.yml declares name: CI (line 1), identical to the main ci.yml. The release.yml verify-ci step calls:

python scripts/ci/verify_ci_gate.py --workflow "CI" --sha "$GITHUB_SHA" ...

If verify_ci_gate.py resolves workflow runs by name, the smoke-only workflow could satisfy the gate before the full CI suite completes, allowing a release to proceed without full test coverage. The smoke workflow also triggers on every push with no branch filter (on: push), so it will fire on release tags.

Fix: Rename the smoke workflow (name: Release Branch CI is already used as the job name — use that or Release Branch Smoke), or have verify_ci_gate.py match by workflow filename rather than display name, or add an explicit filename filter to the script.

🟡 Medium: test job container not idempotent on retry

ci.yml test job (lines 194–228) starts the Postgres container without first removing any existing container. The build job (line 351) correctly prefixes with docker rm -f ... || true. If the test job runner reuses a workspace (e.g., self-hosted) or retries after a partial failure, the docker run will fail with a name conflict.

Fix: Add docker rm -f "${GOBBY_POSTGRES_TEST_CONTAINER}" >/dev/null 2>&1 || true before docker run -d in the test job, matching the build job pattern.

🟡 Medium: release.yml gate_only default is true but condition checks !inputs.gate_only

inputs:
  gate_only:
    default: true
    type: boolean
...
if: ${{ github.event_name == 'push' && !inputs.gate_only }}

For tag pushes (github.event_name == 'push'), inputs.gate_only is null/undefined (no workflow_dispatch inputs), so !inputs.gate_only evaluates to true, which is the intended behavior. However, the intent is implicit and could confuse future editors. Consider inputs.gate_only != true or an explicit github.event_name == 'push' && github.ref_type == 'tag' check.

🟢 Positive CI patterns

• GOBBY_TEST_PROTECT=1 is set globally in the test env block — correctly prevents test isolation bypasses.
• Python 3.13 + 3.14 matrix provides forward-compatibility signal early.
• persist-credentials: false on the release checkout is a good security practice.
• --tmpfs /var/lib/postgresql on test containers prevents cross-run state leakage.
• --cov-fail-under=80 is enforced inline in the pytest command.
• Codecov upload uses fail_ci_if_error: false — upload failures don't block CI.

---

docker-compose.test.yml

Well-structured. tmpfs mount for PostgreSQL data is correct for a test harness (no persistence, fast I/O). Health check with pg_isready and 15 retries at 2s intervals is reasonable. One note: the default PG_SEARCH_SHA256 value in the compose file should stay in sync with src/gobby/data/postgres-pgsearch/version.json — drift here would cause local docker-compose up to pull a different image than CI.

---

CHANGELOG.md

• Well-structured and follows Keep a Changelog 1.1.0 format.
• The Security entry for FastAPI 0.136.3 (MAL-2026-4750) is appropriately terse — good.
• The ci.yml security job ignores CVE-2026-4539 (pygments) with comment "no fix available." Future-dated CVEs like this should include an expected fix timeline or a link to the upstream issue to prevent it from being silently left indefinitely. Consider adding a # TODO: revisit after pygments > 2.19.2 style note.
• Versions jump from 0.4.9 to 0.4.6 in the file — 0.4.7 and 0.4.8 entries appear to be absent. If they were patch releases that were superseded, a note to that effect would help readers following the history.

---

Summary

Severity	Finding
🔴 Critical	Workflow name "CI" collision between ci.yml and postgres-pgsearch-smoke.yml may allow the release gate to pass on the smoke run alone
🟡 Medium	test job Postgres container start is not idempotent — missing docker rm -f guard
🟡 Medium	gate_only condition logic is implicitly correct but brittle — worth making explicit
🟢 Info	CVE-2026-4539 ignore in ci.yml could use a tracking note
🟢 Info	CHANGELOG missing 0.4.7 / 0.4.8 entries (if applicable)

The scope of non-reviewable source changes (+116k lines in 1,475 Python/TS files) is the primary concern for a PR of this type. The PR description's validation evidence is helpful, but does not substitute for per-module review of the PostgreSQL migration paths, the new memory recall agent, and the build coordinator changes. Recommend breaking future releases into multiple smaller PRs or adding per-subsystem review checklists.

🤖 Generated with Claude Code

[claude]

Code Review: Release v0.4.9

This is a large release PR (1,574 files, +116K/-45K lines). Review focuses on the novel code paths and areas of highest risk.

---

Overview

0.4.9 ships four significant areas:

1. PostgreSQL-only runtime — full removal of the SQLite/keyring path, new gobby postgres CLI, pgAudit validation gate, pg_search (BM25) via Dockerfile.
2. Memory recall helper — backgrounded Haiku agent that surfaces relevant memories at turn start, with dedup, stale-cancellation, and freshness guards.
3. Build/review gates — coordinator session tracking, dry-run, planning_seed_state, and a new build-observability endpoint family.
4. Agent/dispatch reliability — _agent_dispatchable() replaces _has_agent(), _development_agent() gains IMPLEMENTATION_DOMAINS routing, workspace merge conflict label guard.

---

Issues & Suggestions

🐛 Bug: dead mode branch in grok.py

src/gobby/cli/installers/grok.py:71

content_path = project_path / ".grok" if mode == "global" else project_path / ".grok"

Both branches of the ternary are identical — mode has no effect. This looks like an incomplete implementation where the "project" path was supposed to differ. If per-project vs. global install paths are not yet differentiated, the ternary should be removed and a comment added; otherwise this is a silent logic error.

---

🐛 Weak type annotation in _postgres_connection()

src/gobby/cli/postgres.py

def _postgres_connection() -> Any:

This returns a psycopg.Connection, which is used as a context manager via with _postgres_connection() as conn. Returning Any loses all type safety on conn. Use psycopg.Connection[Any] or the relevant protocol type. The # nosec annotation is appropriate on the subprocess call, but the type annotation should be tightened.

---

⚠️ ClassVar on frozen dataclasses is not inherited correctly

src/gobby/storage/hub/protocol.py

@dataclass(frozen=True)
class TaskSeqAllocation:
    PRIORITY: ClassVar[int] = 200
    project_id: str

ClassVar fields are not inherited by subclasses and do not participate in LockTarget protocol matching via structural subtyping — Protocol with ClassVar is a known mypy/pyright edge case. The LockTarget Protocol declares PRIORITY: ClassVar[int] which mypy can verify on concrete dataclasses, but runtime checks (e.g., isinstance) will not enforce it. If the intent is to enforce priority ordering at runtime, the acquire_additional_lock docstring already requires LockTarget.PRIORITY comparison — verify that the PostgreSQL adapter actually reads type(lock).PRIORITY rather than lock.PRIORITY (the latter will not find ClassVar on an instance).

---

⚠️ write_postgres_defaults writes DSN in plaintext

src/gobby/config/postgres_bootstrap.py:40

data["database_url"] = database_url
data.pop("database_url_ref", None)

This explicitly removes the keyring reference and writes the raw DSN (which may contain a password) to ~/.gobby/bootstrap.yaml. The KEYRING_DATABASE_URL_REF constant exists precisely to avoid this. The docker-install path may be using a local socket or a passwordless URI, which makes this acceptable there, but for native and external modes where the DSN can contain credentials, this should enforce keyring storage (or at minimum warn). clear_postgres_fields has the right guard (_require_postgres_runtime_bootstrap) but write_postgres_defaults does not distinguish by mode.

---

⚠️ Hardcoded 48-hour cutover deadline

src/gobby/cli/postgres.py

"deadline_at": (activated_at + timedelta(hours=48)).isoformat(timespec="seconds"),

The validation-window deadline is hardcoded to 48 hours. For teams with weekend or on-call constraints, this window may expire before recovery is possible. Consider making this configurable via a flag or bootstrap field, or at minimum documenting the rationale for 48 hours.

---

ℹ️ memory-recall-helper.yaml: instruction comment will become stale

src/gobby/install/shared/workflows/agents/memory-recall-helper.yaml:71

# Note that HEAD's `send_message` signature is `(from_session, target, content, ...)`

Inline API documentation in agent YAML instructions ages poorly — when the signature changes, this comment will silently mislead the agent. The comment about "the runtime change made in 2.1" and "Phase 2.2's enforcement reorder" is good internal rationale during development but will be confusing six months from now. Consider trimming the historical notes and keeping only the current behavioral contract.

---

ℹ️ ChatAttachmentMutation priority ordering

src/gobby/storage/hub/protocol.py

class SessionVariableMutation:
    PRIORITY: ClassVar[int] = 950
    
class ChatAttachmentMutation:
    PRIORITY: ClassVar[int] = 900

ChatAttachmentMutation (900) has a lower priority than SessionVariableMutation (950), but appears below it in the file — the file ordering suggests 900 should sort before 950, yet the class appears after. Not a bug in itself, but the ordering inconsistency between file order and priority value makes it harder to reason about the lock acquisition chain. Consider sorting by priority value.

---

Positive Highlights

• _agent_dispatchable() is a clean, correct fix. Gating on agent_slug in PROMPT_BUILDERS prevents silent dispatch failures when an agent lacks a prompt builder — this is exactly the right invariant to enforce at the routing layer.

• _write_cutover_ticket() uses os.replace() for atomic write with a .tmp intermediary — correct POSIX pattern.

• pgAudit probe in _probe_pgaudit_or_fail() is thorough: checks extension presence, shared_preload_libraries, pgaudit.log setting, and performs an actual write probe with a UUID token verified against the Docker log. This is a solid validation gate.

• subprocess nosec annotations are accurate and scoped: B404 on the import (acknowledged), B603/B607 on the docker exec call with a comment explaining why it's safe (fixed command list, no shell, probe token is shlex.quote-escaped).

• _task_has_label() helper correctly handles None from _field(task, "labels", ()) via the or () guard.

• CHANGELOG is accurate and well-structured; the [Unreleased] section above 0.4.9 will naturally roll forward to the next cycle.

• FastAPI MAL-2026-4750 exclusion is handled at the right level (version_pins.py / lockfile) with an explicit tracking note.

---

Test Coverage Note

The PR passes 14,476 tests with 4,869 skipped. The new gobby postgres CLI commands (activate, backup, restore) are integration-heavy and the unit coverage for the cutover-ticket builder logic (_build_cutover_ticket, _parse_capture_sink, _wal_archive_slot_name) is not obvious from the file listing. These are good candidates for isolated unit tests given the branching on capture_kind and the error paths.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

.github/workflows/release.yml (1)

9-13: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

gate_only is currently a no-op due to push-only job conditions.

Line 41 (and similarly Lines 68, 91, 115) requires github.event_name == 'push', so workflow_dispatch can never run release jobs regardless of inputs.gate_only. That makes the new input misleading and non-functional.

Proposed fix

-    if: ${{ github.event_name == 'push' && !inputs.gate_only }}
+    if: ${{ github.event_name == 'push' || (github.event_name == 'workflow_dispatch' && !inputs.gate_only) }}

-    if: ${{ github.event_name == 'push' && !inputs.gate_only }}
+    if: ${{ github.event_name == 'push' || (github.event_name == 'workflow_dispatch' && !inputs.gate_only) }}

-    if: ${{ github.event_name == 'push' && !inputs.gate_only }}
+    if: ${{ github.event_name == 'push' || (github.event_name == 'workflow_dispatch' && !inputs.gate_only) }}

-    if: ${{ github.event_name == 'push' && !inputs.gate_only }}
+    if: ${{ github.event_name == 'push' || (github.event_name == 'workflow_dispatch' && !inputs.gate_only) }}

Also applies to: 41-42, 68-69, 91-92, 115-116

🤖 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 @.github/workflows/release.yml around lines 9 - 13, The new input gate_only
is currently ineffective because the job-level if conditions restrict runs to
github.event_name == 'push'; update each job if-condition (the ones guarding
release jobs) to also allow workflow_dispatch so the input can take effect —
e.g., change checks from "github.event_name == 'push'" to "(github.event_name ==
'push' || github.event_name == 'workflow_dispatch')" and then honor
inputs.gate_only within the job logic (or add a combined expression that
evaluates inputs.gate_only when event is workflow_dispatch). Ensure you update
all occurrences that currently use github.event_name == 'push' so gate_only
works for workflow_dispatch-triggered runs.

🤖 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.

Outside diff comments:
In @.github/workflows/release.yml:
- Around line 9-13: The new input gate_only is currently ineffective because the
job-level if conditions restrict runs to github.event_name == 'push'; update
each job if-condition (the ones guarding release jobs) to also allow
workflow_dispatch so the input can take effect — e.g., change checks from
"github.event_name == 'push'" to "(github.event_name == 'push' ||
github.event_name == 'workflow_dispatch')" and then honor inputs.gate_only
within the job logic (or add a combined expression that evaluates
inputs.gate_only when event is workflow_dispatch). Ensure you update all
occurrences that currently use github.event_name == 'push' so gate_only works
for workflow_dispatch-triggered runs.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8e025e65-f410-4e41-8d2b-f4f64b9bffe2

📥 Commits

Reviewing files that changed from the base of the PR and between 0a796ba and 42c29fd.

⛔ Files ignored due to path filters (1)

• .gobby/tasks.jsonl is excluded by !**/.gobby/tasks.jsonl

📒 Files selected for processing (2)

• .github/workflows/release.yml
• .gobby/memories.jsonl

[claude]

Code Review: Release v0.4.9

This is a large release PR (1,583 files, +117k/-45k lines). Given the diff is too large for the GitHub API to serve in full, this review focuses on the files available: the CHANGELOG, CI/CD workflows, docker-compose test harness, and the PR description itself.

---

Overview

0.4.9 is a significant infrastructure release: PostgreSQL-only runtime (removing SQLite paths), FalkorDB replacing Neo4j, pg_search (BM25) for search, Grok CLI support, and a raft of agent/session reliability fixes. The scope is well-summarized in the CHANGELOG, which is high quality and follows Keep a Changelog format.

---

CI/CD — Findings

Bug: test job missing docker rm -f guard (ci.yml:196)

The build job (line 350) and postgres-pgsearch-smoke.yml (line 60) both include:

docker rm -f "${GOBBY_POSTGRES_TEST_CONTAINER}" >/dev/null 2>&1 || true

before starting the Postgres container. The test job (line 196) does not. On ephemeral GitHub-hosted runners this is fine, but on any self-hosted runner this would cause the test job to fail when a previous container with the same name is still running. Given the investment in PostgreSQL infra in this release, this should be consistent.

postgres-pgsearch-smoke.yml runs on every push without a branch filter

on:
  push:
  workflow_dispatch:

This file is clearly intended as the release-branch CI, but on: push with no branches: filter means it triggers on every push to any branch in the repo. The concurrency group release-branch-ci-${{ github.ref }} will prevent pile-ups per branch, but the Docker build step (which builds a Postgres image with pg_search) will still run on unrelated branches. Consider adding branches: ["0.4.9", "main"] or scoping to the release branch explicitly.

Duplicated Postgres container setup across three jobs

The full container-start sequence (load metadata, docker build, smoke audit-export helper, docker run with health-check loop) is copy-pasted into test, build, and postgres-pgsearch-smoke.yml. GitHub Actions doesn't have reusable job steps natively, but this could be extracted into a reusable workflow (workflow_call) to avoid future drift. At minimum, the health-check loop is 12 lines duplicated three times — a divergence risk as the Postgres setup evolves.

Action pinning inconsistency between ci.yml and release.yml

release.yml correctly uses SHA-pinned actions for all security-critical steps (checkout, artifact upload/download, PyPI publish). ci.yml and postgres-pgsearch-smoke.yml use @v4 floating major-version pins. The inconsistency is defensible — CI has lower attack surface than the release publishing pipeline — but it's worth noting the difference is intentional policy and not a gap.

---

Security — Findings

FastAPI lock is handled correctly

Locking to fastapi==0.136.1 after pip-audit flagged MAL-2026-4750 in 0.136.3 is the right call. Locking to a known-safe version is preferable to adding the advisory to the ignore list.

Ignored CVEs need owner and review date

The two pip-audit ignores in ci.yml (CVE-2025-69872 for diskcache, CVE-2026-4539 for pygments) have justification comments noting "no fix available," which is good. Consider adding a review date comment (e.g., # revisit after diskcache 5.7+) so the next person to read this knows it's a tracked decision and not an oversight.

docker-compose.test.yml default password is gobby_test

The default GOBBY_POSTGRES_TEST_PASSWORD is the same as the user/db name. This is test-only and the container uses tmpfs so there's no persistence risk, but it's worth documenting that this value must never be reused outside the test harness.

---

Test Coverage

High skip rate warrants a comment

The PR body reports 4,869 skipped tests (~25% of the suite). If many of those skips are Postgres-requiring tests that now only run in CI with the container, that's expected and fine — but it should be documented in the PR description so reviewers know it's not a regression. Adding a note like "4,869 skips expected: X are Postgres-container-gated, Y are voice-extra" makes the numbers interpretable at a glance.

CHANGELOG jumps from 0.4.6 to 0.4.9

There are no 0.4.7 or 0.4.8 entries. If these were patch releases that shipped without changelog entries, or internal milestones that were never tagged, a brief note in the CHANGELOG header (or a placeholder section with "no public release") would prevent future confusion about the version history.

---

Release Process — Looks Good

• The gate_only default (true) on manual workflow_dispatch is a safe pattern that prevents accidental publishes.
• The verify-ci job requiring a successful CI run for the release SHA before building/publishing is solid release hygiene.
• The CHANGELOG is specific, well-scoped, and links task IDs.
• Pre-push validation evidence (exit 0, test counts, pip-audit clean) is included in the PR body — good practice.

---

Summary

The release is well-prepared. The primary actionable items are:

1. Add docker rm -f guard to the test job (parity with build job and smoke workflow).
2. Add branch filter to postgres-pgsearch-smoke.yml to avoid unnecessary runs on unrelated branches.
3. Clarify the 25% skip rate in the PR description.
4. Add review dates to pip-audit CVE ignore comments.

None of these are blockers, but items 1 and 2 are worth addressing before the next release cycle adds self-hosted runner capacity.

🤖 Generated with Claude Code

[claude]

Code Review: Release v0.4.9

This is a large release PR (1,592 files, +117K/-45K lines, 100 commits). The review focuses on correctness, security, and maintainability rather than exhaustive line-by-line coverage.

---

Overview

0.4.9 completes the PostgreSQL-only runtime cutover, migrates the knowledge graph backend from Neo4j to FalkorDB, adds the memory-recall-helper backgrounded agent, introduces gobby build coordinator gating, and adds Grok CLI adapter support. Scope is large but the changes are logically grouped and the CHANGELOG is detailed.

---

Positives

• FastAPI security response is correct. !=0.136.3 in pyproject.toml precisely excludes the malicious version (MAL-2026-4750) while allowing forward-compatible upgrades. No over-restriction.
• pg_search query sanitization is safe. sanitize_pg_search_query in search/keyword.py:316 uses an allowlist (alphanumeric + space/underscore/hyphen), not a denylist. Queries never reach the DB unsanitized.
• Table names are validated via allowlist. BM25SearchBackend only accepts table names present in _TABLE_CONFIGS; user input never flows into the table-name position of SQL strings.
• _SQL_IDENTIFIER_PATTERN guard exists in hub/postgres.py. Identifier injection is covered at the PostgreSQL layer.
• GrokAdapter is lean and idiomatic (34 lines, follows the existing adapter pattern exactly). The TOOL_MAP covers the expected tool aliases well.
• BuildOptions is well-structured. New fields (planning_seed_state, completed_plan_review_rounds, dry_run, coordinator_session_ref) are clean dataclass fields with sensible defaults.
• 1,000-line file limit is respected. The largest non-test source file (servers/routes/configuration.py) sits at 998 lines. The refactors called out in the CHANGELOG (app config leaf models, task CRUD helpers, workflow observers) have kept the limit intact.
• FalkorDB migration SQL (262) is safe. Uses fixed key matching via WHERE key IN (...), no interpolation from user input.
• No type: ignore suppressions introduced in hot paths across the reviewed modules.

---

Issues and Suggestions

1. CI checks still pending (blocking)

The following checks have no recorded conclusion yet:

• Release Branch CI
• Test (Python 3.13)
• Test (Python 3.14)
• Test (Frontend)
• Build Package

The PR body reports a passing local run (14476 passed) but those results are not yet reflected in GitHub CI. The Release Branch CI check in particular is the primary release gate. Recommend holding merge until all checks record SUCCESS.

2. Silent exception swallowing in async dispatch

build/dispatch_tick.py:108 and :131:

except Exception:
    return None  # or logger warning only in the outer wrapper

The inner _project_id_for_task logs at warning level with exc_info=True, which is good. But _run_scheduled_dispatcher_tick wraps kick_dispatcher_tick in a broad catch that logs at warning only and otherwise drops the failure silently for fire-and-forget tasks. If the dispatcher tick consistently fails, there is no observable signal beyond periodic log scanning. Consider incrementing a metric counter or emitting a structured event to make repeated failures discoverable without log grep.

3. BM25SearchBackend.get_stats() silently returns 0 on error

search/keyword.py:176:

except Exception:
    count = 0

No log at any level. A schema migration error or missing pg_search extension would silently surface as document_count: 0 in stats, which is indistinguishable from a legitimately empty index. Add at least a logger.debug(...) with exc_info=True.

4. build/observability.py bare-except with return None

The except Exception: return None path (line 159) produces no log output. Observability helpers that fail silently are self-defeating — this is where you most want a trace.

5. Migration 264 is irreversible

DROP TABLE IF EXISTS gobby_migration_state;

Dropping a bookkeeping table is correct once you are fully PostgreSQL-only, but there is no down-migration path. This is fine if the project has an explicit policy of forward-only migrations; worth documenting in the migration file header if it does not already exist.

6. FastAPI constraint is temporary — add a tracking reference

pyproject.toml:25:

"fastapi>=0.136.0,!=0.136.3",

This is correct for now, but the !=0.136.3 exclusion will silently stop being meaningful once FastAPI publishes 0.136.4+. There is no comment pointing to a task or issue to revisit the constraint once a safe newer release ships. A brief inline comment citing MAL-2026-4750 and a linked task ref would prevent this from becoming stale tech debt.

7. _dispatcher_services_for_db uses unchecked getattr chains

build/dispatch_tick.py:95-99:

service_db = getattr(services, "database", None)
service_task_manager = getattr(services, "task_manager", None)
service_task_db = getattr(service_task_manager, "db", None)
if service_db is not db and service_task_db is not db:
    return None

This is functional but opaque — the duck-typed service contract is implicit. A typed AppContext protocol (or assert on the type) would make the dependency surface explicit and catch mismatches at type-check time rather than silently returning None at runtime.

8. Integration merge commit density

The commit graph contains dense chains of merge commits from gobby/integration/12746-migrate-knowledge-graph-backend. These are fine operationally, but they make git bisect and blame unusable for the knowledge-graph work in this release window. No action required for this PR, but consider squashing integration branches before merging future large feature tracks.

---

Test Coverage

The PR description reports 14476 passed / 4869 skipped / 3 xfailed. Given the PostgreSQL-only cutover, the FalkorDB wizard harness additions (test: add live FalkorDB wizard harness), and the new gcode PostgreSQL DSN broker path, coverage of the new paths appears intentional. The pending CI check is the blocker, not coverage gaps per the CHANGELOG evidence.

---

Summary

The code quality is generally high. No SQL injection vectors, no credential leaks, no obvious logic errors in the reviewed modules. The three items that should be addressed before merge are: (1) confirm all CI checks pass, (2) add a debug log to BM25SearchBackend.get_stats() and observability.py exception paths, and (3) annotate the FastAPI constraint with a tracking reference. Items 4–8 are lower-priority suggestions.

[claude]

Code Review: Release v0.4.9

This is a large release PR (1,592 files, +117K/-46K lines) covering PostgreSQL-only runtime hardening, FalkorDB knowledge-graph backend, Grok CLI support, memory recall helper, build/verification gates, and release pipeline improvements. I reviewed the key change surfaces below.

---

Overview

The release is well-structured with a thorough CHANGELOG, solid validation evidence (14,476 passed / 4,869 skipped / 3 xfailed), and a clean security response to the FastAPI vulnerability. The architecture continues to improve through module decomposition and facade patterns.

---

Security

Positive:

• fastapi>=0.136.0,!=0.136.3 in pyproject.toml correctly excludes the MAL-2026-4750 malicious release while staying on the upgrade path. Clean approach.
• fastmcp>=3.2.0 bump addresses CVE-2025-64340 and CVE-2026-27124.
• release.yml now uses full SHA digest pins on all actions (@34e114876..., etc.) — harder to supply-chain-attack than floating tags.
• Bandit + pip-audit gate in CI; the two ignored CVEs (diskcache transitive via litellm, pygments 2.19.2 latest) are documented with justification.

Concern — CI vs. release action pin inconsistency:
ci.yml uses major-version pins (@v4) while release.yml uses SHA digests. The CI comment ("major-version pinned so Dependabot can carry security updates") explains the trade-off, but this means supply-chain exposure is asymmetric: a compromised actions/checkout@v4 would affect CI but not the release gate. Worth tracking whether to tighten CI pins or document the deliberate policy gap in a security note.

---

Code Quality

src/gobby/storage/hub/postgres.py — placeholder scan cache

The _PLACEHOLDER_SCAN_CACHE = threading.local() accumulates a dict[tuple[str, int], tuple[int, ...]] per thread with no eviction policy. In a long-running daemon processing many distinct SQL queries this will grow unbounded per thread. Consider adding a max-size cap (e.g., 512 entries via collections.OrderedDict + eviction on insert) or switching to a bounded LRU structure.

src/gobby/config/validation_detection.py — wrapper normalization depth

_MAX_WRAPPER_NORMALIZATION_DEPTH = 8 is a hardcoded constant with no path to configure it. A real-world command like timeout 30 env TERM=xterm nice -n 10 uv run --project . pytest already uses 5 wrappers. The limit seems safe for now but the silent truncation (function returns a partial result rather than raising) could produce a confusing false-negative miss. At minimum, a logger.debug log when the depth limit is hit would aid debugging.

src/gobby/config/validation_detection.py — _starts_with_command_prefix

def _starts_with_command_prefix(tokens, prefix):
    return (
        _matches_command_token(tokens[0], prefix[0])
        and tokens[1:len(prefix)] == prefix[1:]
    )

_matches_command_token applies basename matching (Path(token).name == expected) only for tokens[0]. Tokens 1+ use exact equality. This is intentional but means /usr/bin/env matches env while /usr/bin/python as token[1] would NOT match python as prefix[1]. This could silently fail for fully-qualified secondary command tokens. Worth a short comment on this invariant.

src/gobby/config/sessions.py — MemoryRecallHelperConfig

The config currently only has an enabled bool. Given this spawns a background Haiku agent on every turn start, missing fields for timeout_seconds, max_concurrent, or cooldown_seconds could lead to runaway spawning under pathological session patterns. Even conservative defaults (e.g., timeout_seconds=30) would make the feature's behavior contract explicit.

src/gobby/build/service.py — now a facade

Clean pattern. Re-exporting from focused modules is the right approach. The __all__ list is complete and avoids accidental namespace pollution.

src/gobby/agents/run_completion.py — completion registry error isolation

Good: completion registry notify failures are caught and logged at debug level rather than bubbling up and preventing the run from being marked complete. However, if runner.complete_run itself raises an exception (not just returns False), the caller is responsible for catching it. This is fine as the pattern is consistent elsewhere, but a docstring note on the caller's responsibility would help.

---

Architecture / File Size

Two files are approaching the 1,000-line limit defined in CLAUDE.md:

• src/gobby/servers/chat_session.py — 973 lines
• src/gobby/workflows/engine/core.py — 971 lines

Per guiding principle #2, a refactor task should exist for each if it doesn't already. These are not blocking, but they'll hit the wall with any further additions.

---

CI / Release Workflow

Positive:

• Python 3.13 + 3.14 matrix in test job — good forward compatibility signal.
• 80% coverage floor enforced via --cov-fail-under=80.
• verify-ci job in release.yml polls for a successful CI run before allowing publish — prevents racing a green tag against a still-running CI run.
• Postgres test container uses --tmpfs /var/lib/postgresql for speed and isolation.
• build job in CI independently verifies the wheel contains gobby/ui/web/dist/index.html and runs a UI smoke test against the same Postgres container — solid packaging gate.

Minor:

• The security job's pip-audit step has no needs: dependency and could in theory run against a stale lockfile if uv sync in a parallel job is slow. In practice this is fine since each job starts fresh, but worth noting.
• ci.yml still lists "0.4.9" in branches: for push triggers. This is correct for the release branch lifecycle but should be removed post-merge to avoid CI running on a stale reference.

---

Test Coverage

The reported numbers (14,476 passed, 4,869 skipped, 3 xfailed) and the 80% coverage gate provide a reasonable baseline. The new validation_detection.py module is substantial (~600 lines of logic) — verifying it has dedicated unit tests for the wrapper normalization depth boundary and the _starts_with_command_prefix basename matching edge case would be worth confirming.

---

Summary

This is a well-validated release. The main actionable items are:

1. Add eviction to _PLACEHOLDER_SCAN_CACHE to prevent unbounded memory growth in long-running daemon threads.
2. Add a debug log when _MAX_WRAPPER_NORMALIZATION_DEPTH is hit in validation_detection.py.
3. Add rate-limiting fields to MemoryRecallHelperConfig before the helper sees heavy production use.
4. Track refactor tasks for chat_session.py and workflows/engine/core.py before they breach 1,000 lines.
5. Remove "0.4.9" from CI branch triggers post-merge.

None of these are release blockers given the extensive test validation already in place. The security response, CI hardening, and architectural decomposition work are all solid.