Skip to content

fix(cli): templates pull reuses only a template it pulled, not any path in the way - #7116

Merged
miguel-heygen merged 2 commits into
mainfrom
fix/templates-pull-checks-the-pack
Nov 3, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
fix/templates-pull-checks-the-pack

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

What

hyperframes templates pull <style> no longer reports a template as installed when templates/<style> is only an empty folder or a plain file. An empty folder gets the template; a file, or a folder of someone else's files, stops the pull with one line saying so, and stays as it was.

Why

The pull treated any path at templates/<style> as an installed template: it exited 0 with a checkmark and downloaded nothing, so the agent went on as if the template were there.

Related work

None.

How

  • A pull writes .template-sha256 (the pinned hash it unpacked) into the template just before the template moves in whole, so the record exists only in a complete template.
  • alreadyPulled is now the one place that decides "already installed": a folder whose record matches the pinned hash. An empty folder is let through and filled (removed right before the move-in, so a refused pull leaves it alone). Anything else stops the pull before any request, and nothing of the person's is deleted.
  • A template pulled at an older pinned hash is named as such ("holds an older pull of this template … run the pull again to update"), not as someone else's folder.
  • A template pulled by 0.9.9 has no record, so this pull refuses it ("move it aside and run the pull again"). The pack's own file list isn't known to the CLI, so there's no safe way to tell it from someone else's folder.

No visible change

CLI output only.

Test plan

  • templates.test.ts › "stops at a file or someone else's folder where the template goes, and leaves it as it was". Fails without the fix: on main the pull returns with a checkmark.
  • templates.test.ts › "fills an empty folder where the template goes". Fails without the fix: on main the folder stays empty.
  • templates.test.ts › "names a pull of an older pin as such and leaves it". Fails without the fix.
  • "leaves a template it pulled before alone and spends no call" (replaces the test that used an empty folder as an installed template) and "keeps an empty folder when the pull is refused".
  • templates.test.ts 12 of 12, three runs in a row; with main's templates.ts the four new behavior tests fail. CLI typecheck clean.
  • Comments follow CONTRIBUTING.md "Comments": no comment lines added (comment-ratchet judge: 6 of 193 lines, base 6 of 160).

Size

Two files, about 80 lines with the tests.

@heygengenesis heygengenesis Bot 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.

Request changes at f4d74619. The fix itself is right, but CI is red.

Blocker: the Comments check fails on this head.

RAISED comment share: packages/cli/src/commands/templates.ts is 4.3% (8/187), at the base 3.8% (6/160)
    2 comment line(s) added. Cut them, or move the explanation into a name or a type.

That contradicts the PR body's "4.03% to 4.02%". The two lines are the new JSDoc on pulledHere and the trailing // only ever the empty folder pulledHere let through on the rmdirSync. Cut both, or move what they say into names (for example removeEmptyPlaceholder(dest)). Updating the existing pullTemplate JSDoc is fine as it is.

What I verified, all good:

  • On this head, templates.test.ts passes 11/11. With the parent's templates.ts restored, the three new behaviour tests fail: "fills an empty folder", "stops at a file or someone else's folder", and "keeps an empty folder when the pull is refused". So the "Fails without the fix" lines hold.
  • pulledHere checks before any request. It refuses with a PullRefused and deletes nothing. The empty placeholder is removed only after the tar has unpacked, right before the rename. rmdirSync fails safely (ENOTEMPTY) if someone fills the folder in between.

Non-blocking: when pack.sha256 is bumped, a template this CLI pulled at the old pin has a record that no longer matches. It gets the "is there but is not this template" message, which misreads as someone else's folder. A separate message for "record present, different hash", such as "an older pull of this template; move it aside to update", would read better.

@heygengenesis heygengenesis Bot 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.

Approve at 74010b8b. My blocker from f4d74619 is fixed.

  • Comments: the Comments workflow passes on this head. The two comment lines are gone, and their meaning is now carried by the names alreadyPulled, removeEmptyPlaceholder and PULLED_HASH.
  • Non-blocking note: also addressed. A record with a different hash now gets "holds an older pull of this template". The new test checks that message, checks the folder is left untouched and that no fetch happens.
  • Behavior: unchanged otherwise. templates.test.ts passes 12/12 locally at this head.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit d88d2cc Nov 3, 2026
116 of 117 checks passed
@miguel-heygen
miguel-heygen deleted the fix/templates-pull-checks-the-pack branch November 3, 2026 23:39
This was referenced Nov 4, 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.

1 participant