Skip to content

fix: tolerate CRLF casting manifests - #2269

Merged
tamirdresher merged 3 commits into
github:devfrom
tamirdresher:fix/casting-crlf-manifest-2266
Oct 28, 2026
Merged

tamirdresher merged 3 commits into
github:devfrom
tamirdresher:fix/casting-crlf-manifest-2266

Conversation

@tamirdresher

Copy link
Copy Markdown
Collaborator

Summary

  • accept exact or CRLF-to-LF-normalized casting hashes during manifest validation and transaction recovery
  • pin .squad/casting/*.json to LF during init and upgrade, and normalize existing casting JSON during upgrade
  • add regression coverage for SDK validation/preflight, squad doctor, and squad upgrade

Validation

  • SKIP_BUILD_BUMP=1 npm run build
  • npx vitest run --no-file-parallelism --testTimeout=30000 --hookTimeout=30000 test/casting-durable-registry.test.ts test/casting-legacy-migration.test.ts test/cli/doctor.test.ts test/cli/upgrade.test.ts test/cli/init-upgrade-parity.test.ts test/cli/init.test.ts test/init.test.ts (300 passed)
  • node --test test/*.test.cjs (134 passed)
  • OneDrive migration and managed-repair CRLF smoke tests

Fixes #2266

Copilot AI balanced review requested due to automatic review settings October 21, 2026 07:23
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

🟡 Impact Analysis — PR #2269

Risk tier: 🟡 MEDIUM

📊 Summary

Metric Count
Files changed 12
Files added 1
Files modified 11
Files deleted 0
Modules touched 4
Critical files 1

🎯 Risk Factors

  • 12 files changed (6-20 → MEDIUM)
  • 4 modules touched (2-4 → MEDIUM)
  • Critical files touched: packages/squad-sdk/src/casting/index.ts

📦 Modules Affected

root (2 files)
  • .changeset/casting-crlf-manifest-tolerance.md
  • .gitattributes
squad-cli (1 file)
  • packages/squad-cli/src/cli/core/upgrade.ts
squad-sdk (3 files)
  • packages/squad-sdk/src/casting/durable-registry.ts
  • packages/squad-sdk/src/casting/index.ts
  • packages/squad-sdk/src/config/init.ts
tests (6 files)
  • test/casting-durable-registry.test.ts
  • test/cli/doctor.test.ts
  • test/cli/init-upgrade-parity.test.ts
  • test/cli/init.test.ts
  • test/cli/upgrade.test.ts
  • test/init.test.ts

⚠️ Critical Files

  • packages/squad-sdk/src/casting/index.ts

This report is generated automatically for every PR. See #733 for details.

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

🛫 PR Readiness Check

ℹ️ This comment updates on each push. Last checked: commit cec549a

PR Scope: 📦🔧 Mixed (product + infrastructure)

⚠️ 3 item(s) to address before review

Status Check Details
❌ Single commit 3 commits — consider squashing before review
✅ Not in draft Ready for review
✅ Branch up to date Up to date with dev
❌ Copilot review No Copilot review yet — it may still be processing
✅ Changeset present Changeset file found
✅ Scope clean No .squad/ or docs/proposals/ files
✅ No merge conflicts No merge conflicts
✅ Copilot threads resolved 0 active Copilot thread(s) resolved (2 outdated skipped)
❌ CI passing 15 check(s) still running

Files Changed (12 files, +199 −19)

File +/−
.changeset/casting-crlf-manifest-tolerance.md +6 −0
.gitattributes +3 −0
packages/squad-cli/src/cli/core/upgrade.ts +14 −1
packages/squad-sdk/src/casting/durable-registry.ts +54 −11
packages/squad-sdk/src/casting/index.ts +1 −0
packages/squad-sdk/src/config/init.ts +6 −6
test/casting-durable-registry.test.ts +73 −0
test/cli/doctor.test.ts +18 −1
test/cli/init-upgrade-parity.test.ts +1 −0
test/cli/init.test.ts +1 −0
test/cli/upgrade.test.ts +21 −0
test/init.test.ts +1 −0

Total: +199 −19


This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.

@github-actions

Copy link
Copy Markdown
Contributor

🏗️ Architectural Review

Authoritative evidence: base-controlled pull_request_target reporter.

⚠️ Architectural review: 1 warning(s).

Severity Category Finding Files
🟡 warning bootstrap-area 1 file(s) in the bootstrap area (packages/squad-cli/src/cli/core/) were modified. These files must maintain zero external dependencies. Review carefully. packages/squad-cli/src/cli/core/upgrade.ts

Automated architectural review — informational only.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Normalization bypasses shared writer locking and atomic replacement, risking lost updates and unrecoverable casting files.

2 open issues
What changed in this PR

Addresses #2266 by tolerating CRLF casting files in manifest validation and adding LF normalization during upgrade.

Changes:

  • Accept exact or LF-normalized hashes during validation and recovery.
  • Pin casting JSON to LF during initialization and upgrade.
  • Add SDK and CLI regression coverage.
File Description
test/​init.test.ts Checks SDK initialization’s LF rule.
test/​cli/​upgrade.test.ts Covers CRLF normalization during upgrade.
test/​cli/​init.test.ts Checks CLI initialization’s LF rule.
test/​cli/​init-upgrade-parity.test.ts Includes LF rule in parity checks.
test/​cli/​doctor.test.ts Covers CRLF manifest validation.
test/​casting-durable-registry.test.ts Covers CRLF validation, preflight, and reads.
packages/​squad-sdk/​src/​config/​init.ts Adds casting LF attributes.
packages/​squad-sdk/​src/​casting/​durable-registry.ts Accepts CRLF-normalized hashes.
packages/​squad-cli/​src/​cli/​core/​upgrade.ts Adds LF attributes and normalization.
.gitattributes Pins repository casting JSON to LF.
.changeset/​casting-crlf-manifest-tolerance.md Declares SDK and CLI patch releases.

Review effort: Balanced

@tamirdresher

Copy link
Copy Markdown
Collaborator Author

Rebased and manually folded onto dev target 75b15cf4 (#2303 + #2305). New head: 4a5bcfb4213195e9465e16ab5b34105867035ae1.

Conflict resolution kept #2303's upgrade contract intact: plan → verified backup → overwrite, byte-preserved team.md, CRLF-idempotent managed templates including compound .example files, and explicit roster opt-in. I dropped #2269's superseded whole-file .gitattributes update behavior and retained #2303's dry-run-aware, EOL-preserving append helper; only the casting LF rule and normalization behavior were folded in.

The two Copilot findings are fixed separately in 4a5bcfb4:

  • normalization holds the shared casting writer lock across recovery, validation, fresh reads, and the full pass;
  • each JSON file uses flushed temp-file + atomic rename + parent fsync, with fault-injection coverage proving the live file remains intact on failure.

Validation:

  • npm run build (SKIP_BUILD_BUMP=1) ✅
  • npm run lint ✅
  • casting / upgrade / upgrade-idempotent / preview2 CLI fixes / health suites: 263 passed, 1 expected pending, 0 failed (serial, 30s loaded-host timeout)
  • core.autocrlf=true temp checkout: all 4 manifest-backed casting JSON files converted to CRLF; squad doctor reported the pair consistent; squad upgrade --dry-run exited 0 with byte/hash snapshot and git status unchanged ✅

Force-push used an exact lease on prior remote tip 46f6ea07bcc8a4229b11e411ad62c13b1dab0655.

tamirdresher and others added 2 commits October 28, 2026 07:05
Normalize CRLF worktree content when verifying casting hashes and pin casting JSON files to LF during init and upgrade.

Closes github#2266

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 58833b4b-f8e7-42b8-bf04-9121f362f68c
Hold the shared casting writer lock across recovery, validation, reread, and LF normalization. Replace each JSON file through a flushed sibling temporary file and atomic rename.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 995eea17-c356-4912-90ea-5e5b144e7ba9
@tamirdresher
tamirdresher force-pushed the fix/casting-crlf-manifest-2266 branch from 41d842d to 3ac4484 Compare October 28, 2026 05:15
@tamirdresher

Copy link
Copy Markdown
Collaborator Author

Rebased the previously reviewed 4a5bcfb4 series onto dev 2aa2cc41b4d8c539fb7725de9b3bbdeb656d3680. New head: 3ac448423f9e35eec612033cc72f6db7ca5a5df1.

The two commits replayed cleanly. The earlier #2303-compatible resolution remains intact: upgrade still follows the current plan / verified-backup / overwrite flow, preserves user-owned team.md, uses the dry-run-aware EOL-preserving .gitattributes append path, and adds only the casting LF rule plus atomic locked normalization. The SDK CRLF hash-tolerance and recovery behavior are unchanged.

Validation:

  • build (SKIP_BUILD_BUMP=1) passed;
  • TypeScript lint passed;
  • casting / doctor / init / upgrade suites: 534 passed, 43 expected skips, 0 failed across 22 files.

Force-push used an exact lease on the live prior remote tip 41d842d1d5e403ee780abd0a6158e5fda2287c37.

@tamirdresher
tamirdresher merged commit 0aeb071 into github:dev Oct 28, 2026
23 checks passed
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.

Upgrade to 1.1.0 fails with error

2 participants