Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
446 changes: 446 additions & 0 deletions docs/todos/2026-06-19-issue-312-graph-auto-stale-edges/plan.md

Large diffs are not rendered by default.

139 changes: 139 additions & 0 deletions docs/todos/2026-06-19-issue-312-graph-auto-stale-edges/todo.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
# Issue 312 Graph Edge Auto-Stale Reactions

Scope: GitHub issue #312 in branch `issue/312-graph-auto-stale-edges`.

## Validity Evidence

- Worktree: `/Users/A1538552/.codex/worktrees/0c8e/agentmemory`.
- Branch: `issue/312-graph-auto-stale-edges`, initially created from local `origin/main` at `499b53fc`.
- Current status before implementation planning: clean branch, local `origin/main` is 21 commits ahead of the branch; after remote tracking moved, the branch reports 26 commits behind `origin/main`.
- Remote boundary: target only `origin` (`https://github.com/wbugitlab1/agentmemory.git`); do not target `upstream` (`https://github.com/rohitg00/agentmemory.git`).
- Issue #312 is open and requests `KV.graphEdges` state-trigger reactions for `supersedes`, `contradicts`, and `extends`.
- `src/triggers/events.ts` has the existing state-trigger pattern on `KV.sessions`.
- `src/functions/graph.ts` and `src/functions/temporal-graph.ts` write `KV.graphEdges`, but no graph-edge reaction handler exists.
- `src/types.ts` currently excludes `supersedes`, `contradicts`, and `extends` from `GraphEdgeType`; `src/functions/graph-type-guards.ts` and graph extraction prompts reject/drop those lifecycle edge types.
- `src/functions/remember.ts` and `src/functions/relations.ts` contain inline/direct memory supersession behavior, but graph-edge writes do not mutate target `Memory` rows.

## Sprint Contract

Goal: Implement issue #312 so persisted lifecycle graph edges trigger deterministic target-memory reactions.

Scope:
- Add lifecycle graph edge vocabulary for `supersedes`, `contradicts`, and `extends`.
- Add an internal `event::graph::edge::reaction` state-trigger handler on `KV.graphEdges`.
- Add focused tests for graph edge vocabulary, trigger registration, create/delete reactions, dedupe, missing targets, cycle rejection, and multi-edge un-reaction.
- Keep reaction bookkeeping in existing `KV.state`.
- Resolve graph endpoints only through direct memory ids, explicit `GraphNode.properties.memoryId`, or graph node names that exactly match memory ids. Do not infer memory identity from merged `sourceObservationIds`.

Non-goals:
- No MCP tool count changes, REST endpoint additions, auth changes, dependency changes, package metadata changes, migrations, or new external services.
- No `upstream` remote usage.
- No remote push, PR creation, PR merge, issue closure, or thread archival until green verification and the remote-write phase.

Acceptance criteria:
- `GraphEdgeType` and graph extraction allow `supersedes`, `contradicts`, and `extends` lifecycle edges.
- `event::graph::edge::reaction` is registered as a state trigger over `KV.graphEdges`.
- On create, `supersedes` marks the target memory `isLatest: false` and decays strength.
- On create, `contradicts` decays target confidence when present and target strength as the local memory score, floored at `0.05`.
- On create, `extends` adds the source memory id to target `relatedIds` with set semantics.
- Delete/un-reaction is deterministic and only restores after no remaining same reaction group edge still applies.
- Missing or ambiguous memory endpoints skip safely with audit evidence.
- Cyclic lifecycle edges are rejected at extraction where possible and defensively skipped by the handler.

Intended verification:
- Red/green targeted Vitest tests for new behavior.
- `corepack pnpm exec vitest run --exclude test/integration.test.ts test/graph-edge-reactions.test.ts test/events-boundary.test.ts test/graph.test.ts`.
- `corepack pnpm run lint`.
- `corepack pnpm run build`.
- `corepack pnpm test`.
- Required security gates before commit/PR prep because this changes state-trigger behavior, protocol/schema handling, persistence, and agent workflow behavior: Semgrep and staged Gitleaks; OSV only if dependency/package surfaces change.

Known boundaries:
- Current-turn user request invoked `$github-feature-loop` after approving the recommended full approach; assumption: graph edge vocabulary expansion is approved for issue acceptance.
- Branch is behind local `origin/main` by 21 commits; no fetch/pull has run in this turn.
- `github-push-prepare` tool/skill was not discoverable through `tool_search`; local branch-prep steps may need a documented fallback or user checkpoint later.

Stop conditions:
- Endpoint mapping from graph edge endpoints to memories cannot be made conservative and deterministic.
- A required fix expands auth, REST/MCP surfaces, dependencies, migrations, remote state, or package metadata beyond the approved graph vocabulary change.
- Required verification is blocked and no targeted alternative covers the changed surface.

## Feature / Verification Matrix

| Change | Verification method | Status | Evidence |
| --- | --- | --- | --- |
| Lifecycle graph vocabulary | Red/green tests in `test/graph.test.ts` and type/guard compile | Passed | `corepack pnpm exec vitest run --exclude test/integration.test.ts test/events-boundary.test.ts test/graph.test.ts test/graph-edge-reactions.test.ts`: 60 passed |
| State trigger registration | `test/events-boundary.test.ts` asserts `KV.graphEdges` trigger | Passed | Same targeted run: 60 passed |
| Create reactions | `test/graph-edge-reactions.test.ts` covers `supersedes`, `contradicts`, `extends` | Passed | Same targeted run: 60 passed |
| Delete/un-reaction | `test/graph-edge-reactions.test.ts` covers duplicate and multi-edge delete | Passed | Same targeted run: 60 passed |
| Missing/ambiguous/cyclic safety | `test/graph-edge-reactions.test.ts` and `test/graph.test.ts` | Passed | Same targeted run: 60 passed |
| Update/stale event semantics | `test/graph-edge-reactions.test.ts` old-active/new-active tests | Passed | Same targeted run: 60 passed |
| No full graph-edge scan in handler | KV mock fails on `list(KV.graphEdges)` during reaction tests | Passed | Reaction tests passed with `failOnGraphEdgeList: true` |
| Concurrent same-target reactions | Clone-on-read KV `Promise.all` tests | Passed | Same targeted run: 60 passed |
| Full repo compatibility | lint, build, full tests | Passed with note | `corepack pnpm run lint`; `corepack pnpm test`: 208 files/2845 tests passed on isolated rerun; `corepack pnpm run build` passed |

## Subagent Ledger

| Workstream | Scope | Edits allowed | Expected output | Result | Residual risk |
| --- | --- | --- | --- | --- | --- |
| Arena candidate 1 | Implementation strategy | No repo edits | Strategy and rationale | Recommended state trigger + `KV.state` reaction groups | Endpoint mapping still needs careful implementation |
| Arena candidate 2 | Implementation strategy | No repo edits | Strategy and rationale | Recommended explicit graph vocabulary checkpoint and exact-one endpoint rule | Proposed new KV scope/audit op rejected |
| Arena candidate 3 | Implementation strategy | No repo edits | Strategy and rationale | Added untrusted payload narrowing and concurrency/idempotency test ideas | GraphNode staling model rejected |
| Arena judge | Candidate comparison | No repo edits | Scores and base recommendation | Recommended Candidate 1 as base with grafts from 2/3 | Parent must implement and verify |
| Pre-implementation spec/acceptance review | Plan and task record | No repo edits | High/Medium findings or ACCEPT | Found missing update/stale semantics and positive GraphNode endpoint coverage | Plan patched before implementation |
| Pre-implementation architecture review | Plan and graph integration | No repo edits | High/Medium findings or ACCEPT | Found cycle contract mismatch, unsafe provenance resolution, temporal scope creep, and skip-audit target issue | Plan patched before implementation |
| Pre-implementation verification review | Plan and tests | No repo edits | High/Medium findings or ACCEPT | Found missing extracted endpoint, no-scan, cycle, and concurrency coverage | Plan patched before implementation |

## Arena Synthesis

Synthesis file: `/tmp/arena-issue-312/synthesis.md`.

Base: Candidate 1. Grafts: Candidate 2's explicit graph-vocabulary checkpoint and endpoint ambiguity rule, plus Candidate 3's untrusted payload narrowing and concurrency/idempotency tests.

Rejected: GraphNode staling/snapshot rewrite, new public API surface, new audit operation by default, full graph scans in hot handler, and temporal `succeeded_by` aliasing.

## Progress

- Read repo instructions and relevant workflow skills.
- Created/switch branch `issue/312-graph-auto-stale-edges`.
- Validated issue #312 from GitHub issue data and local source evidence.
- Ran arena and cross-judge. Candidate 1 selected as base.
- Created this task record and implementation plan before production code edits.
- Ran pre-implementation plan review with three read-only subagents.
- Triage:
- Fixed plan gap for update/stale events by requiring old-active/new-active semantics and tests.
- Fixed endpoint-resolution risk by forbidding `sourceObservationIds` inference and requiring positive `GraphNode.properties.memoryId`/name tests.
- Fixed cycle mismatch by choosing "drop all lifecycle edges participating in an in-batch directed cycle" and adding 3-node directed cycle tests.
- Fixed temporal scope creep by removing temporal prompt changes from this issue.
- Fixed no-scan/concurrency gaps by adding explicit tests.
- Implemented lifecycle graph edge vocabulary in `src/types.ts`, `src/functions/graph-type-guards.ts`, and `src/prompts/graph-extraction.ts`.
- Implemented extraction-time lifecycle cycle rejection in `src/functions/graph.ts`.
- Implemented `src/functions/graph-edge-reactions.ts` with conservative endpoint resolution, keyed target-memory locking, `KV.state` reaction groups, no `KV.graphEdges` full scan, cycle skip checks, and `relation_update` audit rows.
- Registered `event::graph::edge::reaction` as a `KV.graphEdges` state trigger in `src/triggers/events.ts`.
- Added targeted regression tests in `test/graph-edge-reactions.test.ts`, `test/events-boundary.test.ts`, and `test/graph.test.ts`.
- Verification performed:
- Initial red run failed as expected: missing `event::graph::edge::reaction`, lifecycle graph types dropped, reaction handler absent.
- `corepack pnpm install --frozen-lockfile --ignore-scripts` was required after pnpm ignored-build hardening blocked the first `pnpm exec`; an unwanted generated `allowBuilds` placeholder in `pnpm-workspace.yaml` was removed.
- `corepack pnpm exec vitest run --exclude test/integration.test.ts test/events-boundary.test.ts test/graph.test.ts test/graph-edge-reactions.test.ts` passed: 3 files, 60 tests.
- `corepack pnpm run lint` passed.
- `corepack pnpm test` passed on isolated rerun: 208 files, 2845 tests.
- `corepack pnpm run build` passed; emitted existing bundle/source-map/performance warnings only.
- `semgrep scan --config p/default --error --metrics=off .` passed with 0 findings.
- `gitleaks protect --staged --redact` passed with no leaks after staging the final intended index.

## Implementation Review

- Read-only adversarial review found two important issues:
- Graph-node endpoint resolution treated `properties.memoryId` as authoritative even when the graph node `name` also matched a different memory id.
- Final `extends` unapply could remove a `relatedIds` entry that another writer had independently preserved while the edge was active.
- Fixes applied:
- Graph-node endpoint resolution now collects existing-memory candidates from `properties.memoryId` and exact `name`; conflicting candidates skip with `ambiguous_source_memory` or `ambiguous_target_memory` and audit details.
- `extends` reaction groups now snapshot base and last-applied `relatedIds`; final unapply restores only when the current `relatedIds` and `updatedAt` still match the reaction-owned state.
- Regression tests added for conflicting endpoint candidates and externally preserved `relatedIds`.
- A full-suite run performed in parallel with lint failed two unrelated `test/codex-sdk-provider.test.ts` tests by hitting their 2s timeout; the same file passed in isolation, and the full suite passed when rerun without concurrent checks. This is recorded as a transient verification risk and should block remote automation without human checkpoint approval.

## Review Notes

- The implementation stores reaction bookkeeping in existing `KV.state`, not a new KV scope.
- No MCP tools, REST endpoints, dependency files, package metadata, auth behavior, migrations, or remote configuration were changed.
- Security gates completed: Semgrep passed with 0 findings; staged Gitleaks passed with no leaks.
Loading
Loading