Skip to content

[spark-compete] fix: clean up temp file on os.replace failure in persona pointer - #122

Open
ifeoluwaaj wants to merge 2 commits into
vibeforge1111:masterfrom
ifeoluwaaj:fix/sqlite-timeout
Open

[spark-compete] fix: clean up temp file on os.replace failure in persona pointer#122
ifeoluwaaj wants to merge 2 commits into
vibeforge1111:masterfrom
ifeoluwaaj:fix/sqlite-timeout

Conversation

@ifeoluwaaj

@ifeoluwaaj ifeoluwaaj commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

spark-compete Packet

{
  "schema": "spark-compete-hotfix-v1",
  "event": "hotfix.submitted",
  "submission_mode": "pull_request",
  "submission_target_url": "https://github.com/vibeforge1111/spark-character/pull/74",
  "team": {
    "name": "Sequence",
    "members": [
      "ifesn",
      "micc9ee",
      "londitshabalala",
      "ifeoluwaaj"
    ],
    "device_holder_github": "ifeoluwaaj",
    "device_holder_llm": "ifesn"
  },
  "target_repo": "vibeforge1111/spark-character",
  "issue": {
    "type": "bug",
    "severity": "MEDIUM",
    "category": "bug_fix",
    "description": "clean up temp file on os.replace failure in persona pointer update",
    "title": "Temp file left behind on os.replace failure in persona pointer update",
    "actual_behavior": "In persona.py set_latest_persona_version(), if os.replace() fails (cross-device rename, permission error, disk full), the staging .tmp file is left in the artifacts directory indefinitely. The temp_path.write_text() + os.replace() sequence has no cleanup on failure.",
    "expected_behavior": "The temp file should be cleaned up in all code paths, whether os.replace succeeds or fails, using a try/finally block.",
    "repro_steps": [
      "Trigger set_latest_persona_version() on a read-only filesystem or cross-device rename scenario",
      "os.replace(temp_path, pointer_path) fails with OSError",
      "temp_path (.tmp staging file) is left orphaned in artifacts directory"
    ],
    "affected_workflow": "Persona evolution loop and any code path that updates the latest persona pointer"
  },
  "evidence": {
    "safe_links_only": true,
    "before_after_proof": "BEFORE: os.replace failure left .tmp staging file orphaned in artifacts directory. AFTER: try/finally block ensures temp file cleanup via unlink(missing_ok=True) regardless of os.replace outcome.",
    "links": [
      "https://github.com/vibeforge1111/spark-character/pull/74"
    ],
    "forbidden": [
      "pdf",
      "zip",
      "exe",
      "unknown downloads",
      "shortened links",
      "archives",
      "binaries",
      "tokens",
      "browser cookies",
      "wallet material",
      "raw logs",
      "raw conversations",
      "raw memory",
      "raw patches",
      "private repo maps",
      "private scoring details"
    ],
    "type": "code_review",
    "files": []
  },
  "proposed_fix": {
    "approach": "Wrapped the temp file write and os.replace call in a try/finally block. The finally clause calls temp_path.unlink(missing_ok=True) to remove the staging file whether the replace succeeds or fails.",
    "files_expected": [
      "src/spark_character/persona.py (+3 lines: try/finally cleanup)"
    ],
    "tests_or_smoke": "PYTHONPATH=src python3 -m pytest tests/test_persona.py -v \u2014 9/9 tests pass. unlink(missing_ok=True) is a no-op if file was already replaced.",
    "summary": "Applied fix",
    "files_changed": []
  },
  "pr": {
    "branch": "fix/persona-tempfile-cleanup",
    "title_prefix": "[spark-compete]",
    "author_github": "ifeoluwaaj",
    "body_must_include": [
      "packet",
      "team",
      "pr_author",
      "repo",
      "actual_behavior",
      "expected_behavior",
      "repro_steps",
      "before_after_proof",
      "tests_or_smoke",
      "duplicate_notes",
      "risk_notes",
      "review_claim"
    ],
    "url": "https://github.com/vibeforge1111/spark-character/pull/74"
  },
  "review_claim": {
    "impact_claim": "medium",
    "evidence_types": [
      "passing_test",
      "code_review"
    ],
    "duplicate_notes": "No existing PR addresses temp file cleanup in set_latest_persona_version. The try/finally pattern is a standard Python idiom not yet applied here.",
    "risk_notes": "Minimal risk \u2014 only adds cleanup of a staging file. unlink(missing_ok=True) is a no-op if the file was already replaced successfully.",
    "review_state_requested": "pr_review",
    "duplicate": false,
    "security_safe": true,
    "public_safe": true
  }
}

Team: Sequence

Bug Summary

What: Temp file left behind on os.replace() failure in persona pointer update

Actual behavior: If os.replace() fails, the .tmp staging file is orphaned in the artifacts directory.

Expected behavior: Temp file should be cleaned up in all code paths via try/finally.

Repro steps:

  1. Trigger set_latest_persona_version() on a read-only filesystem
  2. os.replace() fails with OSError
  3. .tmp file left orphaned

Root Cause

# persona.py:111-112
temp_path = pointer_path.with_name(f".{pointer_path.name}.{os.getpid()}.tmp")
temp_path.write_text(f"{resolved}\n", encoding="utf-8")
os.replace(temp_path, pointer_path)  # ← If this fails, temp_path is orphaned

No cleanup on failure.

Fix

temp_path = pointer_path.with_name(f".{pointer_path.name}.{os.getpid()}.tmp")
try:
    temp_path.write_text(f"{resolved}\n", encoding="utf-8")
    os.replace(temp_path, pointer_path)
finally:
    temp_path.unlink(missing_ok=True)  # Always cleaned up

1 file changed, 3 insertions(+)

Files Changed

  • See diff above

Duplicate Notes

No known duplicate PRs.

Risk Notes

Minimal risk.

@ifeoluwaaj

Copy link
Copy Markdown
Contributor Author

Closing - duplicate of driasim PR #264 and Esc1200 PR #236 (sqlite3 timeout)

@ifeoluwaaj ifeoluwaaj closed this Jun 7, 2026
@ifeoluwaaj ifeoluwaaj reopened this Jun 7, 2026
@ifeoluwaaj ifeoluwaaj changed the title [spark-compete] fix(reliability): add timeout to sqlite3.connect in memory_grounded.py [spark-compete] fix(character): handle malformed JSON in voice_judge _load_corpus Jun 29, 2026
@ifeoluwaaj ifeoluwaaj changed the title [spark-compete] fix(character): handle malformed JSON in voice_judge _load_corpus [spark-compete] fix: clean up temp file on os.replace failure in persona pointer Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant