Merge pull request #7920 from deepseek-harness/turtle/guard-stale-upgrade-guide-e6ab3b67a5c6

ci(upgrade-guide): reject guides added under a released version directory

.agents/skills/dsh-create-upgrade-guide/SKILL.md

@@ -29,7 +29,7 @@ Pre-stable internal APIs whose consumers are all updated in the same change need
 
 ## Maintenance
 
-While the root version is unchanged, the current version directory describes the delta from vT to current `master`. When a later change touches a surface a guide there covers, update that guide in the same change; delete the guide when the break is reverted. After the version bump, guides in older version directories are frozen; a new break goes into the new version directory. Never write a guide spanning more than one release.
+While the root version is unchanged, the current version directory describes the delta from vT to current `master`. When a later change touches a surface a guide there covers, update that guide in the same change; delete the guide when the break is reverted. After the version bump, guides in older version directories are frozen; a new break goes into the new version directory. When a release lands while your pull request is open, move its guides to the new version directory: `Policy / Pre-Merge Check` runs `verify-upgrade-guides --base` on the merged tree and rejects a guide added outside the root version's directory unless the `release/dsh-<x.y.z>` branch of that guide's version line already contains it. Never write a guide spanning more than one release.
 
 ## Format
 

.github/AGENTS.md

@@ -4,6 +4,6 @@ Run jobs on Windows runners (`windows-*` labels) under native `pwsh`. Native Win
 
 The manual [Windows packaging workflow](workflows/windows-package.yml) uses the dedicated self-hosted Windows x64 label `dsh-win-package-trial` under native `pwsh`; preserve this routing separately from the automatic CI lanes.
 
-The merge checks run from three workflows named `Policy`, which run on `pull_request_target` and cover pull requests into master or a `release/dsh-x.y.z` branch, so their definitions come from the base branch, and call the `deepseek-harness-check` action directly: `pr-check-on-head-branch.yml` on pull-request updates gives `Policy / Master Change` and `Policy / Pre-Merge Check`, `pr-check-after-approve.yml` on pull-request updates and submitted or dismissed reviews gives `Policy / Approve Check`, which also publishes the review verdict as the check run `Policy / Reviewer Check`, and `pr-check-on-base-branch.yml` on pushes to master or a release branch re-runs the head-branch checks as `Policy / Recheck`. Branch rules require those check names, so keep the job names and the workflow name. Each job reads its mode from a repository variable: `MERGE_POLICY_MASTER_DIFF`, `MERGE_POLICY_PRE_MERGE`, and `MERGE_POLICY_HEAD_DIFF`, each `off` (skip), `report` (run without failing the job), or `block` (fail the job); a missing value or an unknown one counts as `report`. `MERGE_POLICY_HEAD_DIFF` decides whether `Policy / Approve Check` runs and fails; while it runs, `Policy / Reviewer Check` is published whatever that mode says, with `success` when every file that needs review has one and `action_required` with the reviewers to ask otherwise. `Policy / Pre-Merge Check` merges the commit `Policy / Master Change` reports, so it runs only after that job and stays off while `MERGE_POLICY_MASTER_DIFF` is `off`; a merge it cannot do and a build that fails both follow its own mode, so `report` warns and skips the build while `block` fails the job. `Policy / Pre-Merge Check` runs `scripts/pre-merge-check.ts` under plain Node and knows nothing else about the merged tree: that script checks the released manifests against the root version, installs, and builds the Host and Client contracts, so change it in one pull request together with the build it orchestrates.
+The merge checks run from three workflows named `Policy`, which run on `pull_request_target` and cover pull requests into master or a `release/dsh-x.y.z` branch, so their definitions come from the base branch, and call the `deepseek-harness-check` action directly: `pr-check-on-head-branch.yml` on pull-request updates gives `Policy / Master Change` and `Policy / Pre-Merge Check`, `pr-check-after-approve.yml` on pull-request updates and submitted or dismissed reviews gives `Policy / Approve Check`, which also publishes the review verdict as the check run `Policy / Reviewer Check`, and `pr-check-on-base-branch.yml` on pushes to master or a release branch re-runs the head-branch checks as `Policy / Recheck`. Branch rules require those check names, so keep the job names and the workflow name. Each job reads its mode from a repository variable: `MERGE_POLICY_MASTER_DIFF`, `MERGE_POLICY_PRE_MERGE`, and `MERGE_POLICY_HEAD_DIFF`, each `off` (skip), `report` (run without failing the job), or `block` (fail the job); a missing value or an unknown one counts as `report`. `MERGE_POLICY_HEAD_DIFF` decides whether `Policy / Approve Check` runs and fails; while it runs, `Policy / Reviewer Check` is published whatever that mode says, with `success` when every file that needs review has one and `action_required` with the reviewers to ask otherwise. `Policy / Pre-Merge Check` merges the commit `Policy / Master Change` reports, so it runs only after that job and stays off while `MERGE_POLICY_MASTER_DIFF` is `off`; a merge it cannot do and a build that fails both follow its own mode, so `report` warns and skips the build while `block` fails the job. `Policy / Pre-Merge Check` runs `scripts/pre-merge-check.ts` under plain Node with the merged base commit in `BASE_SHA` and knows nothing else about the merged tree: that script checks the released manifests against the root version, installs, rejects upgrade guides added outside the root version's directory, and builds the Host and Client contracts, so change it in one pull request together with the build it orchestrates.
 
 The manual Desktop publication workflow selects the original successful `package` job attempt independently of `upload-artifact` archival status, then routes each retained build through `DESKTOP_PUBLISH_RUNNERS` to its original machine's unique label. Keep Windows run steps on native PowerShell and Mac run steps on the default POSIX shell; shared entrypoints are Node scripts. Test and production jobs use separate `desktop-test` and `desktop-production` approval Environments after a credential-free preparation job.

.github/workflows/pr-check-on-head-branch.yml

@@ -108,6 +108,7 @@ jobs:
         if: steps.merge.outputs.merged == 'true'
         env:
           CHECK_MODE: ${{ vars.MERGE_POLICY_PRE_MERGE }}
+          BASE_SHA: ${{ needs.master.outputs.base_sha }}
         run: |
           # The check names one file and knows nothing else about how the merged tree builds itself.
           if [ ! -f scripts/pre-merge-check.ts ]; then

scripts/pre-merge-check.spec.ts

@@ -42,12 +42,14 @@ describe('pre-merge check', () => {
     }
   })
 
-  it('checks the versions before it installs, and installs before the builds', () => {
-    const steps = preMergeSteps('/repo')
-    expect(steps.map(step => step.name)).toEqual(['versions', 'install', 'host contracts', 'client'])
+  it('checks the versions before it installs, and installs before the upgrade guides, which need the merged base commit, and the builds', () => {
+    expect(preMergeSteps('/repo').map(step => step.name)).toEqual(['versions', 'install', 'host contracts', 'client'])
+    const steps = preMergeSteps('/repo', 'b'.repeat(40))
+    expect(steps.map(step => step.name)).toEqual(['versions', 'install', 'upgrade guides', 'host contracts', 'client'])
     expect(steps[0]).toEqual({ name: 'versions', command: process.execPath, args: [join('/repo', 'scripts', 'verify-version-sync.ts'), '/repo'] })
     expect(steps[1]).toEqual({ name: 'install', command: 'pnpm', args: ['install', '--frozen-lockfile'] })
-    expect(steps[3]?.args).toEqual(['exec', 'tsx', 'scripts/compile-referenced-projects.ts', 'client'])
+    expect(steps[2]?.args).toEqual(['run', 'verify-upgrade-guides', '--base', 'b'.repeat(40)])
+    expect(steps[4]?.args).toEqual(['exec', 'tsx', 'scripts/compile-referenced-projects.ts', 'client'])
   })
 
   it('annotates a failure as a warning and passes in report mode', () => {

scripts/pre-merge-check.ts

@@ -1,6 +1,7 @@
 /**
  * Validate the tree a pull request produces once merged with its base branch: the version
- * synchronization of the released manifests, the install, and the Host and Client builds.
+ * synchronization of the released manifests, the install, the version directories of added upgrade
+ * guides, and the Host and Client builds.
  *
  * The `Policy / Pre-Merge Check` job runs this file with `node`, and knows nothing else about the
  * merged tree, so what that tree needs changes here without touching the check. The file runs under
@@ -23,13 +24,17 @@ export interface PreMergeStep {
 /**
  * The steps that check the merged tree, in order. The versions come first because reading the
  * manifests needs no install, so a mistyped release stops the check before anything is built.
+ * The upgrade-guide step compares with the base-branch commit the job merged, so a release published
+ * after the pull request was written rejects its stale guides; without that commit the step is left out.
  * @param root - Repository root.
+ * @param base - Base-branch commit merged into the pull request; the job sets it as `BASE_SHA`.
  * @returns Steps in run order.
  */
-export function preMergeSteps(root: string): PreMergeStep[] {
+export function preMergeSteps(root: string, base?: string): PreMergeStep[] {
   return [
     { name: 'versions', command: process.execPath, args: [join(root, 'scripts', 'verify-version-sync.ts'), root] },
     { name: 'install', command: 'pnpm', args: ['install', '--frozen-lockfile'] },
+    ...base === undefined ? [] : [{ name: 'upgrade guides', command: 'pnpm', args: ['run', 'verify-upgrade-guides', '--base', base] }],
     { name: 'host contracts', command: 'pnpm', args: ['exec', 'tsx', 'scripts/compile-referenced-projects.ts', 'host-contracts'] },
     { name: 'client', command: 'pnpm', args: ['exec', 'tsx', 'scripts/compile-referenced-projects.ts', 'client'] },
   ]
@@ -120,7 +125,11 @@ export async function preMergeCheck(root: string, options: PreMergeOptions = {})
 const invokedPath = process.argv[1]
 const isMain = invokedPath !== undefined && import.meta.url === pathToFileURL(resolve(invokedPath)).href
 if (isMain) {
-  const outcome = await preMergeCheck(resolve(import.meta.dirname, '..'))
+  const root = resolve(import.meta.dirname, '..')
+  // A job definition older than this file sets no BASE_SHA.
+  const base = process.env.BASE_SHA || undefined
+  if (base === undefined) process.stdout.write('pre-merge-check: BASE_SHA is not set, so added upgrade guides are not checked\n')
+  const outcome = await preMergeCheck(root, { steps: preMergeSteps(root, base) })
   if (outcome.ok) {
     process.stdout.write('pre-merge-check: passed\n')
   } else {

scripts/verify-upgrade-guides.spec.ts

@@ -1,8 +1,9 @@
+import { execFileSync } from 'node:child_process'
 import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { dirname, join } from 'node:path'
-import { afterEach, describe, expect, it } from 'vitest'
-import { collectUpgradeGuideViolations } from './verify-upgrade-guides.ts'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import { collectStaleUpgradeGuideAdditions, collectUpgradeGuideViolations } from './verify-upgrade-guides.ts'
 
 const roots: string[] = []
 
@@ -39,17 +40,38 @@ const VALID_ZH = VALID
   .replace('## Change', '## 变更')
   .replace('## Migration', '## 迁移')
 
-function fixtureRoot(files: Record<string, string>): string {
-  const root = mkdtempSync(join(tmpdir(), 'dsh-upgrade-guides-'))
-  roots.push(root)
+function writeGuides(root: string, files: Record<string, string>): void {
   for (const [file, source] of Object.entries(files)) {
     const path = join(root, 'docs/upgrade-guide', file)
     mkdirSync(dirname(path), { recursive: true })
     writeFileSync(path, source)
   }
+}
+
+function fixtureRoot(files: Record<string, string>): string {
+  const root = mkdtempSync(join(tmpdir(), 'dsh-upgrade-guides-'))
+  roots.push(root)
+  writeGuides(root, files)
   return root
 }
 
+/** Run Git with an identity and no user or system configuration, so the fixture ignores the host's settings. */
+function git(cwd: string, ...args: string[]): string {
+  return execFileSync('git', ['-c', 'user.name=t', '-c', '[email protected]', '-c', 'commit.gpgsign=false', '-c', 'init.defaultBranch=master', ...args], {
+    cwd,
+    encoding: 'utf8',
+    env: { ...process.env, GIT_CONFIG_GLOBAL: '/dev/null', GIT_CONFIG_NOSYSTEM: '1' },
+  })
+}
+
+/** Commit `files` with the root version `version` on top of the current branch. */
+function commit(root: string, version: string, files: Record<string, string>): void {
+  writeFileSync(join(root, 'package.json'), `${JSON.stringify({ version })}\n`)
+  writeGuides(root, files)
+  git(root, 'add', '-A')
+  git(root, 'commit', '-q', '-m', version)
+}
+
 describe('upgrade guide gate', () => {
   it('accepts an absent tree and conforming guide pairs, including prerelease versions and CRLF files', () => {
     expect(collectUpgradeGuideViolations(fixtureRoot({}))).toEqual([])
@@ -127,4 +149,80 @@ describe('upgrade guide gate', () => {
       'v0.1.7/over-limit/guide.zh.md': `${VALID_ZH}${'词 '.repeat(600)}\n`,
     }))).toEqual(['docs/upgrade-guide/v0.1.7/over-limit/guide.md: 501 words exceeds the 500-word ceiling'])
   })
+
+  describe('added guide versions', () => {
+    const MESSAGE = ': added outside the current version directory; the root version is 0.2.2-rc.1, so move the guide to docs/upgrade-guide/v0.2.2-rc.1/<item>/'
+
+    beforeEach(() => {
+      vi.stubEnv('GIT_CONFIG_GLOBAL', '/dev/null')
+      vi.stubEnv('GIT_CONFIG_NOSYSTEM', '1')
+    })
+    afterEach(() => {
+      vi.unstubAllEnvs()
+    })
+
+    /** An `origin` whose master holds released guides and whose `release/dsh-0.2.1` branch adds a backported one. */
+    function originRoot(): string {
+      const origin = fixtureRoot({})
+      git(origin, 'init', '-q')
+      commit(origin, '0.2.2-alpha.3', { 'v0.2.2-alpha.3/frozen/guide.md': VALID, 'v0.2.2-alpha.3/moving/guide.md': VALID })
+      git(origin, 'checkout', '-q', '-b', 'release/dsh-0.2.1')
+      commit(origin, '0.2.1-rc.4', { 'v0.2.1-rc.4/backport/guide.md': VALID })
+      git(origin, 'checkout', '-q', 'master')
+      return origin
+    }
+
+    /** A clone of `origin`'s master whose index bumps the root version to 0.2.2-rc.1 and stages `files`. */
+    function bumpedClone(origin: string, files: Record<string, string>, ...cloneArgs: string[]): string {
+      const root = fixtureRoot({})
+      git(root, 'clone', '-q', '--single-branch', ...cloneArgs, origin, '.')
+      writeFileSync(join(root, 'package.json'), `${JSON.stringify({ version: '0.2.2-rc.1' })}\n`)
+      writeGuides(root, files)
+      git(root, 'add', '-A')
+      return root
+    }
+
+    it('accepts guides added in the current directory, edits of released guides, and moves out of a released directory', () => {
+      const root = bumpedClone(originRoot(), { 'v0.2.2-rc.1/current/guide.md': VALID, 'v0.2.2-alpha.3/frozen/guide.md': `${VALID}edited\n` })
+      git(root, 'mv', 'docs/upgrade-guide/v0.2.2-alpha.3/moving', 'docs/upgrade-guide/v0.2.2-rc.1/moving')
+      expect(collectStaleUpgradeGuideAdditions(root, 'HEAD')).toEqual([])
+    })
+
+    it('rejects guides added in a released directory unless the release branch of their line has them', () => {
+      const root = bumpedClone(originRoot(), {
+        'v0.2.2-alpha.3/stale/guide.md': VALID,
+        'v0.2.1-rc.4/backport/guide.md': VALID,
+        'v0.2.1-rc.4/backport/guide.zh.md': VALID_ZH,
+        'v0.2.0-rc.2/no-branch/guide.md': VALID,
+        'README.md': VALID,
+      })
+      expect(collectStaleUpgradeGuideAdditions(root, 'HEAD')).toEqual([
+        'docs/upgrade-guide/v0.2.0-rc.2/no-branch/guide.md',
+        'docs/upgrade-guide/v0.2.1-rc.4/backport/guide.zh.md',
+        'docs/upgrade-guide/v0.2.2-alpha.3/stale/guide.md',
+      ].map(path => `${path}${MESSAGE}`))
+      expect(git(root, 'for-each-ref', 'refs/remotes/origin/release')).toBe('')
+    })
+
+    it('reads a release branch tip in a shallow clone, fetching it only when absent, without cutting the history it already has', () => {
+      const origin = originRoot()
+      git(origin, 'merge', '-q', '--no-edit', 'release/dsh-0.2.1')
+      commit(origin, '0.2.2-alpha.3', { 'v0.2.2-alpha.3/later/guide.md': VALID })
+      const root = bumpedClone(`file://${origin}`, { 'v0.2.1-rc.4/backport-again/guide.md': VALID }, '--depth=3')
+      const history = git(root, 'rev-list', 'HEAD')
+      expect(git(root, 'rev-parse', '--is-shallow-repository').trim()).toBe('true')
+      expect(collectStaleUpgradeGuideAdditions(root, 'HEAD')).toEqual([`docs/upgrade-guide/v0.2.1-rc.4/backport-again/guide.md${MESSAGE}`])
+      expect(git(root, 'rev-list', 'HEAD')).toBe(history)
+
+      git(origin, 'checkout', '-q', 'release/dsh-0.2.1')
+      commit(origin, '0.2.1-rc.5', { 'v0.2.1-rc.4/backport-again/guide.md': VALID })
+      expect(collectStaleUpgradeGuideAdditions(root, 'HEAD')).toEqual([])
+      expect(git(root, 'rev-list', 'HEAD')).toBe(history)
+    })
+
+    it('names the failing Git command for an unknown base', () => {
+      const root = bumpedClone(originRoot(), {})
+      expect(() => collectStaleUpgradeGuideAdditions(root, 'no-such-ref')).toThrow(/`git diff .* no-such-ref -- docs\/upgrade-guide` failed: /u)
+    })
+  })
 })

scripts/verify-upgrade-guides.ts

@@ -1,12 +1,15 @@
 /**
  * Enforce the `docs/upgrade-guide/v<version>/<item>/guide.md` layout, metadata,
  * sections, and word ceiling defined by the `dsh-create-upgrade-guide` skill.
+ * With `--base <ref>`, it also rejects guides added since that commit outside the root version's directory.
  * `verify-translation-pairing` owns the Chinese sibling's pairing record.
  * @module scripts/verify-upgrade-guides
  */
 
+import { execFileSync, spawnSync } from 'node:child_process'
 import { existsSync, readFileSync, readdirSync } from 'node:fs'
 import { join, resolve } from 'node:path'
+import { parseArgs } from 'node:util'
 import { JSON_SCHEMA, load } from 'js-yaml'
 import { fromMarkdown } from 'mdast-util-from-markdown'
 
@@ -74,8 +77,63 @@ export function collectUpgradeGuideViolations(root: string): string[] {
   return violations
 }
 
+/**
+ * Report tracked guides added since `base` outside `docs/upgrade-guide/v<version>/`, where `<version>` is the
+ * root `package.json` version. A version directory is frozen once that version is released, so a guide that a
+ * pull request wrote before a release must move to the new version's directory before it merges. A guide that
+ * the tip of `origin`'s `release/dsh-<x.y.z>` branch for its version line already contains passes, because
+ * merging a release branch forward carries guides that were current on it. The tip is fetched by commit id,
+ * without updating any ref, only when such a guide exists and the commit is absent locally; a shallow
+ * repository fetches only that commit.
+ * @param root - Repository root: a Git working tree with an `origin` remote.
+ * @param base - Commit the working tree is compared with: the base-branch commit a pull request merges.
+ * @returns diagnostics prefixed by the repository-relative guide path.
+ * @throws Error naming the Git command when Git fails, such as for an unknown `base` or an unreachable `origin`.
+ */
+export function collectStaleUpgradeGuideAdditions(root: string, base: string): string[] {
+  const git = (args: readonly string[]): string => {
+    try {
+      return execFileSync('git', args, { cwd: root, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] })
+    } catch (error: unknown) {
+      const stderr = (error as { stderr?: unknown }).stderr
+      throw new Error(`\`git ${args.join(' ')}\` failed: ${typeof stderr === 'string' && stderr.trim() !== '' ? stderr.trim() : String(error)}`, { cause: error })
+    }
+  }
+  const { version } = JSON.parse(readFileSync(join(root, 'package.json'), 'utf8')) as { version?: unknown }
+  if (typeof version !== 'string') throw new Error('package.json: version must be a string')
+  const current = `${GUIDE_ROOT}/v${version}/`
+  const released = new Map<string, ReadonlySet<string>>()
+  const releasedGuides = (line: string): ReadonlySet<string> => {
+    const tip = git(['ls-remote', 'origin', `refs/heads/release/dsh-${line}`]).split('\t')[0]?.trim() ?? ''
+    if (tip === '') return new Set()
+    if (spawnSync('git', ['cat-file', '-e', `${tip}^{commit}`], { cwd: root, stdio: 'ignore' }).status !== 0) {
+      // Fetching a commit already in a shallow repository with `--depth` would cut the history behind it.
+      const shallow = git(['rev-parse', '--is-shallow-repository']).trim() === 'true'
+      // A background maintenance run after a shallow fetch rewrites .git/shallow under later Git commands.
+      git(['-c', 'maintenance.auto=false', '-c', 'gc.auto=0', 'fetch', '--quiet', '--no-tags', '--no-write-fetch-head', ...(shallow ? ['--depth=1', '--filter=blob:none'] : []), 'origin', tip])
+    }
+    return new Set(git(['ls-tree', '-r', '-z', '--name-only', tip, '--', GUIDE_ROOT]).split('\0').filter(Boolean))
+  }
+  return git(['diff', '-z', '--name-only', '--no-renames', '--diff-filter=A', base, '--', GUIDE_ROOT])
+    .split('\0')
+    .filter((path) => {
+      const line = /^v(\d+\.\d+\.\d+)/u.exec(path.split('/')[2] ?? '')?.[1]
+      // `collectUpgradeGuideViolations` reports a path outside any version directory.
+      if (path === '' || path.startsWith(current) || line === undefined) return false
+      if (!released.has(line)) released.set(line, releasedGuides(line))
+      return released.get(line)?.has(path) !== true
+    })
+    .map(path => `${path}: added outside the current version directory; the root version is ${version}, so move the guide to ${current}<item>/`)
+}
+
 if (process.argv[1] && import.meta.filename === resolve(process.argv[1])) {
+  const { values } = parseArgs({ options: { base: { type: 'string' } } })
   const violations = collectUpgradeGuideViolations(ROOT)
+  try {
+    if (values.base !== undefined) violations.push(...collectStaleUpgradeGuideAdditions(ROOT, values.base))
+  } catch (error: unknown) {
+    violations.push(`cannot check guides added since ${values.base ?? ''}: ${error instanceof Error ? error.message : String(error)}`)
+  }
   if (violations.length > 0) {
     process.stderr.write('verify-upgrade-guides: violations found (see .agents/skills/dsh-create-upgrade-guide/SKILL.md):\n')
     for (const violation of violations) process.stderr.write(`  ${violation}\n`)