Skip to content

Bound generated storage and guard historical checkout startup - #35

Merged
DatMoshu merged 6 commits into
mainfrom
work/589-storage-guards
Nov 8, 2026
Merged

DatMoshu merged 6 commits into
mainfrom
work/589-storage-guards

Conversation

@DatMoshu

Copy link
Copy Markdown
Owner

Content jobs and checkout setup could grow indefinitely or fall back to generating files inside a checkout. Add configurable external-store limits and finished-job retention, while preserving inputs, editable scenes, active/queued jobs and protected files. Producers reserve large copies/packages and stop monitored child processes at the byte limit. Setup uses external environments, canonical inputs and Godot storage; launchers validate physical paths and the exact editable install.

Rebuild pin creation and the full retention selection/deletion pass share an exclusive store lock. The pin body releases the lock so create/render/merge sessions can proceed safely. Deterministic barriers cover both orderings; the previous implementation fails the late-pin controls.

This PR is stacked on #34: base work/SM-GLB5-director at 1ffc72e. Exact reviewed head: 1b611ca. It keeps #34's approved scope unchanged. Integrate #34 first, then retarget this PR to main for owner approval; no agent merge or direct trunk push.

Validation at the exact head:

  • Full gates: 247 pytest passed, 1 skipped, 3 subtests; 15 outfit-lab tests passed; agent routers current.
  • Focused storage/rebuild/client-staging suite: 35 passed, including deterministic concurrency tests and previous-source negative controls.
  • Real headless Blender job completed create/render/finish under an acquired pin, passed VD alpha/anchor roundtrip, preserved source derivatives until pin release, and removed only fresh temporary smoke output afterward.
  • Windows launcher accepts the matching external environment and refuses unguarded historical checkout, repository-local output and wrong editable install.
  • Chief exact-head source review passed. Runs: 20261025_spritemotion-lead_storage-guards and 20261025_spritemotion-lead_storage-pin-lock.

Limits and configuration:

  • Defaults: 5 decimal GB per store; expire completed/failed derivatives only when older than 7 days AND outside the newest 10 finished jobs. Environment/launcher settings are documented in tools/storage/README.md.
  • This is a producer guard, not an OS or fleet-wide quota. Independent stores have independent caps; a child may overshoot during one polling/write interval. Preserved inputs/scenes can fill a store, which then refuses new work rather than deleting authoring data.
  • Historical checkouts require guard invocation and update/rebase before resume. Untouched historical scripts are not retroactively patched; legacy one-off exporters still require explicit external destinations and closure cleanup. No unattended timer is claimed.
  • Main and other checkouts gain these guards only after integration/update. No game/client assets or inherited machine settings are included. Owner approval authorizes this publication; GitHub main approval/automerge remains separate.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-25T05:28:04.159249Z 1b611ca PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1b611ca8aa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread common/storage.py Outdated
Comment on lines +243 to +245
exe = Path(python).resolve()
paths.external_path(exe, checkout)
result = subprocess.run([str(exe), '-c', 'import spritemotion; print(spritemotion.__file__)'],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve the virtualenv interpreter path when validating imports

On Linux/macOS, the environment created by python -m venv normally uses a symlink for bin/python. Resolving that symlink here and executing its target invokes the base interpreter without the virtualenv's site-packages, so startup rejects a correctly installed checkout environment (or checks an unrelated base installation). This blocks the updated shell launchers and direct pipeline/studio startup after normal setup. Validate the physical path separately, but run the import check through the original virtualenv interpreter path.

Useful? React with 👍 / 👎.

Comment thread common/storage.py Outdated
Comment on lines +179 to +180
status.update(state='failed', error='Generated derivatives expired; retained inputs/settings/scene can rebuild.')
p.write_text(json.dumps(status, indent=2), encoding='utf-8')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve completion ordering when marking derivatives expired

When eligible jobs expire, rewriting status.json gives them a fresh mtime, but subsequent retention passes use that same mtime to determine the newest finished jobs. Expired jobs therefore displace the jobs whose outputs were supposed to be retained. With keep=1 and two jobs older than the age threshold, the first prune removes the older job's VD and the immediately following prune removes the newer job's VD, leaving neither retained. Preserve the original completion timestamp or otherwise prevent cleanup updates from changing retention ordering.

Useful? React with 👍 / 👎.

Comment thread tools/uo-content/client_import.py Outdated
Comment on lines +68 to +69
with staging_store(out).session() as store:
return _stage(vd,client,body,out,store)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Protect the selected VD source during staging retention

When staging a finished job that is older than the retention threshold and outside the newest-job allowance, entering this session prunes the source job before _stage reads its VD. The Studio's animation-only /api/stage path passes job/item.vd and job/staged-client here, so the requested import deletes its own source and then raises FileNotFoundError. Protect the source job during the session, as the equipment-staging path already does.

Useful? React with 👍 / 👎.

Base automatically changed from work/SM-GLB5-director to main November 8, 2026 16:15
DatMoshu and others added 2 commits November 8, 2026 10:30
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- startup() runs the interpreter path as given; following a venv's
  bin/python symlink started the base interpreter without the venv's
  site-packages. The physical file and the environment are both checked.
- Retention ranks jobs by when they finished. Expiry records completed_at
  because its status.json rewrite refreshes the file's mtime, which let
  expired jobs displace the jobs meant to be kept.
- Staging a VD protects its source job from the session's retention.
- usage() tolerates files a running producer removes mid-walk, which
  aborted worktree-venv creation.
- The Blender import check supplies the spec global import_asset now reads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@DatMoshu
DatMoshu merged commit 6536fd9 into main Nov 8, 2026
4 checks passed
@DatMoshu
DatMoshu deleted the work/589-storage-guards branch November 8, 2026 16:35
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