Phase 3 — Data-integrity fixes (rides 0.19.0)#317
Merged
Conversation
JournalService.delete_entry deleted the DB row but never de-indexed the RAG chunks, so deleted/private journal content bled forever into RAG answers and claim-time briefings. Add OptimalService.unindex_journal_entry mirroring unindex_playbook (vector-store delete_by_source + tracking-row delete via get_db_context, both idempotent + best-effort), and call it from delete_entry after the row commit inside a try/except so a de-index failure never errors the delete.
The memory distiller emits lessons with a fixed 'Problem: …' opening
shape, so two distinct lessons whose first 100 chars match collided on
learning_id = f"lrn-{md5(content[:100])[:12]}". replace_on_reingest then
routed both to the same source URI and the second ingest's replace_chunks
DELETE wiped the first lesson's chunks — silent data loss.
Hash the full content (widening the hex slice 12→16) so distinct bodies
get distinct ids and each retains its chunks.
… time mark_read and mark_all_read used to zero the unread counter FIRST, then run a bulk UPDATE … WHERE read_at IS NULL that stamped every inbound unread row. A send_chat_message committing between the counter-zero and the UPDATE inserted a new read_at NULL row that the UPDATE then stamped as read — the new message was silently consumed while the counter stayed 0. Mirrors get_unread_messages (same file): SELECT the unread message IDs at call time, UPDATE exactly those IDs, then recompute the unread counter from the DB via the existing _reset_unread_counter helper. A message arriving mid-call is not in the selected ID set, so the UPDATE skips it and the recomputed counter keeps it unread.
…empty/error/disabled
Dead-letter replay mirrors the original journal._schedule_rag_index path: a private entry is never indexed into the shared JOURNALS corpus, and a private learning is still recorded into LEARNINGS as non-shareable. Previously the replay always called index_journal_entry and skipped record_learning for private learnings, leaking private content on replay and dropping the legitimate non-shared learning. Three regression tests.
github-actions
Bot
added
documentation
|
Thanks for opening your first pull request on RoboCo! Quick checklist before review (most of these are enforced by CI, but worth a glance):
See CONTRIBUTING.md for the full workflow and the Code of Conduct for the community standards we follow. Welcome aboard — a maintainer will review shortly. |
Sibling site to the clone fix: the conventions read-clone refresh ran 'git fetch --tags <https://TOKEN@host> <branch>', exposing the PAT in the fetch argv on the orchestrator host. Mirrors the clone site's per-call '-c http.extraheader=Authorization: Basic …' prefix + bare URL. SSH URLs and tokenless public repos unchanged.
…,L27,L14,L28,H19,M13,M14,H20,M16,M45,L25,M15,L24)
…001), hoist AuditLogTable import (PLC0415)
…the GrokCliProvider
…reflects grok agents
…ep so live cost reflects Anthropic cache spend
…ud 429 parks instead of crash-respawning
…ens roll into usage/cost summaries
…r's revisit counter survives a restart
…rates after 2026-08-31
…ilently falling back to a zero-usage env id
…ken-sweep test signatures)
…ck stays reachable
… tokens are bounded
…ry so a stolen cookie's exp is fixed
…t revokes the current jti
…w + jti revocation
[phase9] Auth hardening: bound agent tokens (iat/exp) + JWT jti revocation
[scan-fix] Phase 8: LLM providers / usage / billing
[Phase 7] schemas/conventions/MegaTask/API fixes (15 findings, scan-fix program)
Phase 6 — Panel frontend hardening (rides 0.19.0)
Phase 5 — Background-engine hardening (rides 0.19.0)
Phase 4 — Git / workflow hardening (rides 0.19.0)
github-actions
Bot
added
dependencies
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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.
Phase 3 — Data-integrity fixes (rides 0.19.0)
Third phase of the 2026-07-06 full-codebase-scan fix program. Ten verified-real data-integrity / RAG / notification findings, plus an e2e smoke module, all under the existing
## [0.19.0]CHANGELOG section. Stacks on Phase 2 (fix/scan-state-machine-gateway, PR #316) — base isfix/scan-state-machine-gateway; merge #316 first.Fixes
3985b009) —delete_entryunindexes the journal entry from the JOURNALS RAG corpus. Previously a deleted journal entry stayed searchable forever (the index row was never removed). AddsOptimalService.unindex_journal_entry(best-effort, mirrors the existingunindex_playbookpattern) and calls it fromJournalService.delete_entry.98ce3cf0) —learning_idhashes the full content (not justentry_id), so two distinct learnings that share an entry no longer collide and silently overwrite one another in the LEARNINGS index.ec2cb031) — config load rejects a non-internallocal_llm_base_url. Closes the fire-and-forget RAG indexing hot path calling out to an arbitrary external host; the validator runs once atSettingsload, dev mode unchanged.8ec382eb) — knowledge-share learning broadcasts bulk-insert the per-agent notifications instead of an N+1 loop. Closes the O(agents) insert storm on every learning broadcast.a8855c36) —mark_read/mark_all_readstamp only the rows that were unread at call time (a snapshotidsset, not a re-query). Closes the read-race where a concurrent new notification landed between the unread-query and the update and got retro-acknowledged.9ba49cfe) — notification dedup uses an exactto_agentspredicate + a purpose discriminator + anack DEL-aware comparison. Previously real notifications were dropped three ways: a broadcast shadowing a targeted one, a same-type-different-purpose pair, and an ack-vs-delete state mismatch.7e866e8f) —playbooks.indexed_ok/indexed_atcolumns (migration 064) + a startup reconcile that re-indexes approved-but-unindexed playbooks. Closes the drift where an approved playbook's index write failed and the playbook stayed invisible to retrieval forever.unindex_playbookflipsindexed_okon archive.809caf3b) — RAG indexing failures persist to a dead-letter (rag_index_failures, migration 065) instead of being swallowed; a janitor (reclaim_due) replays them with bounded backoff, and/healthreportsfailed_index_count. Best-effort throughout — a failure never blocks the caller's commit.12da8ec9) — the dead-letter replay (_reindex_journal_entry) mirrors the originaljournal._schedule_rag_indexpath: a private entry is never indexed into the shared JOURNALS corpus, and a private learning is still recorded into LEARNINGS as non-shareable. The original replay always calledindex_journal_entryand skippedrecord_learningfor private learnings — leaking private content on replay and dropping the legitimate non-shared learning. Three regression tests cover the private-learning / shared-learning / private-general matrix.7ae4c8c7) — the institutional-memory briefing exposes a status sentinel distinguishing below-floor / empty / error / disabled, so a caller can tell "nothing relevant" apart from "RAG is off / broken" instead of both reading as an empty list.36a017cc) —sweep_expired_notificationsre-escalates a stale unacked ack-required notification (re-notify + bump) instead of silently dropping it, so a missed formal notification doesn't vanish.c6d931d6) —tests/e2e_smoke/test_data_integrity.py, one scenario per cross-layer finding (C3 / H12 / M27 / M23 / M24 / L26), scripted-agent over the real API + ephemeral Postgres + fake GitHub.Verification
uv run ruff format+ruff check— clean.uv run mypy roboco/— clean.uv run pytest tests/unit/services/test_health.py— 11 passed (8 existing + 3 new C1 regression tests).make e2e-smoke— every Phase 3 scenario passes in isolation. The full-suite run remains order-dependent (session-scopede2e_stackshares thebe-dev-1workspace across scenarios in one file) — pre-existing harness limitation, documented out of scope, not a product regression._reindex_journal_entryis_privateleak) is fixed in12da8ec9; two Minor (a misleading L26 docstring, M24 replay not re-notifying) are acceptable with comments. Three implementer-flagged concerns (M28 Redis guard drop, H12_DbHolderrebind, L23similar_memoryreturn-type change) all adjudicated ACCEPTABLE by opus.Known pre-existing issues (not introduced here)
be-dev-1workspace across scenarios in one file — a per-scenario fixture reset would fix it. Harness change, not a service-layer fix.Model note
Implementer ran on sonnet; whole-branch review on opus. kimi-k2.7-code:cloud is not routable in this harness.
🤖 Generated with Claude Code