Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,23 +118,31 @@ Some operations need explicit `isDryRun()` checks:

### Architecture

<!-- lore:019d4479-fd88-7a7d-96b1-2a6b668dfc45 -->
<!-- lore:019cb31a-14ce-7892-b22a-0327cfcebc13 -->

- **Craft post-publish merge is non-blocking housekeeping**: After all publish targets complete, \`handleReleaseBranch()\` in \`src/commands/publish.ts\` merges the release branch back into the default branch and deletes it. This is a housekeeping step — failures are caught, reported to Sentry via \`captureException\`, and logged as warnings without failing the command. The merge uses \`--no-ff\` with the default (ort) strategy first, then retries with \`-s resolve\` if conflicts occur (handles criss-cross ambiguities in files like CHANGELOG.md). An \`isAuthError()\` helper distinguishes authentication failures (expired tokens) from merge conflicts to provide targeted diagnostics.
- **Registry target: repo_url auto-derived from git remote, not user-configurable**: \`repo_url\` in registry manifests is always set by Craft as \`https://github.com/${owner}/${repo}\`. Resolution: (1) explicit \`github: { owner, repo }\` in \`.craft.yml\` (rare), (2) fallback: auto-detect from git \`origin\` remote URL via \`git-url-parse\` library (\`git.ts:194-217\`, \`config.ts:286-316\`). Works with HTTPS and SSH remote URLs. Always overwritten on every publish — existing manifest values are replaced (\`registry.ts:417-418\`). Result is cached globally with \`Object.freeze\`. If remote isn't \`github.com\` and no explicit config exists, throws \`ConfigurationError\`. Most repos need no configuration — the git origin remote is sufficient.

<!-- lore:019cb31a-14c8-7ba9-b1c4-81b2e8bf7e85 -->

- **Registry target: urlTemplate generates artifact download URLs in manifest**: \`urlTemplate\` in the registry target config generates download URLs for release artifacts in the registry manifest's \`files\` field. Uses Mustache rendering with variables \`{{version}}\`, \`{{file}}\`, \`{{revision}}\`. Primarily useful for apps (standalone binaries) and CDN-hosted assets — SDK packages published to public registries (npm, PyPI, gem) typically don't need it. If neither \`urlTemplate\` nor \`checksums\` is configured, Craft skips adding file data entirely (warns at \`registry.ts:341-349\`). Real-world pattern: \`https://downloads.sentry-cdn.com/\<product>/{{version}}/{{file}}\`.

### Gotcha

<!-- lore:019d4479-fd93-7328-a4b5-f9405e4aad8b -->
<!-- lore:019d9a8f-c76e-7716-b1ca-7546635fecc0 -->

- **Craft postReleaseCommand env vars pollute shared bump-version scripts**: Craft's \`runPostReleaseCommand\` sets \`CRAFT_NEW_VERSION=\<released-version>\` in the subprocess env. If a post-release script calls a shared \`bump-version.sh\` that reads \`NEW_VERSION="${CRAFT\_NEW\_VERSION:-${2:-}}"\`, the env var takes precedence over the positional arg (e.g. \`nightly\`), causing the script to set the version to the already-current release version → no diff → no commit → master stays on the release version. Fixed by replacing \`CRAFT_NEW_VERSION\`/\`CRAFT_OLD_VERSION\` with \`CRAFT_RELEASED_VERSION\` in the post-release env (\`publish.ts:563-564\`). The pre-release command (\`prepare.ts\`) still correctly uses \`CRAFT_NEW_VERSION\`. Consuming repos don't need changes unless they explicitly read \`CRAFT_NEW_VERSION\` in their post-release scripts.

<!-- lore:019c9f57-aa0c-7a2a-8a10-911b13b48fc0 -->

- **GitHub App tokens expire after 1 hour — breaks long-running CI publishes**: GitHub App installation tokens expire after 1 hour (non-configurable). For publish jobs exceeding this (e.g., sentry-native's ~1h 23m symbol upload), the token expires before Craft's post-publish \`git push\` for the release branch merge. Git fails with \`could not read Username for 'https://github.com': No such device or address\` — which looks like a credential config issue but is actually token expiration. No code change in Craft alone can fix this — the \`GITHUB_TOKEN\` env var, git \`http.extraheader\`, and Octokit all use the same expired token. The real fix requires the CI workflow (\`getsentry/publish\`) to generate a fresh token after the Docker container exits, before the merge step.
- **ESM modules prevent vi.spyOn of child_process.spawnSync — use test subclass pattern**: In ESM (Vitest or Bun), you cannot \`vi.spyOn\` exports from Node built-in modules — throws 'Module namespace is not configurable'. Workaround: create a test subclass that overrides the method calling the built-in and injects controllable values. \`vi.mock\` at module level works but affects all tests in the file.

<!-- lore:019d4479-fd9a-7a97-931f-8f9a18e5752e -->
<!-- lore:019c9be1-33d1-7b6e-b107-ae7ad42a4ea4 -->

- **prepare-dry-run e2e tests fail without EDITOR in dumb terminals**: The 7 tests in \`src/\_\_tests\_\_/prepare-dry-run.e2e.test.ts\` fail in environments where \`TERM=dumb\` and \`EDITOR\` is unset (e.g., inside agent shells or minimal CI containers). The error is \`Terminal is dumb, but EDITOR unset\` from git commit. This is a pre-existing environment issue, not a code defect. These tests pass in normal CI (Node.js 20/22 runners) where terminal capabilities are available.
- **pnpm overrides with >= can cross major versions — use ^ to constrain**: pnpm overrides gotchas: (1) \`>=\` crosses major versions — use \`^\` to constrain within same major. (2) Version-range selectors don't reliably force re-resolution of compatible transitive deps; use blanket overrides when safe. (3) Overrides become stale — audit with \`pnpm why \<pkg>\` after dependency changes. (4) Never manually resolve pnpm-lock.yaml conflicts — \`git checkout --theirs\` then \`pnpm install\` to regenerate deterministically.

### Pattern

<!-- lore:019d4479-fd97-764e-a7ec-0d32360b0f16 -->
<!-- lore:019d8c2f-ddaf-72f0-96db-44dd54bd56b8 -->

- **Craft Docker container auth: credentials come from volume-mounted git config**: When Craft runs inside Docker in CI (via \`getsentry/craft:latest\`), git authentication comes from \`actions/checkout\`'s \`http.extraheader\` in the local \`.git/config\`, which is volume-mounted into the container at \`/github/workspace\`. The container sets \`HOME=/root\`, so global git config from the host runner isn't available. The \`GITHUB_TOKEN\` env var is passed separately. Craft's \`handleReleaseBranch()\` doesn't set up its own auth — it relies on whatever git config is present. Other targets (registry, commitOnGitRepository) explicitly inject tokens into clone URLs via \`GitHubRemote.getRemoteStringWithAuth()\` or URL manipulation.
- **Craft/Publish release flow: prepare then accept publish issue**: Craft's release flow is two-phase: (1) Run \`release.yml\` GitHub Action with version "auto" — this runs \`craft prepare\`, auto-determines version from commits, creates the \`release/X.Y.Z\` branch, and opens a publish issue on \`getsentry/publish\` repo (e.g. \`publish: getsentry/craft@2.25.3\`). (2) Add the \`accepted\` label to that publish issue to trigger the actual publish pipeline. Do NOT manually create release branches — always use the workflow. The publish issue URL is emitted in the release job logs as a \`::notice::Created publish request:\` line. The publish repo is configured via \`PUBLISH_REPO\` (defaults to \`getsentry/publish\`).
<!-- End lore-managed section -->
6 changes: 2 additions & 4 deletions src/commands/__tests__/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ describe('runPostReleaseCommand', () => {
[pathJoin('scripts', 'post-release.sh'), '', newVersion],
{
env: {
CRAFT_NEW_VERSION: newVersion,
CRAFT_OLD_VERSION: '',
CRAFT_RELEASED_VERSION: newVersion,
PATH: process.env.PATH,
GITHUB_TOKEN: process.env.GITHUB_TOKEN,
HOME: process.env.HOME,
Expand Down Expand Up @@ -75,8 +74,7 @@ describe('runPostReleaseCommand', () => {
['./increase_version.py', 'argument 1', '', newVersion],
{
env: {
CRAFT_NEW_VERSION: newVersion,
CRAFT_OLD_VERSION: '',
CRAFT_RELEASED_VERSION: newVersion,
PATH: process.env.PATH,
GITHUB_TOKEN: process.env.GITHUB_TOKEN,
HOME: process.env.HOME,
Expand Down
3 changes: 1 addition & 2 deletions src/commands/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@
// Pull --rebase failure can leave the repo in an active rebase state
try {
await git.raw(['rebase', '--abort']);
} catch (_abortError) {

Check warning on line 453 in src/commands/publish.ts

View workflow job for this annotation

GitHub Actions / Lint fixes

[@typescript-eslint/no-unused-vars] '_abortError' is defined but never used.
logger.trace('git rebase --abort failed (may be no rebase in progress)');
}
throw pullError;
Expand All @@ -467,7 +467,7 @@
);
try {
await git.merge(['--abort']);
} catch (_abortError) {

Check warning on line 470 in src/commands/publish.ts

View workflow job for this annotation

GitHub Actions / Lint fixes

[@typescript-eslint/no-unused-vars] '_abortError' is defined but never used.
// merge --abort can fail if no merge in progress (e.g. pull failed)
logger.trace('git merge --abort failed (may be no merge in progress)');
}
Expand All @@ -483,19 +483,19 @@
try {
const status = await git.status();
conflictedFiles = status.conflicted;
} catch (_statusError) {

Check warning on line 486 in src/commands/publish.ts

View workflow job for this annotation

GitHub Actions / Lint fixes

[@typescript-eslint/no-unused-vars] '_statusError' is defined but never used.
logger.trace('git status failed while collecting conflict info');
}
if (conflictedFiles.length > 0) {
try {
conflictDiff = await git.diff(conflictedFiles);
} catch (_diffError) {

Check warning on line 492 in src/commands/publish.ts

View workflow job for this annotation

GitHub Actions / Lint fixes

[@typescript-eslint/no-unused-vars] '_diffError' is defined but never used.
logger.trace('git diff failed while collecting conflict diff');
}
}
try {
await git.merge(['--abort']);
} catch (_abortError) {

Check warning on line 498 in src/commands/publish.ts

View workflow job for this annotation

GitHub Actions / Lint fixes

[@typescript-eslint/no-unused-vars] '_abortError' is defined but never used.
logger.trace('git merge --abort failed after resolve strategy');
}
throw new MergeConflictError(
Expand Down Expand Up @@ -561,8 +561,7 @@
logger.info(`Running the post-release command...`);
await spawnProcess(sysCommand as string, args as string[], {
env: {
CRAFT_NEW_VERSION: newVersion,
CRAFT_OLD_VERSION: '',
CRAFT_RELEASED_VERSION: newVersion,
PATH: process.env.PATH,
GITHUB_TOKEN: process.env.GITHUB_TOKEN,
...Object.fromEntries(
Expand Down
Loading