fix(python_repl): create global REPL state lazily

[yonib05]

Description

python_repl instantiated its global ReplState at module import time:

repl_state = ReplState()

ReplState.__init__ creates the persistence directory and loads the persisted
state file (repl_state.pkl). Doing this at import means simply importing
python_repl has filesystem side effects, before the tool is ever invoked. The
state load also previously ran early inside python_repl() — before the user
was prompted to confirm execution.

This change defers state creation to first use through a get_repl_state()
accessor, makes that accessor thread-safe, and moves the state acquisition to
after the execution consent prompt so an unapproved run never loads the state.
A module-level __getattr__ keeps python_repl.repl_state working for any
existing callers that reference the attribute directly.

Changes

• Replace the import-time repl_state = ReplState() with a lazily-created
  singleton via get_repl_state().
• Add __getattr__ so python_repl.repl_state still resolves to the singleton.
• Guard the lazy initialization with a module-level threading.Lock
  (double-checked) so concurrent first use cannot create two instances.
• Defer get_repl_state() (and the reset_state handling) in python_repl()
  until after the consent prompt, so declining consent does not trigger the
  state-file load or persistence-directory creation. The code preview still
  renders before the prompt without touching state.
• Route internal uses through get_repl_state().

Testing

• TestLazyState asserts the module instantiates no state on import and that
  get_repl_state() returns a reused singleton.
• test_state_not_loaded_before_consent asserts a declined run never calls
  get_repl_state().
• pytest tests/test_python_repl.py passes (38 passed, 1 skipped).
• ruff format --check and ruff check pass on the changed files.

Notes

Switching persistence away from dill (to a non-executable format) is a
separate, larger effort and is intentionally out of scope here; this PR removes
the import-time and pre-consent load paths.

[yonib05]

/strands-ts review

[github-actions]

Review — fix(python_repl): create global REPL state lazily

I reviewed the change and validated it locally. LGTM — this is a clean, well-scoped fix. ✅

Validation

• pytest tests/test_python_repl.py → 37 passed, 1 skipped
• ruff format --check → 2 files already formatted
• ruff check → All checks passed

Correctness

• The PEP 562 module-level __getattr__ correctly preserves existing python_repl.repl_state attribute access. Notably the existing temp_repl_state_dir fixture, which mutates python_repl.repl_state.persistence_dir, continues to work because the read resolves to the lazily-created singleton.
• All internal usages are routed through get_repl_state() (or the locally captured repl_state in python_repl()), including the forked child path in _PtyManager.start — this is safe because the parent instantiates the singleton in python_repl() before the fork, so the child inherits it.
• Any and Optional are already imported, so no new imports were needed.
• The added TestLazyState cases correctly assert (a) no state instance exists immediately after import and (b) get_repl_state() returns a reused singleton.

Minor / optional (non-blocking)

• get_repl_state() uses an unguarded check-then-set (if _repl_state is None: _repl_state = ReplState()), so it isn't thread-safe. The previous import-time initialization was implicitly protected by the import lock. In practice concurrent first-use is unlikely for this tool, so I don't consider this blocking — but if you want to be defensive, a threading.Lock around the initialization would close the race. Happy either way.

No required changes. Nice fix for removing import-time filesystem side effects.

[yonib05]

/strands-ts review

[github-actions]

Re-review (round 2) — HEAD b02859f

Re-reviewed after the update (fix(python_repl): defer REPL state load until after the consent prompt). LGTM ✅ — the change is even stronger than the previous revision.

Validation

• pytest tests/test_python_repl.py → 38 passed, 1 skipped
• ruff format --check → 2 files already formatted
• ruff check → All checks passed

What changed since my last review, and my assessment

1. Thread-safety added — get_repl_state() now uses a module-level threading.Lock with a double-checked if _repl_state is None guard. This fully addresses my earlier (optional) note about the unguarded check-then-set; the pattern is correct and only locks on first initialization.
2. State load deferred past the consent gate — repl_state = get_repl_state() moved to after the y/* confirmation prompt, and the reset_state handling moved with it. This is a genuinely nice improvement: declining consent (or an unapproved run) no longer triggers the persisted-state file load or directory creation. The code-preview panel still renders before the prompt without touching state, which is correct.
3. New test test_state_not_loaded_before_consent patches get_repl_state and asserts it is not called when the user declines — good coverage for the new guarantee.

Minor / optional (non-blocking)

• The PR description is now stale: it still only describes the lazy-init/__getattr__ change and doesn't mention the thread-safety lock or the consent-deferral behavior (which is actually the headline of the latest commit). Worth updating the description so reviewers/maintainers see the current scope.

No required code changes from me. Solid work.

[yonib05]

/strands-ts review

[github-actions]

Re-review (round 3) — HEAD b02859f (unchanged)

Re-ran the review on request. The PR HEAD is identical to what I reviewed in round 2 — no new commits since fix(python_repl): defer REPL state load until after the consent prompt. My prior assessment stands: LGTM ✅

Validation (re-run against current HEAD)

• pytest tests/test_python_repl.py → 38 passed, 1 skipped
• ruff format --check → 2 files already formatted
• ruff check → All checks passed

No required code changes. The only outstanding (optional, non-blocking) item remains updating the PR description, which still describes only the original lazy-init change and omits the thread-safety lock and consent-deferral behavior added in the latest commit.