Skip to content

fix(squad): make Scribe archival incapable of destroying state (#1774, #1783, #1760) - #1792

Merged
bradygaster merged 6 commits into
devfrom
bradygaster-scribe-archival-integrity
Aug 21, 2026
Merged

fix(squad): make Scribe archival incapable of destroying state (#1774, #1783, #1760)#1792
bradygaster merged 6 commits into
devfrom
bradygaster-scribe-archival-integrity

Conversation

@bradygaster

@bradygaster bradygaster commented Aug 21, 2026

Copy link
Copy Markdown
Owner

Archival is a two-half operation — append to a destination, trim from a source. Three production defects came from those halves coming apart, each one silently deleting team history while reporting success.

Closes #1774, closes #1783, closes #1760, closes #1799.

Read this first: what is enforced where

Two different kinds of protection ship here, and they are not interchangeable. Do not read the 439-line tested module as protecting a path it does not run on.

Path How archival happens What protects it here
Scribe agent (STATE_BACKEND: local) An LLM reading charter.md and writing files with ordinary file tools Charter text only. An instruction to a model — disobeyable, exactly the way #1784's binding rule was disobeyable
squad nap / REPL /nap archiveDecisions() in packages/squad-cli/src/cli/core/nap.ts Code. Wired to the SDK module; enforced, not requested

archival.ts has no production caller on the agent path and does not execute during an ordinary Scribe spawn. On the nap path it is load-bearing. Both statements are true and the PR should be read with both in mind.

The most important change: #1799, a live data-loss bug in shipped code

While verifying the above, archiveDecisions() turned out to have all three defects independently of the agent path — in code that is user-invocable today (squad nap at cli-entry.ts:977, REPL /nap at shell/commands.ts:231) and needs no model to misbehave.

Under this repository's actual configuration — .squad/ in .git/info/exclude, no decisions-archive.md yet — it appended every archived record to a brand-new file that can never be committed, while the trim of the tracked decisions.md committed normally.

The new regression test measures that across a real commit boundary. Against pre-fix nap.ts:

AssertionError: expected +0 to be 24
AssertionError: expected '# Decisions\n\n\n' to be '# Decisions\n\n### 2025-07-17: Decisi…'

All 24 decision records destroyed; decisions.md reduced to a bare header.

The five rules and how each is enforced

# Rule Enforced in code Carried in prompt
1 Destination must be committable isTrackedInGit(), resolveTrackedDestination(), isCommittableDestination() — refuse or redirect Rule A
2 Append → verify → then trim archiveEntries() verifies heading containment and count before touching the source; archiveDecisions() verifies the entry count landed Rule B
3 Report entry counts, never bytes formatArchivalReport() refuses to render an unbalanced result Rule C
4 Demote inbox headings on merge prepareInboxBodyForMerge() / demoteHeadings(), fence-aware, clamped at h6 Rule D
5 Never report an unmeasured gate Reports state what was measured Rule E

Rule 1 is deliberately two functions. resolveTrackedDestination() demands an already-tracked path — correct for merging into an existing archive. isCommittableDestination() handles a destination that may not exist yet: it blocks only when the path is untracked and git-ignored, which is the actual #1783 trap. A merely-absent destination still archives, so first-time archival in a normal repo is unaffected.

Why "untracked and ignored" is the precise condition: once .squad/ is excluded, git add -- .squad/file.md fails, so tooling falls back to git add -u, which stages tracked modifications and deletions but silently skips new files. That asymmetry is defect B.

The 29 heading demotions are a structural repair, not a chore

## Context -> #### Context, 29 occurrences, in its own revertible commit.

Those stray H2s sit inside decision records delimited by ### YYYY-MM-DD:. At H2 they outrank their own parent record, so any tool that splits decisions.md on the ### delimiter mis-associates content across record boundaries. That delimiter is exactly what nap.ts uses to decide what to archive, and what record-conservation checks elsewhere rely on. This makes the document honestly parseable.

Kept as a separate commit (b96fe63c) so it can be reverted independently. Verified mechanical: the diff is exactly 29 insertions / 29 deletions, record count 38 and line count 538 both unchanged, and every changed line differs only in its leading # run.

This commit was re-derived from dev after #1782 landed, not carried across the rebase. The original demotion was computed against pre-#1782 content; replaying it would have reinstated records #1782 had archived. The stale commit was dropped and the demotion recomputed, so the resolution is verified by entry count, not by taking either side wholesale: decisions.md 38 records and decisions-archive.md 415 records, both identical to dev. Zero records lost, zero gained.

Test bar: falsifiable, and against the incident shape

Every test asserts artifacts — file contents and entry counts surviving a real git commit — never a routine's own return value. The failure being guarded against is one that reported success for an append that never happened, so a test that trusts the report trusts the thing that lied.

Both suites were mutation-verified — the fix was disabled and the tests confirmed red:

  • Rules 1/2/4 disabled → 8 of 16 red
  • CRLF-safe heading regex reverted → 5 of 21 red, including expected +0 to be 1 where the pre-fix code reported "no archival required" on a CRLF source — reproducing the self-reporting-success shape exactly
  • Lossy splitEntries restored → 2 of 24 red, with a record vanishing outright
  • nap.ts reverted → 3 of 6 red, including total destruction of all 24 records across a commit boundary

Two self-inflicted bugs were caught this way and fixed before merge: a CRLF regex that made the whole fix a silent no-op on Windows (JS . excludes \r, so (.*)$ matches zero headings in a CRLF file — and .squad/decisions.md is CRLF), and a lossy entry split that reordered the document.

Template copies updated — all 11

Missing one regresses on the next squad init.

.squad-templates/ is canonical; scripts/sync-templates.mjs --sync propagates it. Mirrors verified byte-identical by hash, not by eye (scribe-charter.md68131C0C38227131), so any drift would be a sync bug rather than a hand-edit slip.

  • scribe-charter.md × 4 — .squad-templates/, templates/, packages/squad-cli/templates/, packages/squad-sdk/templates/
  • squad.agent.md / .template × 4 — canonical, .github/agents/, and two package mirrors
  • after-agent-reference.md × 2 — a second copy of the Scribe spawn template; missing it would have regressed
  • .squad/agents/scribe/charter.md — the live charter, not covered by sync-templates.mjs, hand-edited

Also updated: decision-hygiene.ts, the watch capability's merge prompt. Note it is watch-only (watch/capabilities/index.ts:31) and does not protect an ordinary Scribe spawn.

On commit ac696ed2

The repo-health security step flagged an unsafe-git pathspec literal appearing in the diff. It hit test fixture prose — sample decision body text whose wording is entirely arbitrary. I reworded the fixture. I did not touch the gate, add an ignore, or relax severity; it passes on its own terms. Flagging it explicitly because "reworded to clear a gate" reads like evasion until someone checks.

Verification

  • npm run lint — exit 0
  • npm run build — exit 0; build-induced version drift restored, not committed
  • 75/75 green across archival.test.ts, nap-archival-safety.test.ts, and the pre-existing nap.test.ts — no regression on existing nap behavior
  • git diff --cached --diff-filter=D --name-only — empty before every commit
  • Branch vs origin/devzero deletions, which for a data-loss PR is the point

Changeset

squad-sdk: minor (new exported surface, no breaks) / squad-cli: minor — the CLI bump moved from patch to minor once nap.ts was wired, because it stops being prose and becomes a behavior change on a user-invocable command.

Tree-mutation guard

Two green suites tonight silently mutated tracked files (#1796 and the template-sync flake), so the blast radius is asserted at this call site rather than assumed:

  • A refusal must write nothinggit status --porcelain is empty afterward. Pre-fix this goes red with expected 'M .squad/decisions.md' to be '', which also exposes why the original loss survived review: the trimmed source shows as modified, but the excluded archive file it created does not appear in git status at all.
  • A normal archive dirties exactly two paths, decisions.md and decisions-archive.md, and nothing else. (This one is a guard, not an incident reproduction — it passes pre-fix. Stating that rather than counting it as red.)

Full pre-fix result at the shipped call site: 4 of 9 red.

Rebased onto current dev

#1782 landed underneath this branch and both changes touch .squad/decisions.md. Resolved by entry count, not by accepting either side: the stale demotion commit was dropped, the demotion re-derived from dev's post-#1782 content, and conservation checked explicitly — decisions.md 38 records, decisions-archive.md 415 records, both identical to dev. Nothing lost, nothing resurrected. Given this PR is about archival destroying records, losing one in a conflict resolution would have been a poor look.

The incident's own evidence survived by accident

While gating the wave, six 2026-08-20 decision records (11,933 bytes) were found existing only in dangling checkpoint commits 0809dcd7 / fc635cc3 — on no branch, one git gc --prune from permanent loss. 0809dcd7 shows decisions.md shrinking 578 lines while decisions-archive.md grows 392: it is the archival incident captured mid-flight.

Worth stating plainly, because it is the argument for this PR. The only reason that evidence exists is that an auto-checkpoint tool unrelated to Squad happened to snapshot the working tree at the right moment. The archival routine itself recorded nothing, reported success, and left no trace of what it had moved. Every rule enforced here exists so the next occurrence cannot depend on that kind of luck.

Copilot AI lite review requested due to automatic review settings August 21, 2026 08:31
@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

🟠 Impact Analysis — PR #1792

Risk tier: 🟠 HIGH

📊 Summary

Metric Count
Files changed 23
Files added 4
Files modified 19
Files deleted 0
Modules touched 7
Critical files 2

🎯 Risk Factors

  • 23 files changed (21-50 → HIGH)
  • 7 modules touched (5-8 → HIGH)
  • Critical files touched: packages/squad-sdk/src/index.ts, packages/squad-sdk/src/state/io/index.ts

📦 Modules Affected

ci-workflows (1 file)
  • .github/agents/squad.agent.md
root (4 files)
  • .changeset/scribe-archival-integrity.md
  • templates/after-agent-reference.md
  • templates/scribe-charter.md
  • templates/squad.agent.md.template
squad-cli (5 files)
  • packages/squad-cli/src/cli/commands/watch/capabilities/decision-hygiene.ts
  • packages/squad-cli/src/cli/core/nap.ts
  • packages/squad-cli/templates/after-agent-reference.md
  • packages/squad-cli/templates/scribe-charter.md
  • packages/squad-cli/templates/squad.agent.md.template
squad-sdk (6 files)
  • packages/squad-sdk/src/index.ts
  • packages/squad-sdk/src/state/io/archival.ts
  • packages/squad-sdk/src/state/io/index.ts
  • packages/squad-sdk/templates/after-agent-reference.md
  • packages/squad-sdk/templates/scribe-charter.md
  • packages/squad-sdk/templates/squad.agent.md.template
squad-state (2 files)
  • .squad/agents/scribe/charter.md
  • .squad/decisions.md
templates (3 files)
  • .squad-templates/after-agent-reference.md
  • .squad-templates/scribe-charter.md
  • .squad-templates/squad.agent.md
tests (2 files)
  • test/cli/nap-archival-safety.test.ts
  • test/state/archival.test.ts

⚠️ Critical Files

  • packages/squad-sdk/src/index.ts
  • packages/squad-sdk/src/state/io/index.ts

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

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

🛫 PR Readiness Check

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

PR Scope: 📦🔧 Mixed (product + infrastructure)

⚠️ 4 item(s) to address before review

Status Check Details
Single commit 6 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 ⚠️ PR includes 2 .squad/ file(s) — ensure these are intentional
No merge conflicts No merge conflicts
Copilot threads resolved 5 unresolved Copilot thread(s) — fix and resolve before merging
CI passing 2 check(s) failing: test, samples-build

Files Changed (23 files, +1884 −91)

File +/−
.changeset/scribe-archival-integrity.md +51 −0
.github/agents/squad.agent.md +11 −4
.squad-templates/after-agent-reference.md +24 −4
.squad-templates/scribe-charter.md +59 −2
.squad-templates/squad.agent.md +11 −4
.squad/agents/scribe/charter.md +37 −1
.squad/decisions.md +29 −29
packages/squad-cli/src/cli/commands/watch/capabilities/decision-hygiene.ts +8 −1
packages/squad-cli/src/cli/core/nap.ts +72 −16
packages/squad-cli/templates/after-agent-reference.md +24 −4
packages/squad-cli/templates/scribe-charter.md +59 −2
packages/squad-cli/templates/squad.agent.md.template +11 −4
packages/squad-sdk/src/index.ts +23 −0
packages/squad-sdk/src/state/io/archival.ts +484 −0
packages/squad-sdk/src/state/io/index.ts +23 −0
packages/squad-sdk/templates/after-agent-reference.md +24 −4
packages/squad-sdk/templates/scribe-charter.md +59 −2
packages/squad-sdk/templates/squad.agent.md.template +11 −4
templates/after-agent-reference.md +24 −4
templates/scribe-charter.md +59 −2
templates/squad.agent.md.template +11 −4
test/cli/nap-archival-safety.test.ts +284 −0
test/state/archival.test.ts +486 −0

Total: +1884 −91


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

⚠️ Squad File Leakage Detected

The following .squad/ files were modified in this PR:

  • .squad/agents/scribe/charter.md
  • .squad/decisions.md

These files affect team routing, agent charters, and decisions.
If intentional, ensure approval from the team lead.

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.

Pull request overview

This PR adds deterministic safeguards for Scribe archival and decision merging, updates synchronized guidance, cleans decision headings, and adds regression tests.

Changes:

  • Adds tracked-destination checks, append/verify/trim behavior, and CRLF/fence-aware parsing.
  • Updates Scribe templates and agent guidance.
  • Adds 24 tests, cleans decision headings, and includes release metadata.
Show a summary per file
File Summary
test/state/archival.test.ts Adds archival, parsing, CRLF, fencing, and hierarchy tests.
templates/squad.agent.md.template Updates Scribe spawn guidance.
templates/scribe-charter.md Updates Scribe charter guidance.
templates/after-agent-reference.md Updates archival reference guidance.
packages/squad-sdk/templates/squad.agent.md.template Updates SDK spawn guidance.
packages/squad-sdk/templates/scribe-charter.md Updates SDK Scribe charter.
packages/squad-sdk/templates/after-agent-reference.md Updates SDK archival guidance.
packages/squad-sdk/src/state/io/index.ts Exports helpers only from the internal barrel; public SDK export remains unresolved.
packages/squad-sdk/src/state/io/archival.ts Implements archival helpers; backend handling, literal Git pathspecs, and concurrent source updates remain unresolved.
packages/squad-cli/templates/squad.agent.md.template Updates CLI spawn guidance.
packages/squad-cli/templates/scribe-charter.md Updates CLI Scribe charter.
packages/squad-cli/templates/after-agent-reference.md Updates CLI archival guidance.
packages/squad-cli/src/cli/commands/watch/capabilities/decision-hygiene.ts Updates prompt rules, but active archival paths are not wired to the deterministic helpers.
.squad/decisions.md Cleans decision heading hierarchy.
.squad/agents/scribe/charter.md Updates the live Scribe charter.
.squad-templates/squad.agent.md Updates the canonical spawn template.
.squad-templates/scribe-charter.md Updates the canonical Scribe charter.
.squad-templates/after-agent-reference.md Updates the canonical archival reference.
.github/agents/squad.agent.md Updates GitHub agent guidance.
.changeset/scribe-archival-integrity.md Documents SDK and CLI release changes.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (14)

packages/squad-sdk/src/state/io/archival.ts:381

  • Verification accepts any destination content containing the selected heading strings and the expected number of level-3 headings. An I/O implementation that writes only headings (or truncates entry bodies) therefore passes, then line 403 removes the full entries from the source and silently loses their bodies. Verify each selected entry's full content, with explicit EOL normalization, or compare the exact appended slice before trimming.
  // Rule 2, step 2 — verify by re-reading. Every heading must literally be
  // present and the entry count must have grown by exactly what we appended.
  const after = io.readFile(destination);
  const afterHeadings = new Set(extractHeadings(after, level));
  const missing = headings.filter((h) => !afterHeadings.has(h));

packages/squad-sdk/src/state/io/archival.ts:348

  • The tracked check does not reject sourcePath === destination (including when the fallback resolves to the source). In that case the append succeeds and verification passes against the source, but the later writeFile rebuilds that same file from kept entries and erases the appended archived entries. Reject identical canonical source/destination paths after resolution, including symlink aliases, before reading or writing.
  const { destination, redirected } = resolveTrackedDestination({
    destination: destinationPath,
    repoRoot,
    fallbackDestination,
    git,

packages/squad-sdk/src/state/io/archival.ts:152

  • scanLines stores only the fence character, so any same-character fence closes an opener regardless of length. A valid four-backtick block containing a three-backtick line will be closed early, and later headings in that still-open block will be treated as real entries or demoted. Track the opening fence length and close only on a same-character run at least that long, with valid closing-fence syntax.
      if (marker === fence) {
        fence = null;

packages/squad-sdk/src/state/io/archival.ts:399

  • select is evaluated again during trimming instead of removing the entries selected in the first pass. A time-dependent or stateful predicate can select A for the append and B for the trim, causing B to be deleted without being archived. Derive kept from the first-pass selection snapshot.
  const kept = entries.filter((e) => !select(e));

packages/squad-sdk/src/state/io/archival.ts:374

  • replace(/\s+$/, '') strips all trailing whitespace from each entry. Markdown uses two trailing spaces for a hard line break, and the real .squad/decisions.md contains such lines (for example around decisions.md:173). Archiving therefore changes entry content and semantics even when the append succeeds. Remove only separator line endings that reassembly intentionally normalizes, preserving other trailing whitespace.
    `${selected.map((e) => e.text.replace(/\s+$/, '')).join(destEol + destEol)}${destEol}`;

packages/squad-sdk/src/state/io/archival.ts:403

  • After destination verification, the default writeFileSync truncates the source in place and the result is never re-read. A disk-full/crash/partial write can leave kept entries missing while this function returns a balanced success; the destination check does not protect the unarchived history. Use an atomic same-directory temp-file/rename and verify the post-trim source against the precomputed kept content before returning.
  io.writeFile(sourcePath, `${rebuilt}${srcEol}`);

packages/squad-sdk/src/state/io/archival.ts:425

  • formatArchivalReport only checks arithmetic equality on a caller-supplied structural object. Any caller can pass balanced fabricated counts (including 0/0) and it will render a report claiming the result was measured, so Rule 5 is not enforced by this API. Make the verified result opaque/branded or have reporting consume a result that can only be produced by the archival operation.
export function formatArchivalReport(result: ArchivalResult, repoRoot?: string): string {
  if (result.removedFromSource !== result.addedToDestination) {
    throw new ArchiveVerificationError(
      `Refusing to report an unbalanced archival: ${result.removedFromSource} removed ` +
        `from source vs ${result.addedToDestination} added to destination.`,

packages/squad-sdk/src/state/io/archival.ts:402

  • The source rebuild also uses \s+, so a kept entry or the preamble can lose trailing spaces/tabs even when no archival content is removed. In Markdown those spaces can encode a hard line break, and this makes an otherwise unrelated source section change. Limit this normalization to trailing line-ending sequences, consistent with the archive payload.
  const rebuilt = [preamble.replace(/\s+$/, ''), ...kept.map((e) => e.text.replace(/\s+$/, ''))]
    .filter((part) => part.length > 0)
    .join(srcEol + srcEol);

packages/squad-sdk/src/state/io/archival.ts:410

  • removedFromSource is populated from selected.length without re-reading sourcePath after io.writeFile. The injected I/O contract explicitly permits a write to be accepted without persistence (the test uses that for append), so the same failure on the trim would return a balanced success/report while leaving the source unchanged. Re-read and verify the source entry-count/heading removal before returning; otherwise this result is not measured.
  io.writeFile(sourcePath, `${rebuilt}${srcEol}`);

  return {
    removedFromSource: selected.length,
    addedToDestination: added,
    headings,
    destination,
    redirected,

packages/squad-sdk/src/state/io/archival.ts:109

  • Git tracks a symlink entry, but appendFile follows it. A tracked destination symlink can therefore point to an untracked or out-of-repository target; this check returns true, the target receives the archive, and the tracked source can still be trimmed and committed without the content. Reject symlink destinations or validate the resolved regular-file target before allowing the append.
  if (isTrackedInGit(destination, repoRoot, git)) {
    return { destination, redirected: false };
  }

  if (fallbackDestination && isTrackedInGit(fallbackDestination, repoRoot, git)) {

packages/squad-sdk/src/state/io/archival.ts:374

  • The source entry text is appended with its original internal line endings while the separators use destEol. Archiving an LF source into a CRLF archive (or the reverse) therefore creates mixed line endings in the destination, despite the stated EOL-preservation goal. Normalize each entry to the destination EOL before appending.
  const payload =
    `${before.endsWith('\n') || before === '' ? '' : destEol}${destEol}` +
    `${selected.map((e) => e.text.replace(/\s+$/, '')).join(destEol + destEol)}${destEol}`;

packages/squad-sdk/src/state/io/archival.ts:272

  • detectEol() is documented as preserving the dominant line ending, but it returns CRLF whenever the document contains even one CRLF. A mostly-LF mixed document will therefore be rewritten to CRLF by the rebuild, changing unrelated lines and creating the whole-file diff this logic is meant to avoid. Count CRLF versus bare LF (or preserve each line's original ending) and cover mixed-EOL input.
function detectEol(markdown: string): string {
  return markdown.includes('\r\n') ? '\r\n' : '\n';

packages/squad-sdk/src/state/io/archival.ts:254

  • Using every ### as an entry boundary is not sufficient for the checked-in decisions format. .squad/decisions.md contains same-level section headings such as ### Layer A under ### 2026-07-27: Dispatch Enforcement (decisions.md:729-739) and ### Option A under the dated workflow entry (decisions.md:429-455). An age predicate selecting the dated heading therefore archives only its prefix; the remaining body is left as orphan entries in the source and is absent from the archive. Distinguish actual dated decision boundaries or normalize same-level subsections before archival, and add a test that archives a real entry with such a body.
    if (line.headingLevel === level) {
      if (current) entries.push({ heading: current.heading, text: current.lines.join('\n') });
      current = { heading: line.text.trim(), lines: [line.text] };

test/state/archival.test.ts:98

  • This centerpiece does not exercise a production archival path: it manually writes an untracked file and manually rewrites the source. It therefore proves only Git's git add -u behavior and can pass while runNap and the watch merge path remain unchanged; the new archiveEntries tests call an otherwise unused helper directly. Add an excluded-.squad test through the real mutation path so the stated pre-fix-failing guarantee protects shipped behavior.
    it('reproduces the pre-fix data loss across a real commit boundary', () => {
      // Pre-fix behavior, replayed literally: append to a NEW timestamped file,
      // then trim the source. No tracked-destination check anywhere.
      const untracked = path.join(squad, 'decisions', 'archive', '2026-08-20-archived-pre7d.md');
      mkdirSync(path.dirname(untracked), { recursive: true });
  • Files reviewed: 20/20 changed files
  • Comments generated: 5
  • Review effort level: Lite

Comment on lines +52 to +55
'Delete an inbox file only after confirming its content is literally present in ' +
'decisions.md. If you archive anything out of decisions.md: verify the destination is ' +
'git-tracked first (git ls-files --error-unmatch), append and verify before trimming, ' +
'and report entry counts moved rather than file sizes.';
Comment on lines +303 to +307
io?: {
readFile: (p: string) => string;
appendFile: (p: string, data: string) => void;
writeFile: (p: string, data: string) => void;
exists: (p: string) => boolean;
repoRoot: string,
git: GitRunner = defaultGitRunner,
): boolean {
return git(['ls-files', '--error-unmatch', '--', filePath], repoRoot) === 0;
Comment on lines +351 to +352
const sourceMarkdown = io.readFile(sourcePath);
const { preamble, entries } = splitEntries(sourceMarkdown, level);
Comment on lines +30 to +34
// Archival integrity
export {
archiveEntries,
countEntries,
demoteHeadings,
@github-actions

Copy link
Copy Markdown
Contributor

🏗️ Architectural Review

⚠️ Architectural review: 3 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/nap.ts
🟡 warning export-surface Package entry point(s) modified with 21 new/changed export(s). New public API surface requires careful review for backward compatibility. packages/squad-sdk/src/index.ts
🟡 warning sweeping-refactor This PR touches 23 files (23 modified/added, 0 deleted). Large PRs are harder to review — consider splitting if possible.

Automated architectural review — informational only.

brady gaster and others added 6 commits August 21, 2026 02:08
Archival is a two-half operation - append to a destination, trim from a
source. Three defects came from those halves coming apart, each silently
destroying team history while reporting success.

Adds packages/squad-sdk/src/state/io/archival.ts enforcing five rules in
code, not only in prompt text:

1. Destination must be git-tracked. resolveTrackedDestination() runs
   `git ls-files --error-unmatch` before any write and either redirects to a
   tracked fallback or aborts. Under a git-excluded .squad/, tracked files
   still commit while brand-new files silently never do, so the trim
   commits and the destination never does.
2. Append, verify, then trim. archiveEntries() verifies by literal heading
   containment AND entry count before touching the source. A failed append
   leaves the source completely intact.
3. Report entry counts, never bytes. formatArchivalReport() refuses to
   render an unbalanced result. Size is not an integrity signal: a merge
   and an archive in the same pass move it in opposite directions.
4. Demote inbox headings on merge. prepareInboxBodyForMerge() shifts a
   body so its shallowest heading lands at h4, fence-aware so `#` lines
   inside code samples are never rewritten.
5. Never report an unmeasured gate outcome.

Scribe's charter, the Scribe spawn template, and the decision-hygiene
watch prompt carry the same rules. Templates edited in .squad-templates/
and propagated with scripts/sync-templates.mjs.

Tests are behavioral, not text-presence assertions: a real git repo with
.squad/ in .git/info/exclude reproduces the pre-fix loss across a commit
boundary, then asserts the fixed path loses nothing. Verified load-bearing
by mutation - disabling the three code-enforced rules turns 8 of 16 red.

Closes #1774
Closes #1783
Closes #1760

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 70370e36-33b0-4786-bd72-4cf15518daa6
Follow-up to f4ef11e, found while measuring the stray-H2 backlog for #1760.
Two defects in the new archival module, both the same class of bug the module
exists to prevent.

1. CRLF. HEADING_RE ended in `(.*)$`, and JS's `.` does not match `\r`, so it
   matched ZERO headings in a CRLF document. `.squad/decisions.md` is CRLF
   (892 CRLF pairs / 893 lines) on Windows, where this repo is developed.
   The failure was silent and total: heading demotion became a no-op, append
   verification passed having checked nothing, and archiveEntries found no
   entries and returned 0/0 -- reproducing the exact false "no archival
   required" gate report from #1783. The line ending is now captured, not
   matched, and is preserved when a heading is rebuilt.

2. Lossless split. splitEntries treated a heading shallower than `###` as
   closing the entry. `.squad/decisions.md` carries 60 such stray `##`/`#`
   headings spliced under entries (#1760, measured), so the rest of each entry
   was re-homed into the preamble and the rebuild reordered the document.
   Boundaries are now drawn only at `###`, making the split lossless by
   construction.

Also preserves the document's dominant line ending on append and rebuild, so
archiving a CRLF file no longer renders as a whole-file rewrite.

Tests (24 total, +8): all 5 CRLF tests and both losslessness tests fail against
the pre-fix code, verified by mutation. The losslessness test round-trips the
real `.squad/decisions.md` byte-for-byte.

Refs #1774
Refs #1783
Refs #1760

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 70370e36-33b0-4786-bd72-4cf15518daa6
The Security Review gate flags the literal unsafe-git pathspec anywhere in a
diff, including inside a test fixture. The fixture body was sample decision
prose, so its wording is arbitrary -- reword it rather than weaken the gate.

Refs #1774

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 70370e36-33b0-4786-bd72-4cf15518daa6
`archiveDecisions()` in the CLI is a second, shipped, user-invocable
archival path (`squad nap`, REPL `/nap`) that had all three archival
defects independently of the agent/charter path — and needs no model
to misbehave to lose data.

Under this repo's actual configuration (`.squad/` in `.git/info/exclude`)
with no archive file yet, it appended every archived record to a
brand-new `.squad/decisions-archive.md` that can never be committed,
while the trim of the tracked `decisions.md` committed normally. A new
regression test measures the result across a real commit boundary and
pre-fix reports `expected +0 to be 24` — total loss.

Wires it to the SDK archival module:

- `isCommittableDestination()` refuses to archive into a destination
  that is untracked *and* git-ignored, leaving the source intact. A
  merely-absent destination still archives, so first-time archival in
  a normal repo is unaffected.
- Append, verify the entry count landed, and only then trim. The old
  code wrote the trimmed source outside the `if (archiveContent.trim())`
  guard, so the skip branch deleted without appending.
- `findHeadingLineIndices()` gives fence-aware record boundaries, so a
  `###` line inside a fenced code sample is no longer treated as a
  record boundary. Shared with the SDK path so the two cannot drift.
- Reports entry counts rather than the byte delta that reported success
  for an append that never happened.

Tests assert the artifact — file contents and counts surviving a real
`git commit` — never the routine's own return value, since the report
is the thing that lied.

Closes #1799
Refs #1774, #1783, #1760

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 70370e36-33b0-4786-bd72-4cf15518daa6
Two green suites tonight silently mutated tracked files, so assert the
blast radius at the archival call site rather than assuming it.

- A refusal must write NOTHING. Pre-fix this path trimmed the tracked
  decisions.md while creating an excluded archive file that git status
  does not show, so the loss was invisible to review. Goes red pre-fix
  with `expected 'M .squad/decisions.md' to be ''`.
- A normal archive must dirty exactly decisions.md and
  decisions-archive.md, and nothing else.

Refs #1799

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 70370e36-33b0-4786-bd72-4cf15518daa6
Structural repair, not cosmetics. These `## Context` / `## Decision` /
`## Consequences` sections sit INSIDE decision records delimited by
`### YYYY-MM-DD:`. At h2 they outrank their own parent record, so any
tool that splits decisions.md on the `###` delimiter mis-associates
content across record boundaries — including `archiveDecisions()`,
which uses exactly that delimiter to decide what to archive.

Re-derived from dev after #1782 landed rather than carried over from
the pre-#1782 tree, so it demotes the 29 strays that actually remain
rather than reinstating records #1782 archived.

Mechanical and verified: record count 38 unchanged, line count 538
unchanged, and every changed line differs only in its leading `#` run.
Diff is exactly 29 insertions / 29 deletions. Kept as its own commit so
it can be reverted independently of the code fix.

Refs #1760

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 70370e36-33b0-4786-bd72-4cf15518daa6
@bradygaster
bradygaster force-pushed the bradygaster-scribe-archival-integrity branch from 308eaa7 to b96fe63 Compare August 21, 2026 09:14
@bradygaster
bradygaster merged commit 911df2a into dev Aug 21, 2026
24 of 26 checks passed
@bradygaster
bradygaster deleted the bradygaster-scribe-archival-integrity branch September 9, 2026 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants