implement create env#1646
Conversation
|
🔒 Automated review in progress — @StellaHuang95 is auto-reviewing this PR. |
Remove verbose narrative comments from the PEP 723 inline-script changes, keeping only must-have documentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39dcc6a3-0fbd-4f36-9d0f-68677de49c27
Apply the reasonable automated review fixes while preserving the PR5 happy-path scope. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88b2e063-b52a-4743-bf2b-9ee639ec7a40
| ); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Copilot generated:
📍 src/managers/builtin/inlineScriptEnvManager.ts:createOrReuseEnvironment (retainLock branch)
The Skeptic found a new (Low) bug in the retain path: lock is set to undefined before retainedLock.retain() is awaited, so if retain() rejects (e.g. fs.rename fails with EACCES/EPERM/EBUSY) the error is only logged and finally sees lock === undefined → no release. On disk the owner-* marker persists but retained was never written, so the intended fast-fail ELOCKRETAINED degrades to a 5-min ELOCKED, and a future retained-keyed cleanup won't detect the orphan. Fix: keep lock non-null until retain() resolves (release in finally on failure), or write the retained sentinel before unlinking the owner marker so a failed transition still leaves a detectable marker.
[verified]
| } | ||
| } | ||
| return build.environment; | ||
| } catch (error) { |
There was a problem hiding this comment.
Copilot generated:
📍 src/managers/builtin/inlineScriptEnvManager.ts:inspectExpectedCacheEnvironment
Architect (High, pre-PR13): the containment primitive was correctly extracted to shared resolveCacheEntryPath/isDescendantPath, but the ownership policy — inspectExpectedCacheEnvironment plus the INLINE_SCRIPT_MANAGER_ID check and delete-gating — still lives private on the manager. PR13's delete path will re-derive "is this entry safely mine" with data-loss stakes; extract this into a shared assertOwnedCacheEntry(environment, cacheRoot, envDir) before then so create-safety and delete-safety cannot diverge.
[verified]
1 participant
No description provided.