Skip to content

fix(repair): prevent avoidable pnpm setup timeouts - #1289

Merged
steipete merged 1 commit into
mainfrom
steipete/validation-identity-reuse-f87932
Aug 28, 2026
Merged

fix(repair): prevent avoidable pnpm setup timeouts#1289
steipete merged 1 commit into
mainfrom
steipete/validation-identity-reuse-f87932

Conversation

@steipete

Copy link
Copy Markdown
Contributor
Additional instructions

MUST: Keep Allow edits from maintainers enabled for this PR so maintainers can help update the branch when needed. This is a same-repository branch and is already writable by repository maintainers; no maintainer-edit opt-out is used.

What Problem This Solves

Resolves a problem where pnpm repair setup could exhaust its shared identity budget after a successful install, before validation began. The original failure was validation identity deadline exhausted during raw worktree head.

Why This Change Was Made

Reuse the actual post-install identity that the source guard just checked, instead of immediately capturing it a third time. The pre-install identity is never reused for the prepared runtime: installed ignored dependencies remain bound to it. Mutation checks, setup-error precedence, executable refresh, production deadlines/reserves, containment, concurrency, and permissions stay unchanged.

User Impact

Repair setup performs less redundant Git/filesystem work without extending its budget or allowing stale source/dependency state to validate. This is separate from the previously landed proof-classification follow-up at PR 1286.

OpenClaw Bay Impact

Unaffected. No queue, workflow, lifecycle/publication, status/telemetry, API, or dashboard contract changes.

Documentation Impact

Reviewed the root contributor/vision guides, documentation index, repair entry point, and internal feature map. Existing repair guidance remains accurate because no operational contract changed. Added a historical proof recipe and compact normalized observations under docs/proof/validation-identity-reuse/, covered by the existing historical-proof lifecycle classification, plus one changelog entry. No root guide or operational runbook changes.

Evidence

Tested committed head: 6b4919e47d6f5e78c3a33f96bf1a7ae2526d29ab. The fresh committed-head production proof passed in 123.550 seconds, with an empty production diff. All recorded source, test, compiled owner/supervisor, and recipe SHA256 values exactly match the reviewed candidate. The tracked observations deliberately remain a historical precommit snapshot at base 5ada40c98ae6dcf31703cf7c3f01e35e8d23a13e plus its recorded production diff; they have not been relabeled as a committed-head run. The fresh head and normalized receipts are recorded below.

Fresh isolated Codex precommit review reported no accepted/actionable finding at its configured P0 threshold. This is not a claim of review at every severity. Committed-branch review, exact-head GitHub CI, and the current-head-and-body ClawSweeper review remain required before merge; no merge or automerge is requested by this publication.

The frozen source and test patch are unchanged. On the prior preparation base, the new regression against an isolated baseline compilation reproduced the exact original third-capture failure at 60 completed Git calls / 15,000 charged ms. The current baseline source/test are verified byte-identical. The patch completes setup at 51 calls / 12,750 charged ms; the ordinary successful fixture path removes 25 Git calls (76 to 51). These are virtual-clock charges, not measured install speed.

Fresh current-base coverage runs pass 11/11 on Node 24.20.0 and 11/11 on Node 26.7.0, each with zero failures or skips. The set includes the new virtual-budget regression, original real-clock executable refresh, stale setup, ignored dependency poisoning, tracked source mutation, install-managed workspace links, setup-error precedence, shared deadlines, and validation reserves. Both new-regression runs report 51 Git calls / 12,750 charged ms. Node 24 was rerun rather than relying on old dependency fingerprints.

The pinned pnpm 11.10.0 build on this base passes; source/build/dependency input fingerprints remain unchanged through the fresh proof and tests. Final static checks, scoped source/test/proof formatting and lint, proof syntax, and documentation checks pass. The earlier documentation inventory included the untracked proof files; a fresh check:static run also passed against the exact staged six-file inventory before this commit. No full macOS coverage gate was repeated.

The earlier Node 26 11-case coverage attempt recorded 8 pass / 3 deadline failures in stale setup, executable refresh, and workspace-link fixtures during the disk-pressure period. The earlier preparation also passed the regression and original real-clock control separately on both Node 24 and Node 26, and the Node 24 covered 11-case set passed. These historical runs remain recorded; they are not relabeled as fresh current-head runs.

The separate peer full-coverage run on 468dab46ff57c65da3336e628482391c2896e4d6 reported 3,967 pass, 1 fail, and 9 skip. Its sole reported failure was the unchanged 129 MiB stdout fixture in test/codex-review-runner.test.ts, also failing isolated coverage against its 20-second budget. Full harness completion was lost/killed with exit 137, not a green full check. Full macOS coverage was deliberately not repeated during preparation. No unrelated fixture, deadline, concurrency, or coverage threshold changed; exact-head GitHub CI remains the full gate.

Real Behavior Proof

Claim and owner: prepareTargetToolchain retains the checked post-install identity; runAllowedValidationCommands permits real validation and rejects ignored runtime or tracked-source tampering before another script dispatch.

Scenario: a task-owned local Git repository and local-only origin, one tracked local dependency, isolated HOME, actual Corepack/pnpm. The validation script checks the installed dependency and prints a unique stdout marker without writing files. The harness then changes ignored .modules.yaml, verifies rejection without dispatch, restores exact bytes and validates again, then changes tracked verify.cjs and verifies another rejection without dispatch.

Commands: with Node 24.20.0, Corepack 0.35.0, and task-local pnpm 11.10.0 shims on PATH, build the pinned repository sources:

corepack pnpm run build:all

Create the ignored artifact parent if needed:

mkdir -p .artifacts

Run the production proof into this new, concrete output directory (it must not already exist):

node docs/proof/validation-identity-reuse/run-proof.mjs .artifacts/validation-identity-reuse-6b4919e-20260828

Nested build commands must also resolve pnpm 11.10.0. The recorded committed run used this identical recipe with a fresh task-local output directory; private host paths and raw driver logs are intentionally omitted here.

Environment and recording: Node v24.20.0, Corepack 0.35.0, pnpm 11.10.0, macOS arm64, provider local-node-test-harness; no image or lease. The test harness installs a transparent spawnSync observer that delegates identical arguments/options to the original function and returns its exact original result. It records only the real production supervisor calls for the fixture's exact verify script and parses their actual JSON stdout/status/background-process receipts. Malformed observation leaves production outcomes unchanged and fails proof separately. The observer and builtin ESM exports are restored in finally. No tool process, result, timeout, clock, or sandbox behavior is replaced.

Observed result: the fresh committed-head proof passed in 123.550 seconds total. Setup took 56.062 seconds in this run (the historical precommit run took 43.553 seconds for setup). Both allowed calls returned ['pnpm verify'] and produced exit-zero receipts with the expected stdout marker and zero background processes. Total dispatch/marker counts were 1/1 after the first allowed call, stayed 1/1 after runtime rejection, became 2/2 after restoring metadata and validating again, and stayed 2/2 after tracked-source rejection. Runtime rejection specifically reported runtimeInputsSha256; tracked-source rejection reported contentTreeSha, status, worktreeSha256. No observer diagnostics; restoration confirmed.

Artifacts: immutable links to the proof recipe, proof README, and recorded precommit observations. The historical record binds its precommit head plus diff; the fresh committed run has source.json.head = 6b4919e47d6f5e78c3a33f96bf1a7ae2526d29ab, an empty production diff, and identical source/build/recipe hashes. Generated committed source.json, observations.json, supervisor-receipts.json, phase records, and raw driver logs are retained locally. The normalized committed receipt below exposes the relevant trace without raw stdout, environment, host paths, secrets, or agent transcripts.

Earlier failed attempts: an outer-watchdog expiry and incomplete Corepack extraction during disk exhaustion remain recorded. After disk recovery, the original marker-file recipe completed setup but correctly failed because writing inside protected node_modules violated the guard. Only that observation mechanism was repaired: stdout receipts replace file writes, while guard and tamper assertions remain intact. The corrected recipe completed under the existing 600-second outer watchdog.

Limits: this is real production-owner execution with genuine Corepack/pnpm, Git, filesystem, subprocesses, final stdout I/O, and real clock. The existing Node-test macOS process fallback is exercised, not Linux namespace/network/filesystem containment or Windows execution. It is not a full OpenClaw dependency install or native-app proof. The separate regression uses real Git plus fake Corepack/pnpm executables and a virtual setup clock. No live repair/apply, GitHub target action, registry credentials, deployment, or global tool change occurred.

Fresh committed-head proof receipt (normalized)
{
  "head": "6b4919e47d6f5e78c3a33f96bf1a7ae2526d29ab",
  "elapsedSeconds": 123.55,
  "source": {
    "head": "6b4919e47d6f5e78c3a33f96bf1a7ae2526d29ab",
    "productionDiffSha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
    "sha256": {
      "src/repair/target-validation.ts": "e6cbe74265e7e0d20ca6c37aa9a40a53257ec76e5009373e4759524a43a2bb8f",
      "test/repair/target-validation.test.ts": "979acc9edca4b77b25d453d59703914fd8f4ad82712bf484fb571a0e6ce6fa54",
      "dist/repair/target-validation.js": "4822598d005160e9df9c0d117a7e983e0f87ae790b255535289ee346439c7370",
      "dist/repair/command-runner.js": "4f4332dfcb43868319cc2652687355144bffe142b45c9ea64833f112da696904",
      "dist/repair/contained-command-worker.js": "50e7a4acde15948d0892bddc32a44a4477cebfdff790635fca520f9df948e813",
      "docs/proof/validation-identity-reuse/run-proof.mjs": "9f25c70c2d203b000171e50f5ec4cd561256b30b61832103fd070ed59344dbb6"
    }
  },
  "observations": {
    "provider": "local-node-test-harness",
    "image": null,
    "lease": null,
    "platform": "darwin",
    "arch": "arm64",
    "node": "v24.20.0",
    "corepack": "0.35.0",
    "pnpm": "11.10.0",
    "pnpmEntrypointSha256": "ff3224d46b47fbb24a7e9fe15fededef7e00892d07d4e376b6762d4899906bfd",
    "clock": "real",
    "injectedToolProcesses": false,
    "observerRestored": true,
    "setupWallMs": 56062,
    "allowed": [
      "pnpm verify"
    ],
    "dispatches": 2,
    "receipts": [
      {
        "supervisorStatus": 0,
        "supervisorSignal": null,
        "supervisorError": false,
        "status": 0,
        "signal": null,
        "error": false,
        "backgroundProcesses": 0,
        "stdoutMarker": true,
        "stdoutSha256": "496f929210abd61d2e4afe97d7dc56055220a8f0ba36d43fec7fd4a6a61dc660"
      },
      {
        "supervisorStatus": 0,
        "supervisorSignal": null,
        "supervisorError": false,
        "status": 0,
        "signal": null,
        "error": false,
        "backgroundProcesses": 0,
        "stdoutMarker": true,
        "stdoutSha256": "496f929210abd61d2e4afe97d7dc56055220a8f0ba36d43fec7fd4a6a61dc660"
      }
    ],
    "boundaries": [
      {
        "name": "allowed",
        "dispatches": 1,
        "stdoutMarkers": 1
      },
      {
        "name": "runtime-tamper-rejected",
        "dispatches": 1,
        "stdoutMarkers": 1
      },
      {
        "name": "restored-runtime-allowed",
        "dispatches": 2,
        "stdoutMarkers": 2
      },
      {
        "name": "source-tamper-rejected",
        "dispatches": 2,
        "stdoutMarkers": 2
      }
    ],
    "diagnostics": [],
    "runtimeRejection": "prepared target pnpm toolchain is stale; refresh dependencies before validation: runtimeInputsSha256",
    "restoredRuntimeAllowed": true,
    "sourceRejection": "prepared target pnpm toolchain is stale; refresh dependencies before validation: contentTreeSha, status, worktreeSha256"
  }
}

@clawsweeper

clawsweeper Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 28, 2026
@clawsweeper

clawsweeper Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 28, 2026, 4:24 PM ET / 20:24 UTC.

ClawSweeper review

What this changes

Reuses the verified post-install checkout identity during pnpm repair setup so validation preparation avoids a redundant identity scan without relaxing stale-input checks.

Regression provenance

Possible regression — probable (reproduction; reviewed change). No predecessor PR is attributed.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep open: current main still performs the redundant post-install identity capture, while this PR supplies a narrow repair, focused regression coverage, and committed-head production-owner proof.

Priority: P2
Reviewed head: 6b4919e47d6f5e78c3a33f96bf1a7ae2526d29ab

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A bounded repair with concrete current-head behavior evidence and focused safety regression coverage.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (logs): The changed production owner, prepareTargetToolchain, supplies the stored identity later consumed before runAllowedValidationCommands dispatches pnpm. The exact-head proof described in the PR body exercises real Corepack/pnpm, Git, filesystem state, and the production supervisor: allowed validation succeeds twice, while ignored-runtime and tracked-source mutations are rejected before additional dispatches; its stated limit is a synthetic macOS fixture rather than containment proof.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (logs): The changed production owner, prepareTargetToolchain, supplies the stored identity later consumed before runAllowedValidationCommands dispatches pnpm. The exact-head proof described in the PR body exercises real Corepack/pnpm, Git, filesystem state, and the production supervisor: allowed validation succeeds twice, while ignored-runtime and tracked-source mutations are rejected before additional dispatches; its stated limit is a synthetic macOS fixture rather than containment proof.
Evidence reviewed 5 items Introduced repair: The branch stores the identity returned by the required post-install assertion instead of immediately recapturing it; the assertion still compares all non-runtime source fields before the runtime is frozen.
Current-main gap: The base revision retains a separate validationSourceIdentity call after the post-install assertion, so the central timeout avoidance is not already implemented on current main.
Regression coverage: The new shared-budget fixture verifies setup completes below its virtual deadline, permits the prepared commands, and rejects a poisoned installed dependency before another validation dispatch.
Findings None None.
Security None None.

How this fits together

The repair lane prepares an isolated target repository and freezes a pnpm runtime before running allowlisted validation commands. Checkout and runtime identities prevent changed source or installed dependencies from reaching those commands.

flowchart LR
  A[Target repository] --> B[Repair setup]
  B --> C[Post-install identity check]
  C --> D[Prepared pnpm runtime]
  D --> E[Allowed validation commands]
  C --> F[Reject stale source or runtime]
Loading

Before merge

  • Complete next step (P2) - No discrete repair remains beyond ordinary exact-head CI completion and maintainer landing review.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Scoped change size production +4/-2; tests +201/-48; proof/docs +550 across 6 files The production behavior change is small relative to the focused regression and retained proof record.

Technical review

Best possible solution:

Retain the returned post-install identity as the prepared runtime binding, with the existing source and runtime mismatch checks unchanged.

Do we have a high-confidence way to reproduce the issue?

Yes. The submitted virtual-clock regression reproduces the redundant identity work exhausting the shared setup budget on the baseline and verifies the repaired path under the same budget.

Is this the best way to solve the issue?

Yes. Reusing the identity that the post-install guard has already captured removes duplicate work while preserving the required comparison and the prepared-runtime stale-input binding.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against e4eff74201f5.

Labels

Label changes:

  • add P2: This fixes a bounded repair-validation timeout without evidence of an active broad user outage.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The changed production owner, prepareTargetToolchain, supplies the stored identity later consumed before runAllowedValidationCommands dispatches pnpm. The exact-head proof described in the PR body exercises real Corepack/pnpm, Git, filesystem state, and the production supervisor: allowed validation succeeds twice, while ignored-runtime and tracked-source mutations are rejected before additional dispatches; its stated limit is a synthetic macOS fixture rather than containment proof.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (logs): The changed production owner, prepareTargetToolchain, supplies the stored identity later consumed before runAllowedValidationCommands dispatches pnpm. The exact-head proof described in the PR body exercises real Corepack/pnpm, Git, filesystem state, and the production supervisor: allowed validation succeeds twice, while ignored-runtime and tracked-source mutations are rejected before additional dispatches; its stated limit is a synthetic macOS fixture rather than containment proof.

Label justifications:

  • P2: This fixes a bounded repair-validation timeout without evidence of an active broad user outage.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (logs): The changed production owner, prepareTargetToolchain, supplies the stored identity later consumed before runAllowedValidationCommands dispatches pnpm. The exact-head proof described in the PR body exercises real Corepack/pnpm, Git, filesystem state, and the production supervisor: allowed validation succeeds twice, while ignored-runtime and tracked-source mutations are rejected before additional dispatches; its stated limit is a synthetic macOS fixture rather than containment proof.
  • proof: sufficient: Contributor real behavior proof is sufficient. The changed production owner, prepareTargetToolchain, supplies the stored identity later consumed before runAllowedValidationCommands dispatches pnpm. The exact-head proof described in the PR body exercises real Corepack/pnpm, Git, filesystem state, and the production supervisor: allowed validation succeeds twice, while ignored-runtime and tracked-source mutations are rejected before additional dispatches; its stated limit is a synthetic macOS fixture rather than containment proof.

Evidence

What I checked:

  • Introduced repair: The branch stores the identity returned by the required post-install assertion instead of immediately recapturing it; the assertion still compares all non-runtime source fields before the runtime is frozen. (src/repair/target-validation.ts:440, 6b4919e47d6f)
  • Current-main gap: The base revision retains a separate validationSourceIdentity call after the post-install assertion, so the central timeout avoidance is not already implemented on current main. (src/repair/target-validation.ts:446, e4eff74201f5)
  • Regression coverage: The new shared-budget fixture verifies setup completes below its virtual deadline, permits the prepared commands, and rejects a poisoned installed dependency before another validation dispatch. (test/repair/target-validation.test.ts:4082, 6b4919e47d6f)
  • Real behavior proof: The checked-in recipe invokes the compiled production setup and validation owners with real Corepack, pnpm, Git, filesystem state, and the contained-command supervisor; the PR body provides an exact-head normalized receipt with two successful dispatches and two no-dispatch tamper rejections. (docs/proof/validation-identity-reuse/run-proof.mjs:164, 6b4919e47d6f)
  • Area history: Recent target-validation history includes Peter Steinberger's deterministic shared-deadline test work and Milton's prior post-install identity repair, establishing shared ownership of this safety-sensitive path. (src/repair/target-validation.ts:354, 9bad4750e84b)

Likely related people:

  • Peter Steinberger: Recent commits established deterministic deadline coverage and hardened the local Git/validation fixtures used by this path. (role: recent repair-validation contributor; confidence: high; commits: 9bad4750e84b, 0b3a12508801; files: src/repair/target-validation.ts, test/repair/target-validation.test.ts)
  • Milton: Authored the previous fix for the post-install identity check when pnpm materializes an untracked lockfile. (role: adjacent identity-fix contributor; confidence: high; commits: 7f9e3f99e312; files: src/repair/target-validation.ts, test/repair/target-validation.test.ts)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete merged commit b1d6b85 into main Aug 28, 2026
19 checks passed
@steipete
steipete deleted the steipete/validation-identity-reuse-f87932 branch August 28, 2026 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant