fix(installer): keep fallback installs stable-only#1217
Conversation
![ouroboros-agent[bot]](https://avatars.githubusercontent.com/in/3091302?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: APPROVE
Metadata
| Field | Value |
|### Recovery Notes
First recoverable review artifact generated from codex analysis log.
---|---|
| PR | #1217 |
| HEAD checked | e667015fe0cb14fd30788c472aa962e52b41ec16 |
| Request ID | req_1779649998_1-retry-1-retry-2-retry-3 |
| Review record | f2dcb24b-a2ed-4ea9-afb3-a8bca802073f |
What Improved
- Unable to inspect PR contents: every shell read failed before execution due sandbox namespace setup failure.
Issue Requirements
| Requirement | Status |
|---|---|
| No linked issue requirement captured | N/A |
Prior Findings Status
Prior concerns could not be checked because /tmp/pr_comments_1217.txt and /tmp/pr_review_comments_1217.txt could not be read.
Blockers
No in-scope blocking findings remained after policy filtering.
Follow-up Findings
| # | File:Line | Priority | Confidence | Suggestion |
|---|---|---|---|---|
| 1 | N/A | High | High | Re-run the review in an environment where local file reads under /tmp/pr_review_1217 and /tmp/pr_diff_1217.patch are available. |
Non-blocking Suggestions
None.
Test Coverage Notes
- Not reviewed; source and test files could not be read.
Design Notes
Unable to assess architecture or boundary impact without file access.
Design / Roadmap Gate
Affected contracts, state, persistence, replay, compatibility, and runtime boundaries could not be inspected.
Directional Notes
Memory was advisory only; no current-HEAD evidence could be gathered.
Test Coverage
- Not reviewed; source and test files could not be read.
Merge Recommendation
Do not merge based on this review run; it did not have access to the PR contents.
Policy Notes
- Omitted 1 finding(s) that referenced files outside the current PR changed-files scope.
Review-Metadata:
verdict: APPROVE
head_sha: e667015
request_id: req_1779649998_1-retry-1-retry-2-retry-3
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true
Reviewed by ouroboros-agent[bot] via Codex deep analysis
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: APPROVE
Metadata
| Field | Value |
|### Recovery Notes
First recoverable review artifact generated from codex analysis log.
---|---|
| PR | #1217 |
| HEAD checked | e667015fe0cb14fd30788c472aa962e52b41ec16 |
| Request ID | req_1779650696_2 |
| Review record | d85cfd2f-528d-4ea1-89c2-db5612d25ce8 |
What Improved
- Remote installer lookup failures now stay stable-only instead of allowing pre-releases by default.
- Local repository installs no longer pass pre-release flags unnecessarily.
Issue Requirements
| Requirement | Status |
|---|---|
| No linked issue / PR body requirements captured | N/A |
Prior Findings Status
No prior human or inline review concerns were present in the provided comment artifacts.
Blockers
No in-scope blocking findings remained after policy filtering.
Follow-up Findings
| # | File:Line | Priority | Confidence | Suggestion |
|---|---|---|---|---|
| 1 | tests/unit/scripts/test_install_runtime_selection.py:123 | Medium | High | Add a positive remote-install test where fake PyPI JSON returns a pre-release version and assert uv tool install receives --prerelease=allow. The current tests cover lookup failure and local installs, but not the new success branch at scripts/install.sh:56-58. |
Non-blocking Suggestions
| - | - | - | None. |
Test Coverage Notes
- Reviewed
tests/unit/scripts/test_install_runtime_selection.py. bash -n scripts/install.shpassed.python3 -m py_compile tests/unit/scripts/test_install_runtime_selection.pypassed.- Could not run pytest because
pytestis not installed in this snapshot environment. - Manually simulated remote stable and remote pre-release PyPI JSON paths; stable omitted
--prerelease=allow, pre-release included it.
Design Notes
The change stays scoped to installer package-selection policy and preserves the existing local-install and runtime-selection boundaries.
Design / Roadmap Gate
Affected boundary is scripts/install.sh remote installation behavior across uv, pipx, and pip. The new PRE_FLAG default is stable-only and only crosses into install commands when PyPI reports a pre-release; local installs remain isolated by IS_LOCAL. No persistence, replay, or compatibility failure was found.
Directional Notes
Maintainer memory emphasized thin boundaries and avoiding unnecessary state/contract churn. Review focus stayed on installer runtime behavior, package resolution flags, and test coverage for changed branches.
Test Coverage
- Reviewed
tests/unit/scripts/test_install_runtime_selection.py. bash -n scripts/install.shpassed.python3 -m py_compile tests/unit/scripts/test_install_runtime_selection.pypassed.- Could not run pytest because
pytestis not installed in this snapshot environment. - Manually simulated remote stable and remote pre-release PyPI JSON paths; stable omitted
--prerelease=allow, pre-release included it.
Merge Recommendation
Approve. No blocking runtime, API, or installer contract issue was found in the changed source. The only follow-up is adding coverage for the positive pre-release detection branch.
Review-Metadata:
verdict: APPROVE
head_sha: e667015
request_id: req_1779650696_2
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true
Reviewed by ouroboros-agent[bot] via Codex deep analysis
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: APPROVE
Metadata
| Field | Value |
|### Recovery Notes
First recoverable review artifact generated from codex analysis log.
---|---|
| PR | #1217 |
| HEAD checked | e667015fe0cb14fd30788c472aa962e52b41ec16 |
| Request ID | req_retry_exhausted_new_timeout_1779693693_1217 |
| Review record | f1083c95-e198-4aad-9f52-b7607c26de73 |
What Improved
- Remote installer lookup failures now default to stable-only installs instead of allowing prereleases by default.
- Installer tests now cover remote-script execution outside a source checkout.
Issue Requirements
| Requirement | Status |
|---|---|
| No linked issue requirement captured | N/A |
Prior Findings Status
No prior human or inline review comments were present in the provided artifacts.
Blockers
No in-scope blocking findings remained after policy filtering.
Follow-up Findings
| # | File:Line | Priority | Confidence | Suggestion |
|---|---|---|---|---|
| 1 | scripts/install.sh:50 | Low | Medium | The comment says this detects whether PyPI's latest published version is a prerelease, but the implementation reads /pypi/<name>/json info.version, which the project’s own scripts/version-check.py treats as stable-only unless all releases are scanned. Consider tightening the comment or scanning releases if latest-prerelease detection is intended. |
Non-blocking Suggestions
| 1 | scripts/install.sh:50 | Documentation | The prerelease-selection comment is broader than the code’s actual PyPI contract. |
Test Coverage Notes
- Reviewed
tests/unit/scripts/test_install_runtime_selection.py. - Could not run
pytest:pytestis not installed in the snapshot environment. - Manually simulated remote installer runs with fake
uv,ouroboros, andcurl: prerelease JSON adds--prerelease=allow; curl failure omits it.
Design Notes
The change is narrowly scoped to installer prerelease flag selection and test harness setup. Runtime and persistence boundaries are unaffected.
Design / Roadmap Gate
Affected boundary is the shell installer’s remote package-resolution path. The changed default now fails closed to stable-only on lookup/parsing failure, and the install command assembly still consistently applies PRE_FLAG across uv, pipx, and pip.
Directional Notes
Maintainer memory emphasized thin boundary changes and avoiding state-sync regressions; review focus stayed on installer contract boundaries and generated install commands. I also checked current PyPI JSON behavior at https://pypi.org/pypi/ouroboros-ai/json as context for the prerelease comment.
Test Coverage
- Reviewed
tests/unit/scripts/test_install_runtime_selection.py. - Could not run
pytest:pytestis not installed in the snapshot environment. - Manually simulated remote installer runs with fake
uv,ouroboros, andcurl: prerelease JSON adds--prerelease=allow; curl failure omits it.
Merge Recommendation
Approve. No blocking runtime, API, data-loss, or critical test-coverage issues were found in the changed installer path.
Review-Metadata:
verdict: APPROVE
head_sha: e667015
request_id: req_retry_exhausted_new_timeout_1779693693_1217
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true
Reviewed by ouroboros-agent[bot] via Codex deep analysis
1 participant
Summary
Changes
PRE_FLAGto disabled inscripts/install.sh.