Skip to content

docs(solutions): document skipped cleanup identity guard - #1392

Merged
marcusrbrown merged 1 commit into
mainfrom
docs/compound-skipped-cleanup-sync
Aug 13, 2026
Merged

docs(solutions): document skipped cleanup identity guard#1392
marcusrbrown merged 1 commit into
mainfrom
docs/compound-skipped-cleanup-sync

Conversation

@marcusrbrown

Copy link
Copy Markdown
Collaborator

Summary

  • Documents the early-routing-to-finally identity gap that can emit a non-fatal object-store warning on skipped workflow-dispatch runs.
  • Clarifies the narrow fix principle: keep overall cleanup unconditional, and gate only optional object-store artifact/metadata sync behind complete repo+run identity.
  • Includes production evidence from PR fix(cleanup): skip object-store sync without run identity #1389 and post-merge run 31743621043, where the empty-prompt skip still occurs and object-store warning signals dropped from 1 to 0 while intended skip behavior remained.
  • Links related solution docs and prevention guidance for same-risk paths.

Verification

  • 15 script test files validated in the change that this doc captures.
  • 346 markdown links validated.
  • Prettier run completed.
  • Schema/module-taxonomy validation completed.
  • Simplicity and TypeScript/source reviews completed.

No code behavior changed in this PR; this is documentation-only.

@fro-bot fro-bot left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Verdict: PASS

Documentation-only change adding one solution doc (docs/solutions/logic-errors/skipped-object-store-cleanup-with-empty-run-identity-2026-08-13.md, +85/-0). No code behavior changes. I verified every technical claim in the doc against the current codebase and all of them hold.

Verification performed:

  • Guard claim — The doc's core code snippet if (storeConfig.enabled === true && repo !== "" && runId !== "") matches src/harness/phases/cleanup.ts:122 exactly, and the object-store artifact/metadata sync is correctly scoped inside it while cache save (:170-181) and prompt-artifact upload (:187-198) remain outside.
  • Run-identity gap claim — Confirmed in src/harness/run.ts: repo/runId initialize to '' (:37-38), are assigned only after runRouting returns non-null (:70-71), and runCleanup is always invoked from finally (:180). The early routing exit (if (routing == null) return 0, :67) is precisely the "valid cleanup work, no valid run identity" path the doc describes.
  • Test-coverage claimssrc/harness/phases/cleanup.test.ts:145-179 is the table-driven test the doc references. It parameterizes both missing repo and missing runId, asserts createS3Adapter/syncArtifactsToStore/syncMetadataToStore are not called, and asserts saveCache and uploadLogArtifact still run — matching the doc's bullet list verbatim.
  • Frontmatter taxonomymodule: development-workflow is a member of the closed taxonomy enforced by scripts/module-taxonomy.test.ts:21.
  • Related links — All four relative links resolve to existing files under docs/solutions/ (best-practices, integration-issues, workflow-issues, logic-errors).

Blocking issues

None

Non-blocking concerns

  • The doc references production run IDs in two places with slightly different framing: the Symptoms section cites run 31730967099 as the reproduction, while the PR description mentions run 31730708006-era context and post-merge run 31743621043. Both post-merge references (31743621043) are consistent between doc and description; only the pre-fix reproduction run differs, which is expected. No action needed — these are external run URLs that cannot be validated from the repo and don't affect doc correctness.

Missing tests

None. This is a documentation-only PR; the behavior it describes is already covered by cleanup.test.ts (which I verified). Docs are validated by the repo's link-check, prettier, schema, and module-taxonomy checks per the PR description.

Risk assessment (LOW/MED/HIGH): LOW

Single added Markdown file, zero code or config changes, no public API surface. Blast radius is limited to documentation. Regression likelihood is effectively nil; the only drift risk is the doc becoming stale if cleanup.ts changes, but it accurately reflects the current implementation and is cross-linked for maintainability.


Run Summary
Field Value
Event pull_request
Repository fro-bot/agent
Run ID 31746193504
Cache hit
Session ses_002f26990ffe7amaWMunVa9NJW

@marcusrbrown
marcusrbrown merged commit 238cad4 into main Aug 13, 2026
15 checks passed
@marcusrbrown
marcusrbrown deleted the docs/compound-skipped-cleanup-sync branch August 13, 2026 21:39
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.

2 participants