The previous registry was generated from a stale main-repo state that was
missing cline's hostBehaviors (merged in #2090). CI's lint:generated-sync
detected the staleness. Regenerated from clean origin/next + hermes changes.
The implementation subagent cross-contaminated the branch with changes from
PR #2121 (phase-identifier parsing consolidation):
- Deleted docs/adr/2121-phase-identifier-parsing-consolidation.md (restored)
- Deleted src/phase-id.cts (restored)
- Deleted tests/phase-id.test.cjs (restored)
- Modified src/roadmap-parser.cts (restored to origin/next)
- Modified src/state.cts (restored to origin/next)
None of these are related to the Cline EoS migration.
Orthogonal review surfaced that resolvePhaseIdForCompletePhase (state.cts) and
cmdStateCompletePhase's idempotency check still used an unanchored
/(\d+[A-Z]?(?:\.\d+)*)/i — even more permissive than the parseProsePhaseField
regex this phase fixes. Reachable corruption: after `milestone complete v0.5`,
`state complete-phase` (no --phase) mined "0.5" from the body line
"Phase: Milestone v0.5 complete" and rewrote STATE.md as "Phase 0.5 complete".
Both sites now delegate to phase-id.cts:parsePhaseFromProse (the same anchored
parser), so a milestone-closure line yields no token and the existing
"unable to resolve" guard fires instead of corrupting. Canonical tokens
(3, 03, 3A, 3.3, "3 of 5", "1 — Setup") are preserved unchanged.
Regression (tests/state.test.cjs, complete-phase suite): `state complete-phase`
on a "Milestone v0.5 complete" STATE.md now rejects and does not mine "0.5".
Demonstrated fail-first.
Refs #2125, #2121
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Phase 2 of epic #2121. state.cts:parseProsePhaseField now delegates to the
anchored phase-id.cts:parsePhaseFromProse (built in Phase 1), removing this
module's independent prose phase-id regex.
Drives #2111: `milestone complete vX.Y` no longer corrupts current_phase. The
body line "Phase: Milestone v0.5 complete" previously had "5" mined from it by
the unanchored regex; the anchored parser returns { phase: null }, so
syncStateFrontmatter's #905 guard preserves the real current_phase. This also
fixes the broader family the review surfaced — every milestone completion
(e.g. v1.0 -> "0") was silently corrupting current_phase, not just .5-versions.
Regression (tests/milestone.test.cjs, in the milestone-complete suite, #2111):
`milestone complete v0.5` on a project with current_phase: "19" now preserves
"19". Demonstrated fail-first end-to-end: reverting the migration reproduces
current_phase = "5".
Closes#2125
Refs #2121
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Orthogonal security review of the Phase 1 surface found two issues; both fixed
and regression-tested:
- MEDIUM ReDoS: the name-extraction regexes /\(([^)]+)\)/ and
/—\s*([^(\n]+?).../ backtrack O(n^2) on a crafted STATE.md field value with a
long unterminated "(" / "—" run (reviewer measured ~38s at 320k chars).
Length-bound both quantifiers to {1,200} -> linear (320k now ~100ms). A real
phase name is far shorter than the cap.
- LOW: parsePhaseFromProse threw on non-string truthy input, unlike its three
sibling #2121 functions. Coerce via String(value) up front.
The identical ReDoS regexes are copied verbatim from the pre-existing
state.cts:parseProsePhaseField; per the no-defer rule that surfaced defect is
fixed inline there too (Phase 2 / #2125 later supersedes that function by
delegating to the bounded phase-id.cts parser).
Adds a behavioral bound-guard regression test (a >200-char parenthetical is not
extracted) and a non-string-coercion test.
Refs #2124, #2121
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add the ADR-2121-locked canonical functions to src/phase-id.cts. No consumer
behavior changes — Phases 2-4 migrate the divergent call sites against them.
- parsePhaseFromProse: anchored prose parser. A phase is returned only when the
STATE.md "Phase:" field VALUE begins with a phase token, so
"Milestone v0.5 complete" yields { phase: null } instead of "5" (the #2111
root cause: the old unanchored \b(\d+..)\b mined the minor-version digit).
Name extraction (parenthetical / em-dash tail, minus status words) unchanged.
- stripConfiguredProjectCodePrefix / isForeignPrefixedPhaseQuery: config-aware
prefix policy. A foreign prefix (MEM-01 when the configured code is LKML) is
preserved rather than collapsed to a bare numeric phase — the #2104 fix's
canonical home (consumed later, outside this epic's critical path).
- roadmapPhaseLookupSources: moved from roadmap-parser.cts so phase-id.cts is
the single owner of the exact -> numeric -> prefix-tolerant ordering.
roadmap-parser.cts now imports it (behavior-identical); its two now-unused
imports (phaseMarkdownRegexSourceExact, OPTIONAL_PROJECT_CODE_PREFIX_SOURCE)
are dropped.
Tests: subject-named suites in tests/phase-id.test.cjs covering the ADR
boundary set (v0.5, v1.0, MEM-01, AB-29, bare 29, zero-padded 029) plus two
fast-check properties: the #2111 "Milestone vX.Y complete never yields a phase"
invariant and a parse/normalize property.
Extend-never-mutate: the 12 pre-existing phase-id.cts exports are unchanged.
Closes#2124
Refs #2121
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Phase 0 of the #2121 epic — an ADR-only PR that LOCKS the contract Phases
1-4 execute against. No production code lands here.
Locks:
- phase-id.cts as the single canonical owner of phase-identifier parsing.
- New pure exports Phase 1 adds: parsePhaseFromProse (anchored; fixes the
#2111 "Milestone v0.5 complete -> 5" class), stripConfiguredProjectCodePrefix
/ isForeignPrefixedPhaseQuery (config-aware; the #2104 fix's home),
and roadmapPhaseLookupSources moved in as sole owner of the 3-source
ordering (fixes the #2114 2-vs-3-source divergence).
- Extend-never-mutate on the 12 existing exports (normalizePhaseName has a
CRITICAL 84-symbol / 20-caller blast radius) — Hyrum's Law.
- The exact exact->numeric->prefix-tolerant lookup ordering.
- A behavioral anti-divergence contract: reference-identity guard +
scripts/lint-phase-id-drift.cjs scanner, modeled on the repo's proven
capability-precedence-parity / package-identity-drift patterns.
#2104 remains blocked on PR #2105 and off this epic's critical path.
Adds the docs/adr/README.md index row. Docs-only; no changeset required
(no-changelog).
Closes#2121
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reverts the LOW-severity dead-import removal (require('fs')/require('path'))
that changed the file hash and broke 9 golden-install-parity fixtures. The
golden test computes per-runtime hashes of installed hook files; regenerating
all 9 fixtures for a cosmetic cleanup is disproportionate. Dead imports are
harmless (Node caches built-in requires) — noted as a follow-up nit.
MEDIUM fixes (code review):
- Wire resolveManagedHookEvents + resolveHookScripts + buildHookBusEntries
from imperative-hook-bus.cts into writeCursorHooksJson — the install path
is now truly descriptor-driven (reads hostBehaviors.managedHookEvents),
not a hardcoded constant that happens to match the descriptor. bin/install.js
passes the descriptor list via opts.managedHookEvents.
- buildHookBusEntries is now consumed (was dead code); entry-building is no
longer duplicated inline.
- Remove try/finally from cursor-hook-bus-upgrade.test.cjs test bodies
(violated CONTRIBUTING.md L342; redundant with t.after cleanup).
LOW fixes:
- Remove dead require('fs')/require('path') from gsd-cursor-pre-tool.js
- Fix resolveManagedHookEvents docstring (all-invalid fallback behavior)
- Add src/runtime-hooks-surface.cts to the AC2 source-guard file list
Security review: no CRITICAL/HIGH/MEDIUM findings (3 LOW are pre-existing
#777 baseline patterns, not regressions).
Replaces the timing-dependent execFileSync(timeout:5000) approach with a
spawnSync-based runHook that tests the scanner's RESULT (exit code + output
shape), never how long it takes.
Root design flaw in the prior approach: execFileSync's timeout (5000ms)
was identical to the scanner's own internal setTimeout(5000ms), creating a
non-repeatable race (F.I.R.S.T. violation: not Repeatable). Under concurrent
test-chunk load — which #2089's 3 new cursor test files redistribute —
node22's event-loop scheduling let execFileSync's SIGTERM win the race,
producing err.status=null → exitCode=1 → spurious property-test failure.
Redesign (F.I.R.S.T.):
- spawnSync (not execFileSync): non-zero exits return a result object,
not an exception — cleaner for property tests
- Non-serializable payloads (BigInt, circular refs, Symbol) are SKIPPED:
the scanner receives JSON via stdin, so these values are outside its
protocol — JSON.stringify throwing is a test-harness artifact, not a
scanner defect
- 30s safety-net timeout is NOT a test assertion: scanner exits in <100ms;
30s only catches a genuinely hung process (6x the scanner's own 5s
internal timer → no race possible)
- Assertions check exit===0 and output structure, never timing
qa-test-architect pipeline: risk=HIGH (security boundary); automation=
subprocess (real shipped hook); test-cases cover happy/boundary/negative/
independence; verified via gsd-test.
The property test's execFileSync timeout (5000ms) was identical to the
scanner's own internal stdin-timeout (hooks/gsd-read-injection-scanner.js:109,
also 5000ms). Under concurrent test-chunk load on linux-node22 — which #2089's
3 new cursor test files redistribute — the scanner subprocess's stdin 'end'
event can fire late enough that execFileSync's SIGTERM arrives before the
scanner's own process.exit(0), producing err.status=null → exitCode=1 →
spurious property-test failure.
The scanner has no process.exit(N!=0) paths; the only non-zero exit is from
the signal-kill race. Doubling the test ceiling to 10000ms gives the scanner's
5000ms internal exit a 5s buffer to win the race deterministically on every
node version.
- Add /gsd-core/bin/lib/host-integration-adapters/imperative-hook-bus.cjs to
.gitignore (tsc-emitted build artifact per ADR-457 convention; matches the
sibling adapter entries at .gitignore:70-89). The subagent authored the
.cts source but missed this entry, leaving the compiled output untracked.
- Fix cosmetic 'Context3' -> 'Context7' typo in test section header comment.
The targeted CI lane runs changed files UNSHARDED; #2088 touched 13 install-heavy
test files that all landed in one chunk, blowing the 600s per-chunk backstop on
the slow Windows runner (pure slowness, not a leak — per run-tests.cjs's own
comment). Weight install*/codex-* files (~10x a unit file) toward the per-chunk
budget so they spread across chunks instead of clustering; light-file chunking is
unchanged (weight 1). Adds harness regression tests (heavy split vs light control).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drive Codex install/uninstall through the descriptor-driven Host-Integration
Interface (declarative embedding adapter → engine surface dispatch) and fold
every positive `runtime === 'codex'` / `isCodex` projection into descriptor-driven
`runtime.hostBehaviors`. Install/uninstall output stays byte-parity-gated
(tests/fixtures/golden-install-parity/codex.json); no other runtime changes.
Three Context7-verified upgrades, each with a test on the user-reachable surface:
- Skill root → canonical $HOME/.agents/skills via a skills-kind `home` override,
with pre-move migration cleanup (stale ~/.codex/skills/gsd-* removed on install
and uninstall; user content preserved). Fixes getGlobalSkillsBase, writeManifest,
and the skill-manifest inventory to honor the override so --skills-root /
sync-skills / the manifest report the real location.
- Six new hooks.json lifecycle events (PreToolUse, PermissionRequest, PreCompact,
PostCompact, SubagentStop, UserPromptSubmit) shared by install + uninstall;
extendedHookEvents reconciled [] -> the schema-valid wired subset.
- Explicit `[agents] max_depth = 1` in the managed config.toml block, pinning the
negotiated dispatch.maxDepth:1 axis. validateCodexConfigSchema now permits a
known-scalar-only bare `[agents]` AgentsToml table (still rejects [[agents]] and
unknown-key break-forms, #2760); mergeCodexConfig preserves the user's own
AgentsToml scalars (max_threads etc.) instead of dropping them.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Code review (HIGH): a hardcoded 'timeout 600 agy' fails with rc 127 on stock
macOS (no GNU timeout/gtimeout), silently losing the agy reviewer. Probe for
'timeout'/'gtimeout' via command -v and fall back to agy's native --print-timeout
alone when neither exists (mirrors scripts/base64-scan.sh). External cap (600s)
stays >= --print-timeout (540s) so it only backstops a pre-session stall. Factor
the prompt into _AGY_PROMPT to avoid duplicating the long -p string across both
branches. Update the agy + #687 tests to assert the probe + bound + fallback,
regen the 17 goldens + size baseline, refresh the maintainer-note version stamp
to 1.0.16.
