Repository navigation
chore(openshell): bump pinned release to 0.1.4 - #2440
Conversation
|
ClawSweeper reviews are opt-in for maintainers. If a review is required, finish the work and validation first, then comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Free's clanker here (an AI agent working on Free's behalf). Review of head Pins against upstream (all match)
Wire compatibility
Behavior changes that could touch OCE
Nits (non-blocking)
CI Required is green on |
freeqaz-openai
left a comment
There was a problem hiding this comment.
Current source-owner update: I reproduced the real merge conflicts against landed main 04514330 and tested a complete resolution. This is new integration work, not a request to repeat the existing upstream pin/wire review. I did not push your branch, opt an agent into Claw review, publish images, or approve an uncreated head.
The PR is one commit, e1871ffd, on 1383bef7. Across its 21 changes (22 old/new paths including the wire-fixture rename), 11 paths diverged on main; seven of those merge cleanly. Exactly eight conflict blocks in four files need resolution:
docs/flows/credential-source-lifecycle.md(3),docs/flows/openshell-sandbox-provisioning.md(3): keep main’s newer PAT/API-key isolation, prerequisites, summaries and history; change the live wire pointer to 0.1.4; keep the bump’s changelog entry in its date position. Don’t rewrite historical checkpoints.docs/testing/openshell.md(1): keep main’s table distinguishingCODEX_ACCESS_TOKENfor default Codex fromOPENAI_API_KEYfor native/custom endpoint; update live release, source and default chart version to 0.1.4.tests/helpers/openshell-kubernetes-real.mjs(1): the two adjacent defaults should beopenShellChartVersion = "0.1.4",thencodexSandbox = "workspace-write",. Don’t take an entire side and silently drop main’s sandbox selection.
I applied the complete resolution to the exact main beforeimages with whitespace errors fatal and checked every resulting Git-source postimage. On that actual integration node --test tests/integration/ci-openshell.test.mjs passed 9/9, zero skipped, including the 0.1.4 chart/digest contract, exposed-host loopback, immutable-image rejection and restricted RuntimeClass failure controls. This is CI-helper proof, not a new real PAT, OpenShell or native-image run. Let actual new-head required CI/review cover the integration; old image and turn receipts remain bound to their reported tuple. The earlier Claw comment is maintainer opt-in, not a verdict, and explicitly says not to request review automatically.
|
One concrete rebase note, because #2515 now edits the same OpenShell fixture. The 0.1.4 release bump does not require adopting #2515’s Codex permission change. If this release PR lands first, keep its current credential-specific behavior; don’t remove the sandbox argument as part of resolving this PR against main. I compared the prepared 0.1.4 rebase described in my earlier review with actual #2515 head This is landing-order guidance, not a request to bundle PRs or an assertion that native-image, account, or OpenShell containment qualification is complete. No new bot review or CI rerun is needed merely to record the conflict. Current rebase correction ( I reapplied the remaining release changes to the current beforeimages with whitespace errors fatal, verified each result, preserved the fixture's |
Signed-off-by: Russell Bryant <rbryant@redhat.com>
e1871ff to
117cc47
Compare
|
Free's clanker here (an AI agent working on Free's behalf). I re-read the rebased head The code delta is nil: pin One thing to know before landing: First Agent Smoke is red on this head for a main-side reason. The run merged Optional: |
There was a problem hiding this comment.
Please update the dev-install assertion to expect 0.1.4. This is an actual failure in the OpenShell advisory for this PR head, separate from the previously reported First Agent Smoke issue. I checked the failure artifact, the chart-packaging command, and the assertion itself.
At 117cc479, the launcher selects 0.1.4 and packages the chart with that version. The real dev-install test still requires 0.1.3-pre.2. Its failure is recorded at that assertion; the retained summary doesn't show the actual Helm list value, so I don’t infer an observed chart version from the failure alone.
- /-0\.1\.3-pre\.2$/,
+ /-0\.1\.4$/,I exercised that exact assertion with the version the launcher packages: old assertion rejects openshell-0.1.4; corrected assertion accepts it and still rejects 0.1.3-pre.2, 0.1.40, and 0.1.4-rc.1. I also prepared a merge with main at e8df0245 and ran the wire + CI-helper suites on that candidate: 36 passed, 0 failed, 0 skipped. Keep main’s current Codex permission selection when resolving the helper conflict; do not restore a removed sandbox default to make the merge work. This supersedes my older rebase advice because main changed.
A second failure is still unresolved. The same hosted artifact records that the authenticated in-sandbox model turn passed, then the test moved on to actual filesystem and network enforcement and failed. The chart assertion cannot explain or clear that second failure, and reaching the boundary check is not proof that enforcement passed. Please run the real advisory and First Agent Smoke on the new head and retain the specific failing boundary/check if still red. I did not rerun the cluster, publish images, request another Claw review, or treat an advisory as a required-check policy change.
Update: I traced the second failure. Preserve the already-merged #2515 behavior when updating this branch; please do not grant an approval or lengthen the timeout to get a pass.
- In the retained diagnostic, the nonce turn passed. The subsequent tool-boundary turn used the old
workspace-write/on-requestconfiguration and hit the helper’s five-minute timeout. Its value-free summary counted six command completions with nonzero exits, one approval request, and the labelsbubblewrapandnamespace. Those counts do not tell us which commands failed, whether that request caused the timeout, or whether an OpenShell boundary allowed or denied anything. - I exercised the actual client script emitted by each fixture, substituting only a local scripted protocol peer and inert timer. At this PR head, a server-initiated command approval request received no response. With current main’s #2515 behavior in the proposed merge, it received decline. A separate control in which the account did not select the required outer-sandbox execution mode stopped before sending a turn. No model, credential, cluster or live command ran for this protocol check; it does not qualify isolation.
- The hosted #2515 advisory actually passed both of these test files, on its own source/pin. The discriminating next run is therefore 0.1.4 + the corrected exact chart assertion + current main’s permission/decline path. We still need the actual allowed and denied filesystem/network assertions on that tuple. Neither the old green run nor the first successful model response supplies them.
Update: I pushed the source correction in 6ec3a73e45. GitHub now reports this PR can merge cleanly with main at b580f8eba; that is a conflict result, not approval to land. I’m a contributor to this head, so please use a fresh independent reviewer.
- Reconciled the two actual conflicts: the fixture keeps main’s explicit Codex permission selection and declines an unexpected approval request; the flow keeps both histories. Compared with Git’s automatic merge, the only other difference is the dev-install assertion above. It now accepts the version this PR packages and still rejects
0.1.3-pre.2,0.1.40, and0.1.4-rc.1. - On that exact source: 36/36 wire and CI-helper tests; 5/5 selected Go launcher tests; all five changed docs below the repository’s 2,500-word limit. The wire tests required normal loopback access; the initial network-sandbox failure is not a product failure or a skipped test.
- Fresh PR CI and fresh QA, including the OpenShell evidence we need. Please check both the installed chart and the actual filesystem/network enforcement stage on this head. Today’s green OpenShell baseline was on main’s older pin; it cannot qualify 0.1.4, clear the second failure, or replace head-bound image qualification. I didn’t rerun the old failure, request another Claw review, publish images, or deploy anything.
|
Free's clanker here (an AI agent working on Free's behalf). Heads-up: #2515 landed on main (26d00dd..0fbd964), and #2555 followed it. This branch now conflicts with main in two files:
The red First Agent Smoke on 117cc47 came from the #2540 per-caller read budget, which #2545 reverted. Merging current main should clear that red as well. As freeqaz-openai noted above, this release bump doesn't need to adopt #2515's Codex permission change. Keep main's side of the fixture and docs and re-apply the 0.1.4 pin. I haven't pushed anything to this branch. |
|
Free's clanker here (an AI agent working on Free's behalf). Re-checked the new head
Nothing else is left from my side. A maintainer still needs to re-review, since the changes-requested review is still open. |
My two requests on 117cc47 are satisfied at 6ec3a73. The dev-install assertion now requires the pinned 0.1.4 chart, and the new hosted OpenShell run passed 9/9, including the actual filesystem/network enforcement path. Both native architectures also passed. This dismisses only my obsolete changes request; I contributed the correction, so this is not approval of my own work. Please have another eligible reviewer assess the current head. No image was published.
6ec3a73 to
5357fbf
Compare
|
Maintainer handoff for
|
mrunalp
left a comment
There was a problem hiding this comment.
Approving. I independently verified the pins against upstream:
v0.1.4→5a7728c3.- The source-archive SHA-256
e7b80d63…matches. - The gateway, sandbox and supervisor image-index digests match ghcr.
- The wire changes the fixture models from
021400beto5a7728c3are additive only (GetProviderCredentials;NetworkEndpoint.token_grant_owner,Sandbox.host_key_fingerprint,ProviderProfileCredential.token_grant_owners/stable_placeholder).
QA advisory (OpenShell), Required CI and First Agent Smoke pass on 5357fbf32. #2629 (pre.4) is stacked on this.
Update the pinned OpenShell release from
v0.1.3-pre.2to stablev0.1.4. This updates the source checksum, immutable gateway/sandbox/supervisor image digests, Helm and CI defaults, and wire-fixture provenance. It does not add the OAuth feature from #1559 or change what the Driver admits.The dev-install test now requires the chart we actually package,
openshell-0.1.4. On the previous head it still required0.1.3-pre.2. The merge frommainpreserves the current Codex permission behavior, including declining an unexpected approval request; it does not relax the sandbox test to make the version bump pass.Verification on
5357fbf323Rebased onto
origin/mainatc655fd7f4, preserving the prior merge resolutions and chart assertion fix. Fresh local checks passed: 36/36 wire and CI-helper tests with no skips, focused OpenShell Go launcher tests, lint, formatting, workspace isolation, documentation/link checks (257 pages and 4,103 links), documentation length, and the OpenShell provisioning flow validator. Checks used existing tools directly after pnpm's automatic dependency reconciliation aborted; no dependencies were installed.Hosted verification for this revision is complete. Required CI and First Agent Smoke passed.
00a0c1e79has exactly this head’s tree,023815aba. This is evidence for that source and run, not for changes subsequently landed on main or a claim that intermittent failures are impossible.The required Maintainers (OCE) code-owner review is requested. Please review this head and use normal branch protections; no duplicate qualification or deployment is requested.
Previous verification on
6ec3a73e45bd9d4ac03, whose tree equals this PR head. This resolves the two concrete verification gaps from the previous failed run; it does not establish why that enforcement run failed or promise it can never recur.0.1.4and rejects0.1.3-pre.2,0.1.40, and0.1.4-rc.1. All five edited docs are within the word limit. These were not live-cluster tests.The earlier native qualification and the author’s separate ARM64 development walkthrough were on
e1871ffd. The author reported two nonce-verified turns through the deployed Gateway and dedicated Codex Harness; they verified the model turns through the revision API because the first-Agent helper omitted an admission default. The separate Compose and Kubernetes Calendar advisories used Sandboxnone, so they do not qualify OpenShell either way.Please review this head independently and use the linked runs for final results. This PR does not publish images, change
latest, or authorize a deployment.