Retry filesystem contention, budget the test suite once - #8
Merged
Merged
Conversation
Three findings from chasing an intermittent staged-checkout failure. isTransientGitError classified only network transport as worth a retry: timeouts, DNS, RPC, EOF, SSL, reset. Filesystem contention matched nothing, so it was the one class of transient failure that never got the retry the clone path already implements. On Windows an indexer, an antivirus scanner, or a git child that has not fully exited holds a handle and the unlink or rename is denied outright rather than deferred, which fails a clone that would have succeeded a second later. The parenthesized SSH form stays non-transient because isGitAuthError is consulted first. The process-boundary tests asserted only `.outcome` on a result whose `message` field carries the reason it failed, so the one observed failure reported `expected "added", received "failed"` and nothing else. All six sites now assert the result object, which prints the message. Bun's 5000ms default sat inside the spread between runners: one measured case costs 0.7s on ubuntu and 5.9s on windows-latest. Two different tests timed out on two consecutive pushes to main, neither anywhere near hanging. scripts/test.ts now sets one budget for the process-boundary suite, which fixes the class rather than the instance, and supersedes the per-test figure added for the first of those two. The mocked unit suite keeps the default. The original failure did not reproduce in 25 consecutive runs, so the contention hypothesis is unconfirmed as its cause. The next occurrence will name its own reason. Co-Authored-By: Claude Opus 5 <[email protected]>
NomadicDaddy
added a commit
that referenced
this pull request
Sep 10, 2026
* docs(release): cut 1.1.1 changelog and bump version * fix(tooling): delete misleading .nvmrc file - .nvmrc contained '24' implying Node.js v24 runtime; project uses Bun exclusively - packageManager [email protected], only-allow bun preinstall hook, bun-types confirm Bun-only - File was previously deleted in 3de0117 but re-added in 1d4ffb2 ('deps') - Verified git ls-files no longer tracks nvmrc; README already documents Bun >= 1.3.14 - package-manager-enforcement feature.json step #5 guards against reintroduction * fix(licensing): add LICENSE file for declared MIT license - package.json declares "license": "MIT" and lists LICENSE in files array, but the file was missing - Any published tarball would have omitted the required license text (MIT violation) - Created LICENSE with full MIT text, copyright Copyright (c) 2026 NomadicDaddy (matches author field) - Prevention guard added to package-manager-enforcement feature.json spec step 6 - Resolves audit finding: audit-licensing-1784089532-license-file-missing-despite-package-json-license-field-and-readme-claim - bun run smoke:qc passes (typecheck + lint + format:check) * feat(cli): add archive CLI command suite with subcommand dispatch - Register sync, verify, migrate, dates, init, unlock as explicit subcommands - Each subcommand has shared target-path parsing and per-command --help output - Not-yet-shipped subcommands exit 1 with 'not available in this release' - Add sync --dry-run (query stars, inspect archive, no git operations) - Add migrate --apply flag handling (deferred to 2.0) - Route dates through extracted dates-command module - Keep bare starsync and set-folder-dates as deprecated aliases with warnings - Warn when starred_repos fallback path is used - Add SUBCOMMANDS const and isSubcommand type guard to cli-utils - New files: help-text.ts, subcommands.ts, dates-command.ts - 19 new tests (41 total), smoke:qc + build pass * feat(security): separate API auth from Git transport, add secret safety - New src/lib/secret-safety.ts: URL credential detection, token redaction, GitHub.com host validation, Git auth error detection, credential guidance - cloneOrPull: validates repository origins as GitHub.com before any git operation - Git runs non-interactively (GIT_TERMINAL_PROMPT=0, core.askPass=) with auth failure guidance - Remote URLs and error messages sanitized — embedded credentials stripped before output - GitHub PAT patterns (ghp_…, github_pat_…) redacted from error messages - Updated .env.example and README with authentication model documentation - 25 new tests (66 total, all pass); smoke:qc, build pass - Updated .aidd/features/git-authentication-and-secret-safety/feature.json: passes true, status completed * docs(readme): correct glob scopes in script table for lint, format, and format:check - Changed lint from eslint "**/*.ts" to eslint "src/**/*.ts" - Changed format from prettier --write "**/*.ts" "*.json" to prettier --write "src/**/*.ts" - Changed format:check from prettier --check "**/*.ts" "*.json" to prettier --check "src/**/*.ts" - All three entries now match package.json verbatim - Resolves audit finding audit-documentation-1784089535 * fix(licensing): add private:true to package.json to prevent accidental publish - Added "private": true to package.json top-level - Verified bun publish refuses with 'attempted to publish a private package' - Updated package-manager-enforcement feature spec step #7 (prevention guard) - Marked audit-licensing-1784089532-private-true finding as resolved * fix(config): remove duplicate .claude/ and dist/ entries from .gitignore - Removed second occurrence of .claude/ and dist/ from .gitignore - Verified: sort .gitignore | uniq -d returns empty; git check-ignore confirms all paths still ignored - Added prevention step #8 to gitignore-and-environment-protection feature.json * fix(qc): remove redundant @typescript-eslint/eslint-plugin and parser devDeps - The typescript-eslint meta-package (8.64.0) bundles both transitively - Neither standalone package is imported in eslint.config.js or any source - Verified: smoke:qc, lint, 66 tests, build all pass - Added prevention step #8 to eslint-code-style-enforcement feature.json * fix(qc): widen lint and format globs to cover scripts/ and test/ directories - Widened lint, lint:fix, format, and format:check in package.json from src/**/*.ts to include scripts/**/*.ts and test/**/*.ts - Fixed perfectionist/sort-imports error in test/index.test.ts (type import before value import with blank-line separation) - Updated README.md Scripts table to reflect expanded glob scopes - Prettier auto-fixed quotes in scripts/set-folder-dates.ts Audit finding: audit-qc-pipeline-1784153818 (Critical) * fix(qc): include bun test in smoke:qc quality gate - Added && bun test to smoke:qc script in package.json so pre-commit gate includes unit tests - Updated README.md Scripts table: smoke:qc now documents typecheck, lint, format check, test - Updated .aidd/ spec.md, assertions.md, CHANGELOG.md, and build-and-compile-pipeline feature.json (prevention step #9) - Verified: bun run smoke:qc passes (typecheck + lint + format:check + bun test, 66 tests) * feat(sync): replace sequential pull loop with bounded concurrency refresh pool - New src/lib/refresh.ts: bounded-concurrency pool (default 4, range 1-8) with per-repo progress (Syncing N/Total), safe refresh (fetch --tags --no-prune, ff-only when clean, block divergent/dirty), clone with transient retry, and graceful interruption (first SIGINT stops scheduling, second terminates) - New src/lib/git-exec.ts: async execFile wrapper (no shell injection) with isTransientGitError classifier (auth/not-found/divergence/integrity never retried) - New src/lib/api-retry.ts: withApiRetry retries 5xx/429 up to 2 times honoring retry-after/x-ratelimit-reset; 401/403/404/422 fail immediately - Updated src/index.ts: --concurrency flag, runSyncPool/processRepository replaces sequential cloneOrPull loop, retained checkout tracking, detailed outcome summary (Added/Updated/Current/Blocked/Skipped/Retained/Failed) - Updated src/lib/help-text.ts: --concurrency documentation in sync help - Updated test/index.test.ts: 26 new tests for refresh pipeline, concurrency parsing, git-exec classification, api-retry (92 total, all pass) * feat(cli): add structured command reporting Add schema-versioned JSON output and shared human reporting across all subcommands. Separate checkout lifecycle, pending rename, run outcome, and finding severity while preserving interruption and exit-code semantics. Document the contract and verify with 104 tests, production build, and real CLI stream checks. * feat(migration): add legacy archive preview Implement read-only identity and rename planning for legacy archives. Classify all 289 live checkouts without modifying archive metadata. * feat(verify): add read-only archive verification Verify legacy and managed archives locally for Git integrity, owner and checkout identity metadata, safe origins, blocked state, duplicate identities, and pending renames. Add schema-versioned CLI reporting, interruption handling, read-only process-boundary tests, documentation, changelog, and completed archive-verification metadata. * feat(api): add programmatic archive operations Expose explicit Bun archive operations with structured reports, progress callbacks, and AbortSignal cancellation. Delegate CLI commands to the library API, deprecate low-level exports, remove the Node engine declaration, and verify with 122 tests plus bundle and standalone builds. * chore(git): hide .aidd metadata and guard against publishing it This repo has a published remote, so its .aidd/ blueprint must never reach GitHub: git history is retroactive and a push cannot be undone. Anchor the ignore rule to the repo root (.aidd/ also matched nested directories) and keep prettier out of it entirely — aidd owns the format of what it writes. The pre-push guard blocks a push carrying .aidd history: a clean tip says nothing about the commits behind it. * chore: groom to workspace TS standards (config-only) - package.json: pin eslint 10.7.0 (was ^10.6.0), typescript 6.0.3 (was ^6.0.3), only-allow 1.2.2 (was ^1.2.2); add --max-warnings 0 to lint and lint:fix scripts - tsconfig.json: add allowSyntheticDefaultImports, erasableSyntaxOnly, noImplicitReturns, noUncheckedSideEffectImports - eslint.config.js and .prettierrc already conform to standard (no change) - AGENTS.md left untouched Gate results (all pass, no surfaced errors): - typecheck: pass - lint: pass (0 warnings, --max-warnings 0) - format:check: pass * chore(tooling): replace only-allow Bun guard * docs(release): cut 1.2.0 changelog and bump version * feat(archive): initialize managed archives - authenticate and persist exact archive owner metadata - require explicit targets and enforce owner and format guards - cover initialization and non-destructive failure paths * feat(archive): lock archive operations - serialize archive inspection and mutation with portable ownership metadata - reclaim only confirmed-dead local locks and add risk-reported forced unlock - route CLI, programmatic API, and deprecated dates entrypoint through locking - cover Windows, macOS, Linux, stale, live, uncertain, and forced cases * feat(archive): migrate managed checkout identities - match and persist stable GitHub repository identities during sync - apply resumable collision-safe canonical checkout migrations - verify migration behavior through real Git process boundaries * feat(archive): manage checkout archive dates - derive Archive Dates from every reachable local Git reference - normalize dates after sync and through the unified CLI and API - add real-Git coverage and remove the standalone compatibility alias * feat(sync): stage new checkout publication - Clone and validate new managed checkouts before atomic publication - Preserve occupied destinations and clean only owned staging directories - Add isolated real-Git and focused retry, collision, and redaction coverage * feat(release): add cross-platform validation gates - run locked Bun quality, build, and compile checks on Windows, macOS, and Linux - validate a representative copied archive offline and guard the opt-in live smoke target - cover repository URL casing, suffix, SSH, trailing slash, and Unicode normalization * docs(release): cut 1.3.0 changelog and bump version * refactor(core): remove unreachable utilities - Remove the unused synchronous archive lock wrapper - Remove the unused aggregate subcommand help lookup - Preserve directly consumed async locking and help exports * ci(release): pin third-party actions - Pin checkout v6 and setup-bun v2 to reviewed full commit SHAs - Enforce immutable references and release-tag comments in release validation tests * ci(release): bound validation runs - Cancel superseded workflow runs within the same pull request or ref. - Bound each platform job to 20 minutes and enforce the workflow contract in tests. * test(release): expose cross-platform report failures Make failed release-boundary assertions print their full reports so platform-specific findings remain actionable in CI. * test(release): expose cross-platform report failures Make failed release-boundary assertions print their full reports so platform-specific findings remain actionable in CI. * docs(archive): reconcile managed archive documentation - align README with the six live commands, explicit targets, staged publication, and the local Bun guard - record explicit targeting and the command-level API as accepted implemented decisions - keep ignored AIDD contracts and completion metadata authoritative on disk * refactor(cli): remove public index dependency - Move sync parser state into dependency-neutral CLI utilities - Preserve parser and help compatibility through public re-exports - Add regression coverage for dispatch imports and sync help * fix(sync): isolate invalid managed checkouts - Continue valid starred and retained checkout planning after identity scan failures - Aggregate blocked and successful outcomes with exit code 1 - Cover normal, dry-run, and retained mixed archives * fix(sync): follow remote default branch - carry GitHub default_branch through managed sync planning - switch or create the validated default branch before fast-forwarding - cover clean, renamed, missing, dirty, divergent, and invalid branch states with real Git * refactor(archive): enforce workflow source boundaries - Split archive API, migration, verification, dates, and refresh coordinators behind stable facades - Add source-shape enforcement and facade characterization coverage - Document the check:max-lines quality gate * fix(security): isolate git subprocess environments - Add one allowlisted environment builder for every Git process boundary - Prevent parent and override secrets from reaching Git helpers, hooks, or filters - Cover async and synchronous boundaries with platform-variable retention tests * chore(tooling): align formatter and lint pins - Pin Prettier 3.9.6 and typescript-eslint 8.65.0 to the live workspace baseline. - Regenerate the Bun lockfile and verify frozen install, typecheck, lint, formatting, tests, and build. * chore(tooling): lint javascript configuration - Add isolated ESLint coverage for eslint.config.js - Enumerate tooling and TypeScript lint targets explicitly - Keep ignored AIDD completion metadata validated locally * feat(githooks): add screenshot artifact guard to pre-push hook Block pushing version tags whose screenshot directory is missing or incomplete. Releases v3.25.0–v3.28.2 shipped without visual records because nothing enforced the artifact. The guard checks for directory existence, a minimum PNG count, and a successful crawl-result.json stamp when present. Refactor the pre-push wrapper to capture stdin once and replay it into each guard, since only the first consumer would otherwise see the ref updates. * feat(verify): add forced checkout recovery * docs(release): cut 1.4.0 changelog and bump version * fix(sync): stream live checkout progress * docs(release): cut 1.4.1 changelog and bump version * chore: add local deployment command * test: make Git environment assertion cross-platform ---------
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three findings from chasing the intermittent staged-checkout failure, plus the red
mainfrom the last push.1.
isTransientGitErrorwas blind to filesystem contention. It classified only network transport as retryable — timeouts, DNS, RPC, EOF, SSL, reset. Filesystem contention matched nothing, making it the one class of transient failure that never received the retrycloneValidatedCheckoutalready implements. On Windows an indexer, an antivirus scanner, or a git child that has not fully exited holds a handle, and the unlink or rename is denied outright rather than deferred, failing a clone that would have succeeded a second later. Verified by probing the classifier directly: 8 representative contention messages all returnedfalse, 3 network messages all returnedtrue. The parenthesized SSH formPermission denied (publickey).stays non-transient, becauseisGitAuthErroris consulted first and its pattern requires those parentheses — covered by a new test.2. The tests discarded the reason.
RefreshResultcarries amessageexplaining any failure, but six process-boundary sites asserted only.outcome, so the single observed failure reportedexpected "added", received "failed"and nothing more. All six now assert the result object.3. The 5000ms default sat inside the spread between runners. One measured case costs 0.7s on ubuntu and 5.9s on windows-latest. Two different tests timed out on two consecutive pushes to main —
recovers dirty valid-ID checkouts…andrefreshes a checkout whose paths this platform cannot represent— neither anywhere near hanging.scripts/test.tsnow sets one budget for the process-boundary suite, fixing the class rather than the instance, and supersedes the per-test figure added for the first of the two. 30s still catches a genuine hang; the mocked unit suite keeps the default. Verified the flag takes effect: a 6s test passes with it and fails at 5.03s without.Honest limit: the original failure did not reproduce in 25 consecutive runs, so the contention hypothesis is unconfirmed as its cause. Finding 2 ensures the next occurrence names its own reason.