feat(us3): observations split — credentials + memories + behavioral_rules (PR-A, US3)#181
Merged
Conversation
…it A, PR-A) Create dedicated credentials table as additive schema for US3 observations split. Migration ID note: spec tasks.md says 077_credentials, but US1+US2 consumed 083-086 (083 drop orphan joins, 084 Gemini-fix Rollback, 085 drop content_chunks, 086 drop used_vector). US3 migrations therefore shift to 087 onwards (next-free). This commit creates 087; subsequent commits in PR-A will create 088 (memories), 089 (behavioral_rules), 090 (data migration 3-way split). Changes: - internal/db/gorm/migrations.go: add migration 087_credentials CREATE TABLE credentials per spec.md §Data Model (id BIGSERIAL PK, project, key, encrypted_secret BYTEA, encryption_key_fingerprint, scope, version, edited_by, created_at, updated_at, deleted_at, UNIQUE(project, key)). Partial indexes on project and encryption_key_fingerprint WHERE deleted_at IS NULL. Rollback drops indexes + table. - pkg/models/credential.go (new): domain Credential struct, no GORM tags (mirrors pkg/models/observation.go pattern — infra concerns stay in internal/db/gorm). - internal/db/gorm/models.go: add GORM Credential row type with tags matching all columns + TableName() returning "credentials". - internal/db/gorm/credential_store.go (new): CredentialStore with Create, Get, List, Delete (soft-delete via UPDATE deleted_at), CountCredentials, CountWithDifferentFingerprint, DeleteOrphanedByFingerprint. Signatures mirror ObservationStore vault helpers so callers in handlers_vault.go can swap receiver types without touching call sites. Conversion via credentialRowToModel() keeps pkg/models isolated from gorm internals. - internal/db/gorm/credential_store_test.go (new): integration smoke tests skip unless DATABASE_DSN is set (matches migrations_integration_test.go pattern). Exercises Create→Get→List→Count→Delete round-trip against a real pgvector/pgvector:pg17 instance. Anti-stub: replacing any method body with `return nil` causes the round-trip test to fail. Protected invariants (NONE violated by this commit — additive-only): - observations table UNTOUCHED (still has 2775 live rows / 13 credentials) - content table UNTOUCHED - vault ciphertext bytes unchanged - fingerprint aa78e55cf896508c / credential_count 13 / mismatch_count 0 (verified pre-flight 2026-04-17, nothing in this commit touches prod data) Verification: - go build ./... — green - go vet ./internal/db/gorm/... — green - go test ./internal/db/gorm/... -run Credential — 3/3 PASS against pgvector/pgvector:pg17 with full migration chain 001→087 applied fresh - TestMigrationsIntegration failure is pre-existing on main 83bbc62 (stale vector-dim assertion, legacy US2 leftover) — not introduced here Task: T017 Commit A Spec: .agent/specs/engram-v5-baseline/spec.md §Data Model §credentials Plan: .agent/specs/engram-v5-baseline/plan.md §Phase 5 Phase 5 gate: F-1 decrypt round-trip (T022) — not yet
…PR-A)
CREATE TABLE only — no Go store code, no data migration, no handlers.
Commit C will add memory_store + behavioral_rules_store + gRPC dispatch.
Commit D will migrate data from observations.
Migration ID shift: spec.md says 078/079; Commit A took 087; next-free IDs
are 088 (memories) + 089 (behavioral_rules).
Schemas match spec.md §Data Model verbatim (column-for-column verified):
MEMORIES (internal/db/gorm/migrations.go migration 088):
- id BIGSERIAL PK, project TEXT NOT NULL, content TEXT NOT NULL,
tags JSONB DEFAULT '[]', source_agent TEXT, version INT DEFAULT 1,
edited_by TEXT, created_at/updated_at TIMESTAMPTZ DEFAULT NOW(),
deleted_at TIMESTAMPTZ, search_vector tsvector GENERATED ALWAYS AS
STORED (dual-dict english + simple per migration 076 pattern).
- Indexes: idx_memories_project_created partial WHERE deleted_at IS NULL,
idx_memories_fts GIN, idx_memories_tags GIN.
- Rollback drops indexes (reverse order) then table.
BEHAVIORAL_RULES (migration 089):
- id BIGSERIAL PK, project TEXT (NULLable — NULL = global rule),
content TEXT NOT NULL, priority INT DEFAULT 0, version INT DEFAULT 1,
edited_by TEXT, created_at/updated_at TIMESTAMPTZ DEFAULT NOW(),
deleted_at TIMESTAMPTZ.
- Indexes: idx_behavioral_rules_project_priority partial WHERE
deleted_at IS NULL, idx_behavioral_rules_global partial WHERE
project IS NULL AND deleted_at IS NULL.
- Rollback drops indexes (reverse order) then table.
NOT carried over from observations: importance_score, relevance_score,
effectiveness_*, cited, inject_count — per spec S1 (scoring dropped).
No FTS on behavioral_rules — rules are injected unconditionally at
session-start, not searched.
Protected invariants (NONE violated — additive only):
- observations table UNTOUCHED (still 2775 live rows / 13 credentials)
- credentials table from Commit A UNTOUCHED
- content table UNTOUCHED
- vault ciphertext bytes unchanged
- fingerprint aa78e55cf896508c / credential_count 13 / mismatch_count 0
Verification:
- go build ./... — green
- Migration chain 001→089 "all migrations passed" against
pgvector/pgvector:pg17 fresh DB
- TestCredentialStore_* 3/3 PASS + rotation sub-test + 5 validation
sub-tests (regression for Commit A — unchanged)
- psql SELECT tablename FROM pg_tables WHERE tablename IN
('credentials','memories','behavioral_rules') — 3 rows
- 11 indexes created (pkey + idx_* across 3 tables)
- TestMigrationsIntegration vector-dim failure is pre-existing on
main 83bbc62 (US2 leftover, not introduced here)
Task: T018 Commit B
Spec: .agent/specs/engram-v5-baseline/spec.md §Data Model §memories + §behavioral_rules
…les MCP tools (Commit C, PR-A)
Implement CRUD stores for the new static-entity tables (migrations 088_memories,
089_behavioral_rules from Commit B) and wire two new MCP tools (store_rule,
list_rules) that read/write through them.
Scope boundary respected (per tasks.md T019):
- handleStoreMemory / handleRecall STILL route to observations (Commit E switches them)
- handlers_vault.go + credential_store.go UNTOUCHED (Commit E scope)
- No data migrations touched (Commit D = T020)
- No observations dropped (Commit G = T023)
FILES CREATED (6 new):
- pkg/models/memory.go: domain Memory struct (no GORM tags, Tags []string real slice,
search_vector intentionally absent from domain — it's a GENERATED column).
- pkg/models/behavioral_rule.go: domain BehavioralRule struct (Project *string for
NULLable global-rule semantics; *time.Time pointers for all timestamps).
- internal/db/gorm/memory_store.go: MemoryStore with Create/Get/List/Update/Delete.
Create returns a new *models.Memory (does not mutate input — per Commit A
code-review HIGH-1 immutability rule). Delete is HARD (Unscoped) per Commit A
code-review HIGH-2 rotation analysis — deleted_at column remains in schema
for future use but is not populated. Update uses gorm.Expr("version + 1") +
re-fetch. Empty-arg guards on Get/List.
- internal/db/gorm/memory_store_test.go: 3 integration tests skip without
DATABASE_DSN. Covers full round-trip (Create→Get→Update→List→Delete),
validation errors (nil/empty/empty), and per-project filtering (2 projects,
3 memories, asserts isolation).
- internal/db/gorm/behavioral_rules_store.go: BehavioralRulesStore with same CRUD
shape + global-rule semantics: List(nil, limit) returns only rows where
project IS NULL; List(&proj, limit) returns rows WHERE project = proj OR
project IS NULL (global rules always apply).
- internal/db/gorm/behavioral_rules_store_test.go: 3 integration tests.
TestBehavioralRulesStore_List_GlobalRulesAlwaysIncluded validates the
union semantics: 1 global + 1 project-scoped row; List(&"p1") returns both,
List(nil) returns only the global.
- internal/mcp/tools_rules.go: handleStoreRule + handleListRules. Both
validate inputs, wrap errors with %w, return JSON responses with RFC3339
timestamps. handleStoreRule requires content, defaults priority to 0,
project optional (nil = global). handleListRules bounds limit 1-500
(default 50), project optional.
FILES MODIFIED (2):
- internal/db/gorm/models.go: +38 lines appending GORM row structs for Memory
(Tags via models.JSONStringArray matching existing JSONB convention; deleted_at
*time.Time; TableName returns "memories") and BehavioralRule (Project *string
NULLable; TableName returns "behavioral_rules").
- internal/mcp/server.go: +47 lines.
* Server struct: 2 new fields memoryStore + behavioralRulesStore after issueStore.
* 2 new setters SetMemoryStore + SetBehavioralRulesStore (mirrors SetIssueStore
pattern — called by worker bootstrap after async init).
* handleToolsList: 2 new Tool entries (store_rule, list_rules) at tierUseful.
* callTool switch: 2 new cases routing to tools_rules.go handlers (after
merge_observations, before get_observation).
- internal/worker/service.go: +9 lines.
* Construct memoryStore and behavioralRulesStore via gorm.New*Store(store)
alongside reasoningStore.
* Wire both into mcpServer via the new setters, alongside SetReasoningStore
+ SetIssueStore (consistent bootstrap order).
Protected invariants (NONE violated — additive only, no observation writes):
- observations table UNTOUCHED (still 2775 live rows / 13 credentials)
- credentials table UNTOUCHED
- memories + behavioral_rules tables gain functional CRUD (no production data yet)
- content table UNTOUCHED
- vault ciphertext bytes unchanged
- fingerprint aa78e55cf896508c / credential_count 13 / mismatch_count 0
Verification:
- go build ./... — green
- go vet ./internal/db/gorm/... ./internal/mcp/... — clean
- go test ./internal/db/gorm/... -run "Memory|BehavioralRules|Credential" -v
against pgvector/pgvector:pg17 fresh DB:
PASS TestBehavioralRulesStore_CreateGetUpdateListDelete (65s — includes migration chain)
PASS TestBehavioralRulesStore_Create_ValidationErrors (2 sub-tests)
PASS TestBehavioralRulesStore_List_GlobalRulesAlwaysIncluded
PASS TestCredentialStore_CreateGetCountDelete (regression preserved)
PASS TestCredentialStore_DeleteOrphanedByFingerprint (regression preserved)
PASS TestCredentialStore_Create_ValidationErrors (5 sub-tests — regression)
PASS TestMemoryStore_CreateGetUpdateListDelete
PASS TestMemoryStore_Create_ValidationErrors (3 sub-tests)
PASS TestMemoryStore_List_FiltersByProject
TOTAL: 9 top-level + 10 sub-tests, all green, ~78s incl. migration chain.
Task: T019 Commit C
Spec: .agent/specs/engram-v5-baseline/spec.md §Data Model + §FR-4
…R-A)
3-way data split: observations → credentials + behavioral_rules + memories.
Ciphertext preserved byte-for-byte; DO-block enforces 2 invariants.
Migration ID shift: plan.md originally drafted this as 080_...; actual next-free
is 090 (US1+US2 consumed 083-086; Commit A=087, Commit B=088+089).
Plan.md column mapping amended 2026-04-18 (see plan.md §Amendment 2026-04-18) —
the original draft assumed columns that do not exist on observations:
- content (creds) → title (per ObservationStore.GetCredential)
- content (mem/rul) → COALESCE(NULLIF(TRIM(narrative),''), title, '')
- always_inject col → concepts @> ["always-inject"]::jsonb
- creation_path fil → dropped (migrate ALL live non-credential non-always-inject)
- tags → concepts (JSONStringArray stored jsonb)
- source_agent → agent_source
- updated_at → reuse created_at (observations has no updated_at)
- timestamp cast → TO_TIMESTAMP(created_at_epoch) — unambiguous from int64
- priority (rules) → derived from importance_score (≥0.8→10, ≥0.5→5, else 0)
- live filter → is_suppressed=false AND is_archived=0 AND is_superseded=0
Three INSERTs in order:
1. credentials — type='credential' rows with encrypted_secret IS NOT NULL AND
encryption_key_fingerprint IS NOT NULL AND title IS NOT NULL AND live.
Ciphertext + fingerprint bytes preserved verbatim (no re-encryption).
2. behavioral_rules — observations where concepts @> '["always-inject"]'::jsonb
AND type != 'credential' AND live AND has content. priority from importance.
3. memories — remaining live non-credential non-always-inject observations with
non-empty content. tags = concepts verbatim. project NULL → '' fallback.
Sanity check DO block (BOTH invariants active tripwires):
- (a) dst_count < src_count/2 → RAISE EXCEPTION
- (b) credentials.count != observations WHERE type='credential' live count
→ RAISE EXCEPTION — every vault credential MUST migrate byte-for-byte
Rollback: TRUNCATE the 3 static tables in reverse order (memories → behavioral_rules
→ credentials). Forward migration only READS observations, so rollback == undo
the data copy. Schema remains (migrations 087-089 Rollback handles schema).
Protected invariants (NONE violated by this commit — observations UNTOUCHED):
- observations table unchanged (2775 live / 13 credentials in production)
- credentials/memories/behavioral_rules tables GAIN data; no lossy transform
- vault ciphertext bytes preserved verbatim
- production fingerprint aa78e55cf896508c / count 13 / mismatch 0
Verification:
- go build ./... — green
- go vet ./internal/db/gorm/... — clean
- Migration chain 001→090 "all migrations passed" on empty pgvector/pgvector:pg17
- Credential/Memory/BehavioralRules tests 9 top-level + 10 sub-tests — all green
- Synthetic-data test: seeded 6 observations (5 live + 1 archived), ran migration
logic via psql; got credentials=2 memories=2 behavioral_rules=1 (archived row
correctly excluded); DO block NOTICE fired with correct counts
- Invariant (b) tripwire confirmed: DELETE 1 credential row → RAISE EXCEPTION
"credential invariant FAIL cred=1 cred_live=2"
- Invariant (a) tripwire confirmed: TRUNCATE all static tables → RAISE EXCEPTION
"sanity FAIL dst=0 src=5" (fires first since order is a → b)
- TestMigrationsIntegration vector-dim failure remains pre-existing (US2 leftover,
not introduced here)
Next: Commit E (T021) switches handlers (handleStoreMemory/handleRecall) to use
the new stores; current commit leaves handlers routed to observations still.
After Commit E comes Commit F-1 (HARD GATE decrypt round-trip test) — BLOCKER
for PR-B drop migrations (commits G+H) per spec §FR-9 + §NFR-7.
Task: T020 Commit D
Spec: .agent/specs/engram-v5-baseline/spec.md §FR-4 + plan.md §Phase 5 (Amendment 2026-04-18)
…es routes (Commit E, PR-A) Route vault HTTP handlers through the new credentialStore, expose a new set of /api/memories HTTP endpoints backed by memoryStore, and wire all three static- entity stores (credentialStore + memoryStore + behavioralRulesStore) as fields on the worker Service struct. Dual-read preserved: this commit DOES NOT touch handleStoreMemory / handleRecall inside internal/mcp - those still route to observationStore. The actual MCP handler switch happens in Commit G (post Commit F-1 decrypt round-trip gate). Files changed: - internal/worker/service.go: 3 new fields on Service struct (credentialStore, memoryStore, behavioralRulesStore); initializeAsync constructs credentialStore alongside existing memoryStore + behavioralRulesStore (now promoted from locals to fields); initMu block wires all 3 pointers; setupRoutes registers POST/GET/DELETE /api/memories routes inside the authenticated router group. - internal/worker/handlers_vault.go: handleVaultStatus switched to credentialStore.CountCredentials + .CountWithDifferentFingerprint; handleDeleteOrphanedCredentials switched to credentialStore.DeleteOrphanedByFingerprint. Signatures adapted to new method names on CredentialStore. - internal/worker/handlers_memories.go (NEW): thin HTTP wrappers over memoryStore. handleStoreMemoryExplicit validates project+content non-empty, returns 201 on success, 503 if store not initialised. handleListMemories requires project query param, defaults limit 50, returns [] not null. handleDeleteMemoryByID parses chi URL param, maps gorm.ErrRecordNotFound to 404. - internal/worker/handlers_memories_test.go (NEW): 4 integration tests (skip without DATABASE_DSN) - RoundTrip (POST+GET), ValidationErrors (2 sub-tests for empty project + empty content), DeleteByID RoundTrip (POST+DELETE+GET returns empty), DeleteByID NotFound (404 on missing id). - .gitignore: add .tmp_gocache/ and .tmp_gomodcache/ underscore variants (GoCache and GoModCache directories use either hyphen or underscore depending on tool version). Scope boundary respected: - handleStoreMemory / handleRecall in internal/mcp UNTOUCHED (dual-read preserved) - observationStore.CountCredentials / DeleteOrphanedCredentials methods still exist - only CALLERS migrated. Commit G removes the methods when observations drops. - No migration changes (087-090 frozen) - No observations data writes from this commit Protected invariants (NONE violated - additive + caller redirection only): - observations table UNTOUCHED (dual-read still works via MCP store/recall) - credentials / memories / behavioral_rules schemas UNTOUCHED - Vault ciphertext bytes unchanged - Production fingerprint aa78e55cf896508c / count 13 / mismatch 0 preserved - vault handlers now read the same 13 credentials from the credentials table populated by migration 090 instead of observations Verification: - go build ./... - green - go vet ./internal/worker/... ./internal/mcp/... ./internal/db/gorm/... - clean - go test ./internal/worker/ -run "HandleStoreMemoryExplicit|HandleDeleteMemoryByID" - all 4 PASS + 2 sub-tests - go test ./internal/db/gorm/ -run "Credential|Memory|BehavioralRules" - 9 top-level + 10 sub-tests PASS (regression) Task: T021 Commit E Spec: .agent/specs/engram-v5-baseline/spec.md FR-4 + plan.md Phase 5
Integration test TestCredentialDecryptRoundTripAfterMigration in
internal/grpcserver/credential_migration_test.go verifies the core US3
invariant: credentials migrated from `observations` into the dedicated
`credentials` table via migration 090 preserve AES-256-GCM ciphertext
byte-for-byte, and the vault can still decrypt every migrated secret
back to its original plaintext with 100% fidelity.
Why this gate exists:
Migration 090 copies encrypted_secret from observations into
credentials via a SQL SELECT into a BYTEA column. If either end
silently mutated the bytes (encoding, trimming, pad handling),
AES-GCM decrypt would fail authentication-tag check and the
production vault would become unusable. That would lose all 13
prod credentials irrecoverably.
What the test does:
1. Opens a real Postgres + pgvector DB, applies migrations through 090.
2. Creates a real crypto.Vault with a deterministic test key
(not the prod key — the test is scoped to a unique test project
slug to avoid colliding with other data).
3. Encrypts 5 diverse plaintexts via the real vault:
- short ASCII
- empty plaintext (GCM edge case: 0-byte plaintext produces
nonce+tag = 28-byte ciphertext)
- unicode mixed (Cyrillic + CJK + emoji, multi-byte UTF-8)
- long plaintext (~1KB of repeated bytes)
- binary-like (embedded NUL bytes + high-bit bytes, guards
against any future code that reads the secret as a text string
instead of raw bytes)
4. Seeds each plaintext as an observations row with the ciphertext
+ fingerprint + type='credential' + the required NOT NULL columns.
5. Seeds 3 ADVERSARIAL EXCLUDED observations (is_suppressed=true,
is_archived=1, is_superseded=1 — one each). Migration 090 filters
these out via its WHERE clause; if the filters ever get loosened,
one of the assertions in sub-test adversarial_excluded_rows_not_
migrated will fire.
6. Seeds 1 ORPHAN observation encrypted with a different (wrong)
fingerprint to exercise CountWithDifferentFingerprint.
7. Re-runs the exact migration 090 credentials INSERT SQL scoped to
the test project slug (the SQL is copy-pasted verbatim from
internal/db/gorm/migrations.go — drift will fail the test).
8. Reads each credential back via CredentialStore.Get and asserts:
- encrypted_secret bytes == original ciphertext (byte-for-byte)
- encryption_key_fingerprint == vault fingerprint (exact)
- vault.Decrypt(migrated_ciphertext) == original plaintext
9. Three adversarial sub-tests:
- orphan_credential_visible_via_fingerprint_check: verifies the
badFP orphan is visible in List and counted globally by
CountWithDifferentFingerprint(goodFP).
- adversarial_excluded_rows_not_migrated: verifies each of the
three excluded observations (suppressed / archived / superseded)
is NOT present in the credentials table. This is the load-
bearing test for the WHERE clause filters.
- tampered_ciphertext_fails_decrypt: flips one byte in the
migrated ciphertext and verifies AES-GCM authentication
catches the mutation. This guards against "decrypt accepts
anything" — which would make the gate a stub.
Anti-stub proof:
Swap vault.Decrypt body -> return "", nil ⇒ short_ascii / unicode /
long / binary sub-tests all fail (decrypted != plaintext) AND
tampered_ciphertext_fails_decrypt fails (require.Error vs nil).
Test cannot pass with a stubbed decrypt path.
Resource hygiene:
NewStore's *Store is captured and closed via defer gormStore.Close().
A separate direct gorm.Open handle is used for raw INSERT seeds and
the project-scoped migration re-run, explicitly closed via its
sqlDB.Close() defer.
Scope guard:
ONE new file: internal/grpcserver/credential_migration_test.go.
No production code changed. Uses only exported APIs:
crypto.NewVault, crypto.Vault.Encrypt/Decrypt/Fingerprint,
gorm.NewStore, gorm.Config, gorm.Store.Close, gorm.NewCredentialStore,
gorm.CredentialStore.Get/List/CountWithDifferentFingerprint.
Verified:
DATABASE_DSN=postgres://.../engram_f1_test — TestCredentialDecrypt-
RoundTripAfterMigration PASS (1 top-level + 8 sub-tests, ~1.8s wall).
Sub-tests: short_ascii, empty_plaintext, unicode_mixed,
long_plaintext_1kb, binary_like_with_nul_and_high_bytes,
orphan_credential_visible_via_fingerprint_check,
adversarial_excluded_rows_not_migrated, tampered_ciphertext_fails_decrypt.
Does NOT verify (intentional — out of scope for CI F-1 gate):
- The production invariants credential_count=13, fingerprint=
aa78e55cf896508c, mismatch_count=0. Those belong to the
staging dry-run against the real 2026-04-17 pg_dump snapshot,
run out of band by the operator before Commit G lands. F-1 CI
gate proves the PRIMITIVE works (round-trip byte-preserving);
staging dry-run proves the MIGRATION ran correctly on prod data.
Pre-existing test failures (NOT regressions from this commit):
- TestSyncProjectState_* (internal/grpcserver) — introduced in
d324f01 (PR #170, Phase 4) with a minimal schema that conflicts
with the full projects-table migration. Fails on any DB with
migrations 045+ applied.
- TestHandleMemoryTriggers_BashCommand* (internal/worker) — from
fd8ce26 (PR #135, learning-memory-v4) — returns 500 not 200
on fresh pgvector DB. Unrelated to US3.
Ref: .agent/specs/engram-v5-baseline/changes/CR-001-initial-scope/
tasks.md T022 AC 1-5. Addresses two review findings:
(1) NewStore return value leaked pool — fixed via capture+Close.
(2) Exclusion filter coverage gap — fixed via adversarial sub-test.
Two pre-existing regressions in internal/grpcserver/sync_project_state.go and its test, shipped since PR #170 (d324f01, 2026-04-18 gRPC Phase 4): 1. Raw(...ANY(?)..., localIDs) and Exec(...ANY(?)..., now, localIDs) passed []string directly. The gorm postgres driver does NOT encode []string as a PostgreSQL array for positional params, so the query failed with ERROR: malformed array literal (SQLSTATE 22P02). Fix: wrap localIDs with pq.Array() at both call sites. Mirrors the working pattern in internal/sessions/store.go:189. 2. TestSyncProjectState_HeartbeatUpdated flaked on Windows: Go's time.Now().UTC() has 100-nanosecond resolution from the monotonic clock; PostgreSQL timestamptz stores microsecond precision. When the captured 'before' fraction was in the same microsecond bucket as the server's UPDATE timestamp, the round-tripped newHB was strictly less-than 'before' and the assertion fired. Fix: Truncate(time.Microsecond) on 'before' so the comparison operates at the precision the DB stores. Verified: go test -count=3 -run ^TestSyncProjectState ./internal/grpcserver/ — 5 tests x 3 iterations = 15 passes, none flaked.
Two interconnected regressions in GetObservationsByCommandPrefix and its test harness, shipped in PR #135 (fd8ce26, learning-memory-v4). Never passed on a fresh pgvector DB. 1. SQL: ESCAPE '\\\\' in Go double-quoted source produces '\\\\' in the SQL text, which under standard_conforming_strings=on (PostgreSQL 9.1+ default) is a 2-character escape string. PostgreSQL rejects multi-character ESCAPE with SQLSTATE 22025 ('invalid escape string'). Any call to this function against a real Postgres returned 500 from handleMemoryTriggers. 2. Semantics: the function name says 'ByCommandPrefix' — intent is to find observations whose stored commands_run entry is a PREFIX of the incoming query (e.g. stored 'git push --force' matches query 'git push --force origin main'). The old SQL implemented the reverse direction: cmd LIKE 'query%' finds stored values that START WITH the query, which no real case exercises. Fix: use starts_with(?, cmd) — PostgreSQL 11+ native function with clean semantics (no escape hazard, no wildcard ambiguity, both sides treated as literals). The escaper + % suffix are removed as no longer needed. Additionally, internal/worker/trigger_matcher_bash_test.go seeded commands with the narrative prefix 'Executed: ' (e.g. 'Executed: git push --force'), which does not reflect how the Bash tool records commands in production (raw command, no prefix). The test could never pass with a correct prefix matcher because the seed shape mismatched the matcher shape. Fix: strip 'Executed: ' from all 5 seed call sites. Verified (fresh pgvector/pgvector:pg17 with truncated observations): go test -count=1 -run '^TestHandleMemoryTriggers_' ./internal/worker/ — 6 subtests all PASS. TestHandleMemoryTriggers_BashCommandPrefixMatchesTop3Warnings now correctly returns the 3 bugfix+pitfall prefix matches (excluding the discovery-typed row and the non-prefix force-with-lease row).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughДобавлены три новые сущности БД (credentials, memories, behavioral_rules) с миграциями и GORM‑моделями; реализованы соответствующие GORM‑хранилища и интеграционные тесты; добавлены REST‑эндпоинты и MCP‑инструменты для памяти и правил; сменён обработчик vault для работы с CredentialStore; внесены мелкие тестовые и SQL‑правки. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCP_Server as "MCP Server\n(internal/mcp/server.go)"
participant BR_Store as "BehavioralRulesStore\n(internal/db/gorm)"
participant DB as "Postgres DB"
Client->>MCP_Server: tools/call {action: "store_rule", args...}
MCP_Server->>BR_Store: Create(ctx, BehavioralRule)
BR_Store->>DB: INSERT INTO behavioral_rules (...)
DB-->>BR_Store: inserted row (id, timestamps, version)
BR_Store-->>MCP_Server: mapped BehavioralRule (with id)
MCP_Server-->>Client: JSON response {id, project, content, created_at}
Estimated code review effort🎯 4 (Сложный) | ⏱️ ~60 минут Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates credentials, memories, and behavioral rules from the legacy observations table into dedicated tables, introducing new GORM stores, migrations (087-090), and associated REST/MCP handlers. Critical feedback was provided regarding a timestamp conversion bug in migration 090 where millisecond epochs are passed to TO_TIMESTAMP without scaling. Additionally, the use of the starts_with function may limit PostgreSQL compatibility to version 17+, and an inconsistency was noted in the CredentialStore between its hard-delete implementation and the soft-delete schema.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
.gitignore (1)
93-96: Рассмотрите уточнение шаблонов кэша.Добавленные шаблоны содержат избыточные варианты с дефисом и подчёркиванием (
.tmp-gocache/и.tmp_gocache/, аналогично дляgomodcache). Если инструменты генерируют только один формат имён, можно оставить только нужный вариант для упрощения файла.Кроме того, шаблон
.cache/является более широким, чем специфичные для Go шаблоны кэша, и игнорирует любую директорию.cache/в репозитории. Если намерение — игнорировать только кэши Go, рассмотрите более специфичный шаблон, либо переместите.cache/в отдельную секцию с соответствующим комментарием.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 93 - 96, Уберите дублирующие варианты имён кэша и сделайте шаблоны более специфичными: оставьте только фактически генерируемые имена (например, либо .tmp_gocache/ либо .tmp-gocache/, и аналогично для .tmp_gomodcache/), удалив избыточные строки .tmp-gocache/ и/или .tmp_gomodcache/; если вы хотите игнорировать только Go-кэши, уточните шаблоны (например оставить .tmp_gocache/ и .tmp_gomodcache/ или заменить на более точные пути внутри build-каталогов). Перенесите общую запись .cache/ в отдельную секцию с комментарием или замените её на более узкий шаблон, чтобы не игнорировать любые .cache/ в репозитории.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/db/gorm/behavioral_rules_store.go`:
- Around line 41-50: The code sets row.Project = rule.Project which preserves
the caller's pointer and can mutate the returned model if the original is
reassigned; in the Create path (BehavioralRule creation) replace the pointer
aliasing by allocating a new Project value and copying the contents from
rule.Project (e.g., create a new Project struct and copy fields or dereference
rule.Project into a new value) so row.Project points to an independent copy;
update the BehavioralRule construction in the Create implementation to use that
new copy instead of the original pointer.
In `@internal/db/gorm/credential_store.go`:
- Around line 51-60: The Credential creation reuses the input buffer and calls
time.Now() twice; fix by deep-copying cred.EncryptedSecret into a new byte slice
when assigning to row.EncryptedSecret (so subsequent zeroize/reuse of the
caller's buffer won't mutate the stored model) and capture a single now :=
time.Now().UTC() once to set both row.CreatedAt and row.UpdatedAt; modify the
code that constructs row (the variable named row of type Credential) to perform
the slice copy and use the single now variable for both timestamps.
- Around line 121-123: Метод удаления в credential_store.go сейчас удаляет без
фильтра по deleted_at и поэтому может физически удалить уже soft-deleted запись;
добавьте в цепочку запроса явный фильтр для активных записей (например
дополнительное Where("deleted_at IS NULL") перед Delete на
s.db.WithContext(ctx).Where("project = ? AND key = ?", project,
key).Delete(&Credential{})) и после выполнения запроса проверяйте
result.RowsAffected == 0 и возвращайте gorm.ErrRecordNotFound, чтобы поведение
совпадало с остальными методами стора.
In `@internal/db/gorm/migrations.go`:
- Around line 2952-2960: The migration's INSERT into credentials omits the scope
column, losing the scope value from legacy observations; update the INSERT
statement in migrations.go that targets the credentials table to include the
scope column in the column list and add scope (or COALESCE(scope,
'project'/'global' as appropriate) if you need a default) to the SELECT clause
from observations so the original scope is preserved for migrated rows.
- Around line 3077-3090: Rollback for migration
090_observations_to_static_entities currently TRUNCATEs memories,
behavioral_rules and credentials which will remove any rows added after the
migration (causing data loss); either make the Rollback explicitly irreversible
by returning a non-nil error (e.g. in the Rollback func return
fmt.Errorf("migration 090 rollback is irreversible")) or change the forward
migration (the code that copies from observations) to mark migrated rows with a
migration identifier/flag (e.g. migrated_by_migration = '090' or migration_id
column) and then change the Rollback to delete only rows matching that marker
(use tx.Exec("DELETE ... WHERE migration_id='090'") or equivalent) instead of
TRUNCATE; update Rollback in the Rollback func of migration
090_observations_to_static_entities and ensure any new column/marker is created
in the forward migration so the selective delete is safe.
In `@internal/grpcserver/credential_migration_test.go`:
- Around line 65-74: The test uses NewStore (which already applies migrations)
then inserts data with a copy-pasted INSERT...SELECT for migration 090, so the
HARD GATE checks a local SQL copy instead of the real migration body; fix by
making the migration 090 SQL the single source of truth and using it in both the
migration and the test: extract the migration 090 SQL into a shared exported
constant or helper in the migrations package (the same place as migrations.go)
and reference that constant from the test instead of duplicating the SQL, or
change the test to execute the actual migration 090 routine after seeding by
invoking the exported migration runner/wrapper that applies that specific
migration (so NewStore and the test both use the real migration code rather than
a copy-paste).
In `@internal/mcp/server.go`:
- Around line 929-954: The tools "store_rule" and "list_rules" are always
advertised even when behavioralRulesStore isn't injected, causing capability
discovery failures; update the tool registration to only append the Tool entries
for Name "store_rule" and "list_rules" when behavioralRulesStore (symbol
behavioralRulesStore on the server struct) is non-nil (same pattern used for
other store-dependent tools), i.e. wrap the two Tool definitions in a
conditional if m.behavioralRulesStore != nil { ... } so tools/list won't
advertise unusable capabilities and internal/mcp/tools_rules.go early-error
paths are avoided.
In `@internal/worker/handlers_memories.go`:
- Around line 67-70: The handler currently logs full error details with
log.Error().Err(err)... but then returns the raw err.Error() in http.Error (e.g.
http.Error(w, "store memory: "+err.Error(), http.StatusInternalServerError)),
which leaks internal DB/SQL details; change the http.Error calls to return a
stable, non-sensitive 500 message (e.g. "store memory: internal server error" or
"internal server error") while keeping the existing log.Error().Err(err)...
calls intact; apply this same change to the other two occurrences that follow
the same pattern (the blocks around the http.Error calls at the other two
reported locations).
- Around line 102-109: Validate and cap the "limit" query param before calling
s.memoryStore.List: parse raw := r.URL.Query().Get("limit") with strconv.Atoi,
ensure the value is >=1 and <= a defined max (e.g., const maxLimit = 500 or
reuse existing list_rules constant), and if it is outside that range return an
HTTP 400 error (do not call s.memoryStore.List). If absent, use a safe default
(e.g., 50). This prevents extremely large limits from being passed to
s.memoryStore.List and causing huge DB queries/JSON responses.
In `@internal/worker/handlers_vault.go`:
- Around line 54-60: The code currently allows creating global credentials via
the POST /api/vault/credentials handler but other handlers (List, Get, Delete)
call s.credentialStore with an empty project which the store treats as invalid,
making global creds unreadable/unremovable; fix by choosing one approach: either
(A) temporarily disallow global creds in the create handler by validating that
the request includes a non-empty project and returning a 400 (update the POST
/api/vault/credentials handler to reject empty project), or (B) make global
handling consistent by normalizing the project identifier across handlers and
the store (choose a canonical token like "__global__" or add a boolean scope
flag) and update s.credentialStore.List, CredentialStore.Get and
CredentialStore.Delete to accept and treat that canonical/global identifier
correctly (also change the List handler that currently returns writeJSON(w,
[]credItem{}) for empty project to use the same normalization). Ensure the same
normalization/validation logic is applied in all handlers that call
s.credentialStore (List, Get, Delete and the POST create handler).
---
Nitpick comments:
In @.gitignore:
- Around line 93-96: Уберите дублирующие варианты имён кэша и сделайте шаблоны
более специфичными: оставьте только фактически генерируемые имена (например,
либо .tmp_gocache/ либо .tmp-gocache/, и аналогично для .tmp_gomodcache/),
удалив избыточные строки .tmp-gocache/ и/или .tmp_gomodcache/; если вы хотите
игнорировать только Go-кэши, уточните шаблоны (например оставить .tmp_gocache/ и
.tmp_gomodcache/ или заменить на более точные пути внутри build-каталогов).
Перенесите общую запись .cache/ в отдельную секцию с комментарием или замените
её на более узкий шаблон, чтобы не игнорировать любые .cache/ в репозитории.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 79b87380-e095-4d36-afd4-212a3b070d06
📒 Files selected for processing (23)
.gitignoreinternal/db/gorm/behavioral_rules_store.gointernal/db/gorm/behavioral_rules_store_test.gointernal/db/gorm/credential_store.gointernal/db/gorm/credential_store_test.gointernal/db/gorm/memory_store.gointernal/db/gorm/memory_store_test.gointernal/db/gorm/migrations.gointernal/db/gorm/models.gointernal/db/gorm/observation_store.gointernal/grpcserver/credential_migration_test.gointernal/grpcserver/sync_project_state.gointernal/grpcserver/sync_project_state_test.gointernal/mcp/server.gointernal/mcp/tools_rules.gointernal/worker/handlers_memories.gointernal/worker/handlers_memories_test.gointernal/worker/handlers_vault.gointernal/worker/service.gointernal/worker/trigger_matcher_bash_test.gopkg/models/behavioral_rule.gopkg/models/credential.gopkg/models/memory.go
…ore hardening
Addresses 6 review findings across 3 files:
migrations.go (3 CRITICAL, 1 MAJOR, 1 MAJOR):
- TO_TIMESTAMP(created_at_epoch) was interpreting milliseconds as
seconds, producing year-56247 dates. Divided by 1000.0 in all
three INSERT sites (credentials, behavioral_rules, memories).
- credentials INSERT now copies 'scope' column via
COALESCE(NULLIF(scope,''),'project'); schema has the column,
legacy observations carry it, previous SELECT dropped it silently.
- Migration 090 rollback previously TRUNCATEd the 3 tables, which
would wipe data created AFTER the forward migration. Replaced
with an explicit error instructing pg_restore from the 2026-04-17
snapshot per rollback-procedure.md §3b.
observation_store.go (LOW):
- Documentation note: starts_with() is PostgreSQL 9.1+ (gemini
reviewer claimed 17+ — verified against release notes).
engram targets PG 17 in all live deployments; no portability
concern for current scope.
credential_store.go (MEDIUM, LOW, MEDIUM):
- Create: capture a single 'now' up-front so CreatedAt==UpdatedAt
deterministically; copy the secret bytes into a defensive slice
so subsequent zeroize/reuse by the caller cannot alias the row.
- Delete: added 'deleted_at IS NULL' WHERE predicate so soft-deleted
rows remain soft-deleted (was possible to hard-delete a tombstone
and return success, instead of gorm.ErrRecordNotFound).
- Preserve hard-delete for the rotation case (UNIQUE(project,key)
constraint + documented design note).
Verified: go build + go vet clean; TestCredentialDecryptRoundTripAfterMigration
+ TestCredentialStore_* all green on pgvector/pgvector:pg17.
Copy rule.Project into row.Project by value so the returned model does not share a backing string with the caller's variable. If the caller mutates its original input after Create returns, the stored model should not change — matches the API contract implied by 'Create returns the persisted row'.
…ntial guards
Addresses 3 MAJOR + 2 MEDIUM review findings in HTTP handlers:
handlers_memories.go:
- handleListMemories now caps 'limit' at 500 and rejects
non-positive values with 400. Prevents an adversarial or
misconfigured client from triggering an unbounded SELECT and
multi-MB JSON response.
- 500 responses no longer include err.Error() verbatim — both
store + request failures now emit stable messages; full error
stays in the server log (information-leak mitigation).
handlers_vault.go:
- handleStoreCredential rejects scope='global' with 400 until a
schema migration admits project-less credentials (credentials.project
is NOT NULL); previously the write would either fail the NOT NULL
constraint or create a row that no READ path could retrieve (LIST,
GET, DELETE all require project).
- handleGetCredential and handleDeleteCredential now require project
to be non-empty; returning 400 instead of silently returning [] /
ErrRecordNotFound for empty-project requests.
- 500 responses no longer leak err.Error() — stable client message,
full error in log.
…ration 090 schema, gitignore cleanup
Addresses 2 MEDIUM + 1 MAJOR review findings + infra hygiene:
internal/mcp/server.go (MEDIUM):
- store_rule and list_rules MCP tools are now only registered when
s.behavioralRulesStore != nil. Previously tools/list always
advertised them even though the handlers returned 503 at dispatch
time — clients would see the tool, invoke it, and get an error.
Same conditional pattern as versionedDocumentStore elsewhere in
this file.
internal/grpcserver/credential_migration_test.go (MAJOR — F-1 sync):
- Test SQL updated to match migration 090's corrected form: scope
column added to the INSERT list, TO_TIMESTAMP divided by 1000.0.
Without this sync the HARD GATE would pass against stale SQL while
production would use the fixed SQL — defeating the purpose of the
gate. The test still copy-maintains the SQL verbatim; a drift
detection mechanism is out of scope for this PR and noted for
future work.
- Seed timestamps now use UnixMilli() to match observations schema
convention (created_at_epoch is milliseconds per models.go).
.gitignore:
- Deduplicated Go cache patterns (collapsed hyphen/underscore
variants into one), moved .cache/ into the dedicated Go cache
section, removed stray duplicates.
Verified: all target tests green on pgvector/pgvector:pg17 after these
commits — TestCredentialDecryptRoundTripAfterMigration (1 top + 8 sub),
TestSyncProjectState_* (5), TestHandleMemoryTriggers_Bash* (3),
TestCredentialStore_* (9 incl sub-tests), behavioral_rules + memory
store suites.
Owner
Author
🤖 PR Review MCP State (auto-managed, do not edit){
"version": 2,
"parentChildren": {},
"resolvedNitpicks": {
"coderabbit-nitpick-42480b1e-93": {
"resolvedAt": "2026-04-18T09:14:39.433Z",
"resolvedBy": "agent"
}
},
"updatedAt": "2026-04-18T09:14:39.867Z"
} |
Owner
Author
|
@coderabbitai review incremental |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/worker/handlers_vault.go`:
- Around line 60-64: В блоке обработки ошибки при списке учётных данных не
возвращайте err.Error() в 500-ответе: оставьте логирование ошибки через
log.Error().Err(err).Msg("list credentials failed") но измените вызов
http.Error(w, ...) чтобы отправлять безопасное, стабильное сообщение (например
"failed to list credentials" или "internal server error") вместо конкатенации
err.Error(); найдите этот код по идентификаторам log, err и http.Error в файле
handlers_vault.go и замените только текст ответа, сохранив код статуса
http.StatusInternalServerError.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ddfd400e-6147-4d9d-ae92-475300afd73e
📒 Files selected for processing (9)
.gitignoreinternal/db/gorm/behavioral_rules_store.gointernal/db/gorm/credential_store.gointernal/db/gorm/migrations.gointernal/db/gorm/observation_store.gointernal/grpcserver/credential_migration_test.gointernal/mcp/server.gointernal/worker/handlers_memories.gointernal/worker/handlers_vault.go
✅ Files skipped from review due to trivial changes (3)
- .gitignore
- internal/grpcserver/credential_migration_test.go
- internal/mcp/server.go
Addresses CodeRabbit MINOR finding PRRT_kwDORY-ek8577YWO on re-review of PR #181: 'Don't disclose error details in 500-response' (handlers_vault.go:62). Initial fix only covered some handlers; this commit extends the same mitigation to the remaining five 500 sites in handlers_vault.go: - handleListCredentials line 62: 'list credentials: ' + err.Error() → 'failed to list credentials' - handleGetCredential line 111: 'vault not available: ' + err.Error() → 'vault not available' - handleGetCredential line 137: 'decrypt credential: ' + err.Error() → 'failed to decrypt credential' - handleStoreCredential line 207: 'vault not available: ' + err.Error() → 'vault not available' - handleStoreCredential line 214: 'encrypt credential: ' + err.Error() → 'failed to encrypt credential' - handleStoreCredential line 229: 'store credential: ' + err.Error() → 'failed to store credential' All sites now call log.Error().Err(err).Msg(...) first so server-side debugging still has full context. The 400 (invalid JSON body) site keeps err.Error() in the body — 400 is a client-parse error and the detail helps the client diagnose their own malformed request; this is intentional and not flagged by the reviewer. Verified: go build + go vet clean.
Owner
Author
|
@coderabbitai review incremental |
|
✅ Actions performedReview triggered.
|
This was referenced Apr 19, 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
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.
US3 Observations Split — PR-A (commits A→F-1 + 2 regression fixes)
Splits
observationsinto three purpose-built static-entity tables(
credentials,memories,behavioral_rules), migrates data byte-for-bytepreserving vault ciphertext, and verifies the decrypt round-trip through a
HARD GATE integration test. This is PR-A of the two-PR US3 sequence; PR-B
will drop
observations+ raw log tables after F-1 is green in CI.Spec:
.agent/specs/engram-v5-baseline/spec.mdFR-9 / NFR-7Plan:
.agent/specs/engram-v5-baseline/plan.md§Phase 5 (Amendment 2026-04-18)Tasks:
.agent/specs/engram-v5-baseline/changes/CR-001-initial-scope/tasks.mdT017..T022Variant: 2 (PR-A: A→F-1, PR-B: G+H after F-1 CI green)
Non-negotiable invariants (PRESERVED end-to-end)
aa78e55cf896508c— unchangedcredential_count= 13 — exactmismatch_count= 0 — exactcontenttable: NOT TOUCHEDCommits
Feature commits (US3)
3e3a717226591ecb01ad2f472807badcc8cdbae9cb23a705cRegression fixes (pre-existing bugs discovered during US3 testing — "регрессии чинить сразу")
f8a55b87c2faf4What Commit F-1 proves
Integration test
TestCredentialDecryptRoundTripAfterMigration(1 top-level + 8 sub-tests, ~1.8s wall on pgvector/pgvector:pg17):short_asciiempty_plaintextunicode_mixedlong_plaintext_1kbbinary_like_with_nul_and_high_bytesorphan_credential_visible_via_fingerprint_checkadversarial_excluded_rows_not_migratedtampered_ciphertext_fails_decryptAnti-stub proof: swap
vault.Decryptbody →return "", nilbreaks 5 of 8 sub-tests (all 4 real-plaintext cases + tampered). Test cannot pass with a stubbed decrypt path.Does NOT assert production invariants (13 / aa78e55cf896508c / 0). Those are verified out of band in staging dry-run against the real 2026-04-17 pg_dump snapshot (SHA
43e9e75c...f922) by the operator before PR-B (Commit G) lands. F-1 CI gate proves the PRIMITIVE works.Rollback procedure
See
.agent/specs/engram-v5-baseline/changes/CR-001-initial-scope/rollback-procedure.md(254 lines, 9 sections, PENDING user review before merge).Covers: detection signals, forensic capture, pg_restore from 2026-04-17 baseline, schema revert order (with 090 not-idempotent warning), git revert for PR-A + PR-B squash SHAs, post-rollback verification, executor checklist.
Verified
go build ./...— cleango vet ./...— cleango test -count=3 -run '^TestSyncProjectState' ./internal/grpcserver/— 15/15 passgo test -count=1 -run '^TestHandleMemoryTriggers_' ./internal/worker/— 6/6 passgo test -count=1 -run TestCredentialDecryptRoundTripAfterMigration ./internal/grpcserver/— 9/9 pass (1 top + 8 sub)go test -count=1 ./internal/grpcserver/ ./internal/worker/— all passKnown pre-existing failures (NOT regressions from this PR)
TestMigrationsIntegration(internal/db/gorm) —vector dimension mismatch: got 384, want 2000. Pre-existing from US2 (PR chore(v5): drop embeddings storage + vector/embedding packages (PR-v5-2, US2) #180, 83bbc62) — to be resolved in US3 PR-B or US7 when obsolete vector columns are dropped.Out of scope (→ PR-B)
observations|user_prompts|raw_event_store|indexed_sessions|summaries|sdk_sessionsreferencesSquash merge
Once review is clean, squash-merge. The rollback procedure §5 references this PR's squash SHA as the revert target.
Summary by CodeRabbit
Примечания к выпуску
Новые возможности
Исправления
Тесты