Skip to content

Memory system: four latent defects found while splitting sw-memory.sh #5969

Description

@github-actions

Found while writing unit coverage for the lib/memory-*.sh modules extracted in #5902. None of these were introduced by that refactor, and none were fixed there — it was a pure code move. Each is currently pinned by an assertion in the new suites, so fixing one means deliberately updating a test rather than silently changing behaviour.

1. memory_decay_old never decays real failure memory

memory_decay_old guards its jq filter on type == "array", but ensure_memory_dir seeds {"failures":[]} and memory_capture_failure writes into .failures[]. The document is an object, so the filter falls through to else . and nothing is decayed. Age-weighting has therefore never applied to repo failure memory.

Pinned by: object-shaped failures.json is left untouched (known gap) in sw-lib-memory-store-test.sh.

2. Every domain-expanded search returns []

_expand_domain_keywords appends its synonyms space-separated onto a single line. memory_ranked_search then iterates keywords with while IFS= read -r kw, so kw is the entire joined string and grep -qiF "$kw" never matches an entry. Any query containing auth, api, db, ui, test, deploy, error or perf scores 0 and returns [] — i.e. expansion makes search strictly worse than no expansion. A non-expanding query like migration matches correctly.

Pinned by: domain-expanded query returns no results (known gap) in sw-lib-memory-search-test.sh.

3. ab-results.jsonl is not JSONL

memory_ab_record_result appends jq -n output, which is pretty-printed, so each record spans many lines. cmd_memory_ab_report works around this by grepping for the group label rather than parsing lines. Anything that reads the file line-by-line gets fragments. Either pass -c to the writer or rename the file.

Pinned by: records are pretty-printed, not one per line (known quirk) in sw-lib-memory-ab-test.sh.

4. Concurrent memory writes are only partly locked (FM-2)

memory_capture_failure and memory_record_fix_outcome take an flock on ${failures_file}.lock, but the other writers — memory_update_metrics, memory_capture_decision, memory_capture_pattern — use bare tmp-file + mv with no lock. Two pipelines finishing at once on the same repo can interleave a read-modify-write and lose an update. With --worktree parallelism and fleet mode both writing into the same $MEMORY_ROOT/<repo_hash>, this is reachable in normal operation.

Separately: a flaky assertion in sw-daemon-test.sh

test_failure_classification_wired runs grep -A 50 ... | grep -q ... under set -o pipefail. grep -q exits on first match, the upstream grep takes SIGPIPE, and PIPESTATUS comes back 141 0, failing the test. Measured at roughly a 20-25% failure rate over 15 runs, on both this branch and a clean main checkout. This is the grep | grep -q variant of the pitfall already documented under "Common Pitfalls" in .claude/CLAUDE.md.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions