Port/rust#1846
Conversation
Phase 1 (tests pass): one language-agnostic behavior spec per tests/*.py, mirrored under specs/tests/, every behavioral claim cited as (relpath:Lstart-Lend) back to the original Python. - 120/120 test files covered (incl. fixtures/benchmarks/__init__) - zero specs with zero citations - all 3923 citations validated within source line bounds Adds scripts/verify-specs.sh (coverage + zero-citation audit per runbook 1.4). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 1 (source pass): one language-agnostic behavior spec per mempalace/*.py, mirrored under specs/src/, every behavioral claim cited as (relpath:Lstart-Lend) back to the original Python. - 66/66 source modules covered (incl. backends/, sources/, i18n/ packages) - zero specs with zero citations - all 2547 citations validated within source line bounds Completes Phase 1 spec extraction (tests pass landed in prior commit). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
I apologise for the mistake. Was intended for my forked repo. Please delete this PR... |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive suite of behavior specifications in Markdown format for the MemPalace package modules and tests, along with a verification script scripts/verify-specs.sh to track spec coverage and citations. Feedback on the verification script highlights a security vulnerability regarding insecure temporary file creation in /tmp (CWE-377) and a robustness issue with filename handling, suggesting the use of mktemp, a cleanup trap, and null-delimited file finding.
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.
| find specs -name '*.md' | while read -r f; do | ||
| n="$(grep -coE '\([^)]+:L[0-9]+' "$f" || true)" | ||
| printf '%4s %s\n' "${n:-0}" "$f" | ||
| done | sort -n | tee /tmp/spec-citation-report.txt | ||
| echo "--- coverage ---" | ||
| echo "source modules: $(git ls-files 'mempalace/*.py' | wc -l) source specs: $(find specs/src -name '*.md' 2>/dev/null | wc -l)" | ||
| echo "test files: $(git ls-files 'tests/*.py' | wc -l) test specs: $(find specs/tests -name '*.md' 2>/dev/null | wc -l)" | ||
| echo "--- specs with ZERO citations (must be empty) ---" | ||
| awk '$1==0{print $2}' /tmp/spec-citation-report.txt |
There was a problem hiding this comment.
Issues:
- Insecure Temporary File Creation (CWE-377): Writing directly to a hardcoded path in
/tmp(/tmp/spec-citation-report.txt) can lead to symlink attacks or conflicts if multiple users or concurrent processes run the script. Usingmktempis much safer. - Robust Filename Handling: Using
findpiped towhile readwithout-print0andIFS=can fail or misbehave if filenames contain spaces, newlines, or backslashes.
Suggestion:
Use mktemp to securely create a temporary file, set up a trap to clean it up on exit, and use find -print0 with read -d '' to robustly handle any special characters in filenames.
| find specs -name '*.md' | while read -r f; do | |
| n="$(grep -coE '\([^)]+:L[0-9]+' "$f" || true)" | |
| printf '%4s %s\n' "${n:-0}" "$f" | |
| done | sort -n | tee /tmp/spec-citation-report.txt | |
| echo "--- coverage ---" | |
| echo "source modules: $(git ls-files 'mempalace/*.py' | wc -l) source specs: $(find specs/src -name '*.md' 2>/dev/null | wc -l)" | |
| echo "test files: $(git ls-files 'tests/*.py' | wc -l) test specs: $(find specs/tests -name '*.md' 2>/dev/null | wc -l)" | |
| echo "--- specs with ZERO citations (must be empty) ---" | |
| awk '$1==0{print $2}' /tmp/spec-citation-report.txt | |
| report_file=$(mktemp) | |
| trap 'rm -f "$report_file"' EXIT | |
| find specs -name '*.md' -print0 | while IFS= read -r -d '' f; do | |
| n="$(grep -coE '\([^)]+:L[0-9]+' "$f" || true)" | |
| printf '%4s %s\n' "${n:-0}" "$f" | |
| done | sort -n | tee "$report_file" | |
| echo "--- coverage ---" | |
| echo "source modules: $(git ls-files 'mempalace/*.py' | wc -l) source specs: $(find specs/src -name '*.md' 2>/dev/null | wc -l)" | |
| echo "test files: $(git ls-files 'tests/*.py' | wc -l) test specs: $(find specs/tests -name '*.md' 2>/dev/null | wc -l)" | |
| echo "--- specs with ZERO citations (must be empty) ---" | |
| awk '$1==0{print $2}' "$report_file" |
What does this PR do?
How to test
Checklist
python -m pytest tests/ -v)ruff check .)