Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Explicit suppression policies can be silently ignored by older selected runners without capability enforcement.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds verifier-scoped cache suppression while retaining conservative workspace preservation.
Changes:
- Adds typed Python and Node cache controls.
- Records suppression and workspace-disposition evidence.
- Adds focused regressions and supporting documentation.
File summaries
| File | Description |
|---|---|
apps/web/src/App.tsx |
Extends frontend verifier policy types. |
crates/crony-domain/src/lib.rs |
Defines typed cache-suppression policies. |
crates/crony-runner/src/main.rs |
Updates verifier test construction. |
crates/crony-runner/src/retained_provider_receipt_tests.rs |
Updates receipt test policies. |
crates/crony-runner/src/source_checkpoint.rs |
Updates checkpoint test policies. |
crates/crony-runner/src/verifier.rs |
Applies and records native cache controls. |
crates/crony-runner/src/workspace.rs |
Adds NUL-safe dirty-file accounting and regressions. |
crates/crony-server/src/planning.rs |
Updates planning and policy validation patterns. |
crates/crony-store/src/contract_revision.rs |
Supports the extended verifier variants. |
docs/ARCHITECTURE.md |
Documents behavior and compatibility limits. |
docs/EVALS.md |
Records the focused regression matrix. |
docs/evidence/2026-09-16-verifier-cache-suppression.md |
Provides implementation and acceptance evidence. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@dstkwll, thanks for the offer to coordinate. This is a compatibility and landing-order request for PR #305 / issue #297 and PR #307 / issue #79, not a confirmed defect report or a request for a broad rewrite. Rationale and integration checks#294 and #305 both change runner capability admission and current-epoch dispatch, including resume/recovery. #307 also changes shared planning/store surfaces. Please check that integration preserves all of the following:
Please confirm your preferred landing order and flag any minimal adjustment needed, with rationale. If no code adjustment is needed, a compatibility confirmation is useful. A sanitized description of the pinned toolchain and approved validation setup used for your passing gates would also help; no credentials or IT-policy workaround are requested. We will need fresh evidence on the exact integrated candidate. Separate PR test results do not prove the combination. Rajesh's requested review on #305 and Shyam's on #307 remain separate; this coordination does not replace owner review or outstanding acceptance gates. No merge, auto-merge or deployment is requested. |
|
@afateen, thanks for the clear coordination request and for separating compatibility work from the outstanding owner reviews and acceptance gates. I agree with the integration requirements you listed. Proposed order: #294 looks like the first candidate based on its completed local validation and passing hosted CI at There is a concrete integration seam: #294 changes the shared sender to a typed result, while #305 adds its own typed delivery path behind the boolean sender. I suggest reconciling these into one final admission path that preserves both capability checks, the current-epoch guard, and each caller's handling of failure. This is not a standalone defect claim against either PR. One behavior needs explicit agreement: #294 currently fails an unsupported cache-policy recovery before dispatch, whereas #305 retains dependency-incompatible recovery and lets unrelated compatible commands progress. For the initial experiment, I propose preserving those existing semantics: missing cache support rejects recovery; with cache support satisfied, missing dependency support retains it. Neither condition sends unsupported work. If you prefer a unified temporary-incompatibility policy, let's decide that separately rather than changing recovery semantics implicitly during conflict resolution. Bounded experiment, if that works for you: create a separate local integration branch from the exact agreed PR heads, make only the necessary integration adjustments, and test:
I will post the exact source hashes, minimal adjustments, commands/results and limitations, including failures. The temporary branch is an experiment, not a replacement PR or approval of either contribution. Your remaining native/SQL/browser acceptance and Rajesh's/Shyam's requested reviews remain separate. Sanitized setup: personal Apple Silicon Mac, fully local isolated worktrees outside cloud-backed storage, owned disposable PostgreSQL 17 fixtures and candidate server/runner binaries. Currently verified tools are Homebrew Rust 1.98.1, Node 22.23.1 and pnpm 11.19.0; the repository specifies Rust minimum 1.94, not an exact Rust pin. Tests use the committed lockfiles ( No merge, auto-merge, deployment or Factory intake is proposed. Please confirm the provisional order and recovery behavior, or suggest a small adjustment. |
|
@dstkwll, yes: your proposed #294-first order works as a provisional integration order, subject to its owner review and conflict resolution. #307 and #305 can follow independently according to their remaining gates. For the bounded experiment, preserve the existing recovery behavior you described:
Your proposed single final admission path should retain both capability checks, current-epoch fencing, caller-specific failure handling and queue progress beyond one blocked page. Preserve unknown files and ordinary research notes/probes; no deletion exception is requested. Retain #307's early rejection, no-side-effect and selector-fence regressions when including it. The current snapshot still shows #294 at This confirms the scope of your separate local experiment, not replacement PRs, combined acceptance, native-provider execution, Factory intake, publication, merge or deployment. Rajesh's/Shyam's reviews and the remaining #305 native/SQL/browser/adversarial gates stay separate. Thank you for the concrete proposal and sanitized setup details. |
There was a problem hiding this comment.
🟡 Changes recommended
Critical recovery, resume, and Windows-specific admission behavior lacks direct regression coverage.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 3
- Review effort level: Balanced
|
@afateen, thanks for confirming the scope. The local experiment is complete, with useful results and two concrete integration/test findings. Exact inputs and disposition
The integration retains one typed final sender, both available/global capability checks on the same runner, epoch fencing, cache rejection before preparation, dependency-incompatible retention, and the existing caller-specific outcomes. Verified results
Findings to carry forward
Remaining limitsThe inherited startup fixture passed its rejection checks but failed the production TLS-storage contact assertion on this Mac. A focused diagnostic still observed zero storage requests after five seconds and Copilot Astra/high reviewed the integration and pagination correction; I independently checked the findings and executed the tests. This experiment does not replace #305's remaining native/SQL/browser/adversarial gates, vendor-provider acceptance, or Rajesh's/Shyam's reviews. A live mixed recovery/approval queue test remains desirable before claiming that entire lifecycle. No merge, auto-merge, deployment, or automatic Factory intake was enabled. My recommendation remains the agreed provisional #294-first order, subject to owner review, with #307/#305 following their own readiness. The queue and driver findings above are concrete follow-ups for the later integration. |
There was a problem hiding this comment.
🟡 Changes recommended
The source-bound evidence contains stale fingerprints and contradicts the stated current-head revision.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 16/16 changed files
- Comments generated: 2
- Review effort level: Balanced
Summary
Python verification could leave bytecode caches that forced preservation of an otherwise clean worktree. Apply native cache controls only to verifier children: direct Python gets
-Band its environment setting; wrappers and Node compile caches have explicit typed opt-ins. Keep the conservative file-preservation gate and record requested controls separately from observed source state.Closes #140.
Scope and provenance
7eecd39e43720851512225844bf5d8158e8a944f.63006e57ae0808f125978037b6fe492bb7c53d7e(documentation-only provenance clarification). Tested implementation revision:debbc2138c27b7e7cc6e9c212719be3072421c15; code and workflow are unchanged in this documentation follow-up.Behavior
Explicit controls require the selected runner's available, global
verifier-cache-suppression-v1capability. The server enforces this during scheduling and current-epoch sends, including resumes and durable recovery. Unsupported launch consumes no attempt. Offline direct-plan creation and the exact legacy readiness response remain available for policies without explicit controls.Automatic suppression for omitted controls needs an updated runner. Environment-only wrappers can discard settings; explicit file writes remain possible. Requested suppression is not proof of zero caches or ownership. Unknown ignored files remain preserved. Node policy covers its native compile cache only.
Validation
-E/-I/-IE, Node cache disabling, clean linked-worktree removal and byte-preserving ignored-file retention.debbc2138c27b7e7cc6e9c212719be3072421c15(CI run). All six hosted checks also passed for the current documentation-only head63006e57ae0808f125978037b6fe492bb7c53d7e(CI run); all six local gates passed again.Feature evidence and reproduction. Earlier runtime evidence is source-bound to the revisions recorded there; the current-main reconciliation was checked with the six local gates.
Integration and limits
A separately agreed local-only experiment combines this PR with the published #305 and #307 heads. Its changes are not part of this PR; results are reported in the discussion. Full recovery/resume lifecycle permutations and native Windows acceptance are not claimed by the macOS evidence. Owner review and the other PRs' acceptance gates remain separate.
This PR remains draft. No merge, auto-merge, deployment or automatic Factory intake has been enabled.