Recall v2.2: trustworthy memory lifecycle, provenance, and updater#12
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces version 2.2.0 of Recall, adding robust provenance, lifecycle controls, and supersession workflows for trustworthy agent memory. It introduces three new MCP tools (mark_memory_status, verify_memory, and supersede_memory), updates retrieval and consolidation logic to respect memory statuses, and adds an optional conservative updater script (recall-update.sh). Feedback on these changes highlights a security vulnerability in the updater script regarding insecure temporary file paths in /tmp, a bug where optional health checks can crash the script during rollback, and a validation gap in src/tools.ts allowing a source end line to be specified without a start line.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| INSTALL_CRON="" | ||
| YES=0 | ||
|
|
||
| BACKUP_DIR=".recall-update" |
There was a problem hiding this comment.
The script uses hardcoded temporary file paths in /tmp (e.g., /tmp/recall-update-d1.err, /tmp/recall-update-health.json, /tmp/recall-update-tools.json). In shared environments, this can lead to symlink attacks, unauthorized access, or denial of service if another user creates these files first. It is highly recommended to create a secure, unique temporary directory using mktemp -d at the start of the script, register an EXIT trap to clean it up, and place all temporary files inside it.
| BACKUP_DIR=".recall-update" | |
| BACKUP_DIR=".recall-update" | |
| SECURE_TMP_DIR="$(mktemp -d -t recall-update.XXXXXX)" | |
| trap 'rm -rf "$SECURE_TMP_DIR"' EXIT |
| err "worker URL not detectable; set RECALL_WORKER_URL=https://your-worker.example.com so --apply can run required smoke tests" | ||
| return 1 | ||
| fi | ||
| run curl -fsS --max-time 10 "${url%/}/health" >/dev/null |
There was a problem hiding this comment.
When mode is set to "optional" (such as during a rollback), any failure in the curl health check will cause the script to crash and exit immediately due to set -e. Optional smoke tests should fail gracefully with a warning rather than aborting the entire process.
| run curl -fsS --max-time 10 "${url%/}/health" >/dev/null | |
| if [[ "$mode" == "optional" ]]; then | |
| run curl -fsS --max-time 10 "${url%/}/health" >/dev/null || { warn "optional health check failed"; return 0; } | |
| else | |
| run curl -fsS --max-time 10 "${url%/}/health" >/dev/null | |
| fi |
| if (source_line_start !== null && source_line_end !== null && source_line_start > source_line_end) { | ||
| throw new Error('source_line_start must be less than or equal to source_line_end'); | ||
| } |
There was a problem hiding this comment.
In validateOptionalLineRange, the script checks that source_line_start is less than or equal to source_line_end if both are specified. However, it does not prevent a logical inconsistency where source_line_end is specified but source_line_start is omitted (null). If an end line is provided, a start line must also be specified.
if (source_line_start === null && source_line_end !== null) {
throw new Error('source_line_start must be provided if source_line_end is specified');
}
if (source_line_start !== null && source_line_end !== null && source_line_start > source_line_end) {
throw new Error('source_line_start must be less than or equal to source_line_end');
}|
Follow-up from pre-PR async review: Fixed both blockers in
Verification after fix:
|
|
Addressed Gemini Code Assist review in
Verification:
|
Summary
Ships Recall v2.2 as “trustworthy memory” rather than generic RAG:
store_memorymark_memory_status,verify_memory,supersede_memoryrecall-update.sh) with migration tracking, protected config handling, smoke tests, rollback, and cron check supportReview notes
wrangler.toml,.dev.vars,.recall-api-key, MCP configs, and.recall-update/are protected/ignored.--applyrequires migration tracking and smoke tests; older installs can bootstrapschema_migrationsand record already-applied historical migrations based on live schema.verify_memorystatus preservation, secure updater temp files, optional rollback smoke behavior, and provenance line-range validation.Verification
npm run typechecknpm test— 6 files / 45 testsbash -n recall-update.sh./recall-update.sh --install-cron weekly— prints only, no install without--yesRECALL_WORKER_URL=https://recall-hermes.cashcon57.workers.dev ./recall-update.sh --doctorgit diff --check origin/main..HEADorigin/main..HEADtools/list: 10 toolstools/listreturned 10 tools and lifecycle/provenance store/retrieve/verify/delete smoke passed