fix(sandbox): acknowledge unchanged policy revisions

[NaveCohenMonday]

Summary

Decouple sandbox policy-revision acknowledgement from OPA policy loading. A
newer sandbox revision whose effective policy hash is already loaded can now be
acknowledged without reloading identical policy content, allowing desired and
current revisions to converge.

Related Issue

Fixes #2518

Changes

• Track the acknowledged sandbox policy version separately from the loaded
  policy hash.
• Acknowledge only newer, sandbox-scoped revisions with the same non-empty
  effective policy hash.
• Wait for required policy-runtime or middleware reconciliation to succeed
  before acknowledging the revision.
• Exclude global policy, local overrides, equal or older versions, empty
  hashes, and different policy content from the shortcut.
• Emit acknowledgement success telemetry only after the gateway accepts the
  loaded-status report.
• Document the same-hash revision acknowledgement invariant.

Testing

• CARGO_INCREMENTAL=0 mise run pre-commit passes
• Unit tests added/updated
• E2E tests added/updated (maintainer requested test:e2e after review)

Validation:

CARGO_INCREMENTAL=0 mise exec -- cargo test -p openshell-sandbox --lib

test result: ok. 99 passed; 0 failed

CARGO_INCREMENTAL=0 mise exec -- cargo test -p openshell-sandbox \
  same_hash_ --lib

test result: ok. 3 passed; 0 failed

The scripted poll-loop tests cover v1/H → v2/H exactly-once acknowledgement
without an OPA reload, failed middleware reconciliation with no premature
status followed by retry and one acknowledgement, and the local/global,
changed/empty-hash, equal, and older-version scopes.

Checklist

• Follows Conventional Commits
• Commits are signed off (DCO)
• Architecture docs updated

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[github-actions]

All contributors have signed the DCO ✍️ ✅
_{Posted by the DCO Assistant Lite bot.}

[NaveCohenMonday]

I have read the DCO document and I hereby sign the DCO.

[drew]

gator-agent

PR Review Status

Validation: This concentrated sandbox-policy fix directly addresses confirmed issue #2518 and has no duplicate candidate.
Head SHA: 182ef51768dfa66becb6847e0349170621cbddab

Review findings:

• Warning — missing stateful regression coverage: the added predicate tests do not exercise the poll loop, so they cannot catch duplicate or premature acknowledgements. Please add a scripted-poll test proving v1/H → v2/H emits exactly one LOADED(2) without reloading OPA, a failed required middleware reconciliation emits nothing and retries, and a later success emits exactly one acknowledgement. Include the relevant negative scopes (local override, global source, changed/empty hash, and equal/older version).
• See the inline warning about emitting success telemetry before gateway acknowledgement succeeds.

Docs: The architecture invariant is documented. No Fern docs/navigation update is required because this fixes existing --wait semantics without changing a command or user workflow.

Tests: No local tests were run. The author reports pre-commit and two focused predicate tests passing; required CI is still waiting for the copy-PR mirror.

Next state: gator:in-review pending an author update. A test:e2e pipeline run will be required after review feedback is resolved because this changes sandbox policy-enforcement reconciliation.

[NaveCohenMonday]

Addressed in c20b6b8c.

The poll loop now accepts a scripted gateway and middleware connector in tests,
while production continues to use the cached gRPC client and normal registry
connector. The new stateful coverage proves:

• v1/H → v2/H emits exactly one LOADED(2) and leaves the OPA generation
  unchanged;
• a failed required middleware reconciliation emits no LOADED(2);
• the identical next poll retries reconciliation, and a later success emits
  exactly one LOADED(2);
• local override, global source, empty hash, equal version, and older version
  do not use the shortcut;
• a changed hash follows the normal OPA reload and status path.

I also carried the acknowledgement event kind and policy hash in
PolicyStatusUpdate. The OCSF success event now emits from
run_policy_status_reporter only after report_policy_status returns
successfully.

Validation:

CARGO_INCREMENTAL=0 mise exec -- cargo test -p openshell-sandbox --lib
test result: ok. 99 passed; 0 failed

CARGO_INCREMENTAL=0 mise run pre-commit
Finished successfully

I additionally mutation-checked the two critical guards: allowing equal
versions caused the exactly-once test to fail, and allowing acknowledgement
before runtime reconciliation caused the failed-middleware test to fail.

[drew]

gator-agent

Re-check After Author Update

Thanks @NaveCohenMonday. I reviewed current head c20b6b8c365dd5821594673d5fe06bf4cbe2ae93 after your update explaining the new scripted poll-loop coverage and post-RPC acknowledgement telemetry.

Validation: This concentrated sandbox-policy fix directly addresses confirmed issue #2518.
Head SHA: c20b6b8c365dd5821594673d5fe06bf4cbe2ae93

What I checked: the full current-head diff, the same-hash candidate and reconciliation gates, status-reporter delivery path, stateful positive/negative poll tests, and the architecture invariant.

Disposition: the two prior gator findings are resolved. The poll-loop tests now cover same-hash exactly-once acknowledgement, middleware failure/retry, and the requested negative scopes; success OCSF telemetry now follows a successful gateway status RPC. The independent review found no blocking correctness or security issue. It left one non-blocking determinism warning and one optional telemetry-test suggestion inline.

Docs: architecture/sandbox.md accurately documents the invariant. No Fern docs/navigation update is required because this fixes existing semantics without adding a direct UX workflow.

Tests: Code-only review; no local tests were run. A test:e2e pipeline run is required because this changes sandbox policy-enforcement reconciliation.

Next state: gator:watch-pipeline.

[github-actions]

Label test:e2e applied, but pull-request/2557 does not exist yet. A maintainer needs to comment /ok to test c20b6b8c365dd5821594673d5fe06bf4cbe2ae93 to mirror this PR. Once the mirror exists, re-apply the label or re-run Branch E2E Checks from the Actions tab.

[drew]

/ok to test c20b6b8

[johntmyers]

@NaveCohenMonday you're hitting some flakey tests which have been turned off as of today. i'd rebase to see if we can get the pipeline green so we can prep to merge

[johntmyers]

gator-agent

Author Follow-Up Nudge

This PR has been in gator:in-review for more than 48 business hours waiting on the requested rebase after the flaky branch-check failure.

@NaveCohenMonday, please rebase onto current main and push the updated head so the branch pipeline can rerun. If this is no longer planned, please say so and a maintainer can close it out.

[NaveCohenMonday]

Rebased onto current main (4cb77a9) as requested and force-updated the existing fork branch.

New head: 25202b2

The two-commit range-diff is patch-identical to the previously reviewed head, and both commits retain the Nave Cohen DCO sign-off.

Validation on the rebased head:

• CARGO_INCREMENTAL=0 cargo test -p openshell-sandbox --lib: 107 passed, 0 failed
• CARGO_INCREMENTAL=0 mise run pre-commit: passed
• git diff --check: passed

This fork head now needs /ok to test 25202b2 so the copy-PR branch checks can rerun.

[johntmyers]

/ok to test 25202b2

[johntmyers]

gator-agent

Re-check After Author Update

Thanks @NaveCohenMonday. I checked current head 25202b2671f2acafae824e699f21f660210e179b after your August 9 update explaining the requested rebase and patch-equivalent range-diff.

Head SHA: 25202b2671f2acafae824e699f21f660210e179b
Base SHA: 4cb77a900ebd6b789d2b68daaba4830866833b1c
Merge base SHA: 4cb77a900ebd6b789d2b68daaba4830866833b1c
Patch ID: 76105582c3c712f7f4d52b37605c7b6515c2d860
Gator payload: 3

What I checked: the durable Gator feedback ledger, current patch identity, prior marked reviews and inline findings, and the current required-check state. The ledger confirms this rebase is patch-equivalent to the last reviewed head, so I did not re-review the unchanged effective patch. Drew's original findings and their later resolutions remain preserved.

Disposition: resolved. No blocking review findings remain.

Tests: test:e2e remains required because this changes sandbox policy-enforcement reconciliation. I posted /ok to test 25202b2671f2acafae824e699f21f660210e179b so the copy-PR pipeline can proceed.

Next state: gator:watch-pipeline.

[johntmyers]

gator-agent

Monitoring Complete

Monitoring is complete because this PR has merged.

Head SHA: 25202b2671f2acafae824e699f21f660210e179b
Gator payload: 3

Final status: Gator preserved Drew's original review and inline findings, their subsequent resolutions, and the patch-equivalent same-SHA disposition. Required branch, Helm, and E2E gates passed, maintainer approval was present, and the PR merged from gator:merge-ready.

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