Skip to content

Preserve verified artifacts before Smithers worktree cleanup - #125

Closed
aviggiano wants to merge 21 commits into
mainfrom
agent/preserve-verified-artifacts
Closed

aviggiano wants to merge 21 commits into
mainfrom
agent/preserve-verified-artifacts

Conversation

@aviggiano

@aviggiano aviggiano commented Jul 23, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • atomically publish the exact verifier-approved artifact bytes into canonical storage before a Smithers task can complete
  • include declared generated-test companions in the same validated publication set
  • fall back to an exclusive, durable descriptor write when a mounted filesystem rejects hard links
  • emit a typed, non-resumable Modal terminal diagnostic when an already-terminal checkpoint cannot advance
  • provision new Modal benchmark storage as version-2 volumes and verify the resolved version and identity before attachment
  • reap successful Smithers task worktrees after their verified artifacts have been durably published
  • add real lifecycle, recovery, volume-version, and no-hard-link regression coverage

Root cause

The generated workflow validated task-owned mirror files, but canonical publication happened later in an out-of-process synchronizer. Smithers could therefore reap a successful worktree containing only ignored artifacts before the synchronizer copied them. The resulting terminal artifact-contract failure was then classified as resume-required even though the durable run had no resumable transition.

The first publication fix used a hard link to provide an atomic no-replace boundary. Some durable mounted filesystems reject hard links with EPERM, which made verification fail even though ordinary exclusive file creation was supported. The fallback retains no-replace behavior with O_EXCL, writes through the opened descriptor, fsyncs, validates exact bytes and file identity, and removes a partially written destination on a handled error.

Large benchmark workflows can also exhaust the fixed inode ceiling of Modal version-1 volumes while many Smithers worktrees are active. The Modal JavaScript SDK does not yet expose a public filesystem-version selector for named volumes, so launch now performs a narrow control-plane version-2 creation preflight, verifies the returned version, and checks that the public lookup resolves the same volume. Successful worktrees are then reaped because canonical artifact publication no longer depends on retaining them.

Validation

  • Artifacts: 49 tests passed
  • Runtime: full package test suite passed, including the real Smithers worktree cleanup lifecycle (one unrelated optional smoke skipped)
  • Modal: 172 tests passed
  • Modal and runtime typechecks/builds passed
  • Real Modal mounted-volume probe reproduced hard-link rejection and passed exact-byte, idempotency, conflict, cleanup, and file-mode checks
  • Live Modal version-2 named-volume create, metadata verification, lookup, and deletion probe passed
  • ESLint, Prettier, and git diff --check passed
  • GitHub release and benchmark gates are running on the updated commit

Fixes #122

Greptile Summary

This PR fixes a race condition where a successful Smithers worktree could be reaped before its verified artifacts were durably persisted to canonical storage, causing artifact-contract failures that were incorrectly classified as resumable. It also adds a fallback publication path for filesystems that reject hard links, upgrades Modal benchmark volumes to filesystem version 2 to avoid inode exhaustion, introduces Kimi subscription-auth lifecycle management, and emits a typed non-resumable diagnostic when an already-terminal checkpoint cannot advance.

  • Artifact publication fix: publishFileDurableExclusive in packages/artifacts/src/safe-paths.ts atomically seals verified bytes (primary artifact + generated-test companions) into the canonical artifact directory before verifyArtifacts returns, using hard-link where available and an O_EXCL descriptor write + fsync fallback for Modal's mounted volumes.
  • Volume v2 preflight: getOrCreateModalV2Volume in packages/modal/src/volume.ts calls the undocumented control-plane volumeGetOrCreate endpoint with an explicit filesystem-version-2 request, verifies the returned version and volume identity, then falls back to the public fromName lookup for attachment.
  • Terminal diagnostic: NonResumableTerminalRunError replaces a generic Error in repairModalEvalRunRecord, is caught in the worker's diagnosticCodeForError hook, and maps to the "terminal-run-non-resumable" permanent-failure code so the overseer does not schedule another Modal generation.

Confidence Score: 5/5

Safe to merge; the core artifact-publication race fix is well-tested with a real Smithers lifecycle integration test, and the Modal volume v2 path includes runtime identity verification.

The new publishFileDurableExclusive function handles all expected failure modes (EPERM from hard links, EEXIST from concurrent publishers, EIO-style close failures) with correct fd lifecycle management. The two findings are style-level concerns: a string-match heuristic and an incomplete status-spelling list, neither of which affects correctness on the current version.

Files Needing Attention: smithersSnapshotHasMissingRunHistory in packages/runtime/src/smithers.ts and isTerminalRunStatus in packages/modal/src/resume.ts are worth a second look at the next Smithers version bump.

Important Files Changed

Filename Overview
packages/artifacts/src/safe-paths.ts Adds publishFileDurableExclusive — an atomic no-replace publication function using hard-link + O_EXCL fallback with fsync and identity verification.
packages/modal/src/volume.ts New file creating Modal v2 volumes via the undocumented control-plane API with post-creation version and identity verification.
packages/runtime/src/smithers.ts Adds --force/--retry-failed passthrough and recovery helpers; the missing-run-history string-match heuristic is the only notable fragility.
packages/modal/src/resume.ts Introduces NonResumableTerminalRunError and modalDurableRunNeedsResume; isTerminalRunStatus missing "cancelled" spelling is a minor inconsistency.
packages/runtime/src/templates/smithers/workflows/workflow.tsx Adds pre-reap canonical publication of verified bytes including generated-test companions.
packages/modal/src/auth.ts New Kimi subscription-auth subsystem with token refresh, lockfile-guarded staging, and credential reconciliation.
packages/modal/src/runner.ts Integrates v2 volume creation, Kimi auth lifecycle, and public benchmark secret values.
packages/runtime/src/validate.ts Narrows agent validation to topology-referenced agents only.
packages/modal/src/worker.ts Uses modalDurableRunNeedsResume and maps NonResumableTerminalRunError to typed diagnostic.
packages/runtime/test/smithers-artifact-publication.integration.test.ts Integration test directly exercising the core artifact-survives-reap fix.
packages/modal/src/topology-config.ts Replaces regex-based loop rewriting with proper YAML parse/stringify.
packages/modal/src/worker-lineage.ts Adds same-workspace resume logic before the generation check.

Reviews (8): Last reviewed commit: "Type Modal topology loop test" | Re-trigger Greptile

@socket-security

socket-security Bot commented Jul 23, 2026 •

Copy link
Copy Markdown

Warning

Review the following alerts detected in dependencies.

According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.

Action Severity Alert  (click "▶" to expand/collapse)
Warn High
Obfuscated code: npm @moonshot-ai/kimi-code is 90.0% likely obfuscated

Confidence: 0.90

Location: Package overview

From: packages/runtime/package.json → npm/@moonshot-ai/kimi-code@0.29.1

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/@moonshot-ai/kimi-code@0.29.1. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Warn High
Obfuscated code: npm drizzle-orm is 90.0% likely obfuscated

Confidence: 0.90

Location: Package overview

From: pnpm-lock.yaml → npm/smithers-orchestrator@0.29.0 → npm/drizzle-orm@0.45.2

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/drizzle-orm@0.45.2. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Warn High
Obfuscated code: npm effect is 90.0% likely obfuscated

Confidence: 0.90

Location: Package overview

From: pnpm-lock.yaml → npm/smithers-orchestrator@0.29.0 → npm/effect@3.21.4

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/effect@3.21.4. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Warn High
Obfuscated code: npm json-schema is 90.0% likely obfuscated

Confidence: 0.90

Location: Package overview

From: pnpm-lock.yaml → npm/smithers-orchestrator@0.29.0 → npm/json-schema@0.4.0

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/json-schema@0.4.0. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Warn High
Obfuscated code: npm rimraf is 90.0% likely obfuscated

Confidence: 0.90

Location: Package overview

From: pnpm-lock.yaml → npm/smithers-orchestrator@0.29.0 → npm/rimraf@5.0.10

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/rimraf@5.0.10. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Warn High
Obfuscated code: npm robust-predicates is 90.0% likely obfuscated

Confidence: 0.90

Location: Package overview

From: pnpm-lock.yaml → npm/smithers-orchestrator@0.29.0 → npm/robust-predicates@3.0.3

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/robust-predicates@3.0.3. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

@aviggiano
aviggiano marked this pull request as ready for review July 23, 2026 19:52
Comment thread packages/artifacts/src/safe-paths.ts
Comment thread packages/modal/src/volume.ts
Comment thread packages/artifacts/src/safe-paths.ts
@aviggiano
aviggiano force-pushed the agent/preserve-verified-artifacts branch from 6616240 to 5e2cfda Compare July 24, 2026 16:04
@socket-security

socket-security Bot commented Jul 24, 2026 •

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Added@​moonshot-ai/​kimi-code@​0.29.1751009899100
Addedproper-lockfile@​4.1.210010010075100
Addedsmithers-orchestrator@​0.29.07610010096100
Added@​types/​proper-lockfile@​4.1.41001007980100

View full report

@aviggiano

Copy link
Copy Markdown
Collaborator Author

Superseded by #140, which incorporates this PR’s complete artifact-preservation change set together with the pinned-source hardening tracked in #139. Closing this PR so review can continue in one place.

@aviggiano aviggiano closed this Jul 27, 2026
@aviggiano aviggiano reopened this Jul 27, 2026
@aviggiano

Copy link
Copy Markdown
Collaborator Author

Closing as superseded by #140, which now contains the complete artifact-preservation change set plus the pinned-source hardening.

@aviggiano aviggiano closed this Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preserve verified artifacts before Smithers worktree reaping

1 participant