Skip to content

fix(sandbox): order sidecar provider updates by generation - #2849

Merged
johntmyers merged 1 commit into
mainfrom
fix/2847-provider-env-generation/johntmyers
Aug 21, 2026
Merged

fix(sandbox): order sidecar provider updates by generation#2849
johntmyers merged 1 commit into
mainfrom
fix/2847-provider-env-generation/johntmyers

Conversation

@johntmyers

Copy link
Copy Markdown
Collaborator

🏗️ build-from-issue-agent

Summary

Fix process/network sidecar provider-environment convergence by separating delivery ordering from the opaque digest-derived revision. The network supervisor now assigns a connection-local monotonic generation to each distinct snapshot, and the process supervisor applies only newer generations.

Related Issue

Closes#2847

Changes

  • crates/openshell-sandbox/src/sidecar_control.rs: carry a provider-environment generation through bootstrap and updates; serialize generation assignment, state mutation, and publication; suppress only the current fingerprint.
  • crates/openshell-sandbox/src/lib.rs: order process-side installs by generation and retain the fingerprint as snapshot identity.
  • e2e/rust/tests/credential_gating.rs: verify a fresh exec transitions safely through endpointless provider unbound → bound → unbound states without sandbox recreation.
  • architecture/sandbox.md: document the fingerprint and delivery-generation invariants.

Deviations from Plan

None — implemented as planned. The final review strengthened the E2E placeholder assertion and replaced timing-based replay checks with FIFO barriers.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated

Tests added:

  • Unit:cargo test -p openshell-sandbox — 122 passed across library, binary, and integration targets; includes descending fingerprints, duplicate suppression, A → B → A, and stale-generation replay.
  • Integration: Sidecar Unix-socket bootstrap/update round trips are covered in sidecar_control.rs.
  • E2E:OPENSHELL_E2E_KUBE_TEST=credential_gating mise run e2e:kubernetes:sidecar — 1 passed; verifies unbound → bound placeholder → unbound convergence in the Kubernetes sidecar topology without exposing credential values.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)

Documentation updated:

  • architecture/sandbox.md: documented the sidecar provider environment ordering contract.

@johntmyers

johntmyers commented Aug 20, 2026

Copy link
Copy Markdown
CollaboratorAuthor

🏗️ build-from-issue-agent

E2E Test Attestation

Local E2E tests passed on the rebased PR commit. CI does not currently run this focused E2E test, so this comment serves as the verification record.

FieldValue
Commitac9b9a9151d90db06d5824451359ee58b2fecdb8
CommandOPENSHELL_E2E_KUBE_TEST=credential_gating mise run e2e:kubernetes:sidecar
Gateway modeHelm on Docker-backed k3d, Kubernetes process/network sidecars
Result✅ All passed

Test Summary

1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 336.02s

Tests Executed

  • credential_gating::credentialed_endpoint_gates_work_end_to_end — PASSED

The strengthened live-update assertion verifies unbound → bound OpenShell placeholder → unbound state across fresh execs without printing credential or placeholder values. The ephemeral k3d cluster was deleted by the test wrapper.

drew
drew previously approved these changes Aug 20, 2026

@johntmyersjohntmyers left a comment

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: Project-valid focused fix for accepted issue #2847; the author is a verified maintainer.
Head SHA: 9ccee5c8e87327268ea765aa9b8c0893c735dae3
Base SHA: b2ea81822bb5263679ed00ff0fbe8c08577abab5
Merge base SHA: b2ea81822bb5263679ed00ff0fbe8c08577abab5
Patch ID: a521e877e4a694966c9c656ea5eeefa48259db1e
Gator payload: 4
Review mode: initial
Previous reviewed SHA: none
Review budget exhausted: no
Maintainer decision required: no

Blocking findings:

  • No blocking findings remain

Carried findings:

  • None

Non-blocking suggestions:

  • None

Docs: Fern docs are not needed because this is an internal sidecar delivery-ordering correction with no direct user-facing UX or published API change; the architecture invariant is documented in architecture/sandbox.md.

Next state: gator:in-review pending required E2E dispatch.

@johntmyersjohntmyers added gator:in-review Gator is reviewing or awaiting PR review feedback test:e2e Requires end-to-end coverage labels Aug 20, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied for 9ccee5c. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@johntmyers
johntmyersforce-pushed the fix/2847-provider-env-generation/johntmyers branch from 9ccee5c to ac9b9a9CompareAugust 20, 2026 22:36
@github-actions

Copy link
Copy Markdown

@johntmyers
johntmyers requested a review from drewAugust 20, 2026 22:52

@johntmyersjohntmyers left a comment

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: Project-valid focused fix for accepted issue #2847; the author is a verified maintainer.
Head SHA: ac9b9a9151d90db06d5824451359ee58b2fecdb8
Base SHA: ef296806f52c03956a4bb7ce9a384105160f9c6a
Merge base SHA: ef296806f52c03956a4bb7ce9a384105160f9c6a
Patch ID: bbb00095b42ba24f8d736006e12335b88bf698d5
Gator payload: 4
Review mode: follow_up
Previous reviewed SHA: 9ccee5c8e87327268ea765aa9b8c0893c735dae3
Review budget exhausted: no
Maintainer decision required: no

Thanks @johntmyers. I checked your current-head E2E attestation for the unbound → bound → unbound fresh-exec path, and reviewed the author-only rebase delta against the new main-process session integration.

Blocking findings:

  • No blocking findings remain

Carried findings:

  • None

Docs: Fern docs are not needed because this is an internal sidecar delivery-ordering correction with no direct user-facing UX or published API change; the architecture invariant is documented in architecture/sandbox.md.

Tests: test:e2e is applied. Current-head Branch Checks completed successfully, and current-head E2E run 32425060215 is in progress.

Next state: gator:watch-pipeline

@johntmyersjohntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status gator:blocked Gator is blocked by process or repository gates and removed gator:in-review Gator is reviewing or awaiting PR review feedback gator:watch-pipeline Gator is monitoring PR CI/CD status labels Aug 20, 2026
Closes#2847
Separate sidecar delivery ordering from opaque provider environment fingerprints and cover live bind/unbind convergence.
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
@johntmyers
johntmyersforce-pushed the fix/2847-provider-env-generation/johntmyers branch from ac9b9a9 to 37b3ddaCompareAugust 20, 2026 23:36
@johntmyersjohntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status and removed gator:blocked Gator is blocked by process or repository gates labels Aug 20, 2026
@johntmyers
johntmyers added this pull request to the merge queueAug 20, 2026
Merged via the queue into main with commit 0300e6dAug 21, 2026
56 of 62 checks passed
@johntmyers
johntmyers deleted the fix/2847-provider-env-generation/johntmyers branch August 21, 2026 00:08
@johntmyers

Copy link
Copy Markdown
CollaboratorAuthor

gator-agent

Monitoring Complete

Head SHA: 37b3ddabf64918204af396b48a6b2313f8933f75
Gator payload: 4

Monitoring is complete because this PR has merged.

Final status: The PR merged after gator review reached gator:watch-pipeline, required E2E coverage was dispatched, and maintainer approval was present.

I removed the active gator:* label because there is nothing left for gator to monitor on this PR.

@johntmyersjohntmyers removed the gator:watch-pipeline Gator is monitoring PR CI/CD status label Aug 21, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2eRequires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(supervisor): accept distinct provider environment revisions in process sidecar

2 participants

@johntmyers@drew