docs(context): capture 2026-05-14 triage-session learnings
Adds eight Session learnings entries covering the seven defect PRs opened in this session plus the multi-PR triage operational pattern: - #3095/#3490 — executor failure classification + cross-runtime sentinels - #3489/#3499 — STATE.md idempotency oracle for state.complete-phase - #3488/#3501 — same-phase short-form depends_on lookup index - #3491/#3502 — git rev-parse --is-inside-work-tree primitive for nested worktree detection - #3493/#3500 — extractCurrentMilestone generic Phase Details continuation - #3503/#3504 — homedir substring containment trailing-slash anchor - #3346/#3505 — Codex AoT TOML leaf key event-name composition - #3506/#3507 — label-scoped stale-bot sub-job pattern Plus operational lessons on GitHub auto-close-keyword parsing in PR bodies, gh-templates-first hook gating, sub-agent capacity ceilings on 24 GB systems, and trust-but-verify discipline on agent return reports. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
76
CONTEXT.md
76
CONTEXT.md
@@ -673,3 +673,79 @@ Module owning all OS-facing I/O for the tool: runtime-aware command-text renderi
|
||||
- stash pop can fail if main brought in the same files: drop the stash with `git stash drop` when files are already present
|
||||
- PR hook requires reading all listed contribution files before `gh pr create` will execute — including `pull_request_template.md`, all typed templates, and all issue templates
|
||||
- changeset type for seam additions: `Changed` (not `Added`) — expands existing module, not a new standalone feature
|
||||
|
||||
### 2026-05-14 — Executor failure classification + cross-runtime provider sentinels (#3095, PR #3490)
|
||||
|
||||
- scope: new SDK query `agent.classify-failure` parses free-text agent return body and returns `{ class: 'quota-exceeded' | 'classify-handoff-bug' | 'unknown-failure', sentinel?, retryAfterSeconds? }`
|
||||
- handler: `sdk/src/query/agent-failure-classifier.ts`; registered in `command-static-catalog-foundation.ts` (DECISION_ROUTING_STATIC_CATALOG) and `command-manifest.non-family.ts` (`mutation: false`, `outputMode: 'json'`)
|
||||
- workflow wiring lives in `get-shit-done/workflows/execute-phase.md` step 7; replaces single-branch "real failure" routing with class-distinct prompts (quota → wait-for-reset; classify-handoff-bug → existing spot-check; unknown → existing continue/stop)
|
||||
- sentinel-order invariant: most specific first. `429` beats `too many requests`; `quota` beats `resource_exhausted`. All matches case-insensitive; canonical sentinel value is lower-cased form
|
||||
- cross-runtime sentinel coverage (catalogued in `docs/research/provider-rate-limit-signals.md`):
|
||||
- Anthropic / Claude Code: `usage limit`, `rate limit`, `quota`, `429`, `retry-after`
|
||||
- GitHub Copilot CLI: `rate_limit` (stem covers `rate_limited`, `rate_limit_error`, `user_weekly_rate_limited`)
|
||||
- OpenAI Codex CLI: `429`, `usage_limit_reached`, `too many requests`
|
||||
- Google Gemini CLI: `RESOURCE_EXHAUSTED`, `exceeded your`
|
||||
- precedence rule: quota sentinel wins over `classifyHandoffIfNeeded` bug when both appear — a quota-kill that also crashed the completion handler is still a quota event, recovery differs
|
||||
- proactive-signal forward path (NOT yet usable from GSD orchestrator): Anthropic exposes `anthropic-ratelimit-{requests,tokens,input-tokens}-remaining` + `retry-after`; Agent SDK emits `RateLimitEvent` with `status: 'allowed' | 'allowed_warning' | 'rejected'`. Claude Code subprocess does NOT forward these to hooks/statusline today — tracked in upstream anthropics/claude-code#33820, #22407, #32796
|
||||
- `retry-after` parser uses `\bretry[-_ ]after[:\s]+(\d+)\b` to avoid embedded-word false matches like `noretry-after`
|
||||
|
||||
### 2026-05-14 — STATE.md idempotency oracle for `state.complete-phase` (#3489, PR #3499)
|
||||
|
||||
- `cmdStateCompletePhase` in `get-shit-done/bin/lib/state.cjs` previously wrote STATE.md unconditionally; re-invoking on a phase already superseded by a later one rewound Status / Last Activity / Current Position to stale values silently
|
||||
- invariant: STATE.md's canonical `Current Phase` field is the idempotency oracle. If the field names a phase distinct from the resolved target, return `{ updated: [], phase, idempotent: true, note: "phase already superseded; no-op" }` without touching STATE.md
|
||||
- legacy-bridge banner `'state.complete-phase' not in native registry; falling back to gsd-tools.cjs` is the user-visible signal that this path runs the .cjs handler, not a typed SDK port — when fixing handler bugs, the .cjs path is canonical until ported
|
||||
|
||||
### 2026-05-14 — Same-phase short-form `depends_on` requires its own lookup index (#3488, PR #3501)
|
||||
|
||||
- bug: DAG resolver in `sdk/src/query/phase.ts` (`phasePlanIndex` Pass 2) only resolved `depends_on` via `planMap` (full-stem `<phase>-<plan>`) and `canonicalToId` (canonical-prefix); same-phase short-form `[NN]` was never indexed, so `'01'` never matched plan IDs keyed as `'99.9-01'`
|
||||
- fix: add third index `shortFormToId` built from `extractCanonicalPlanId(p.id).slice(lastIndexOf('-') + 1)` — works for integer, letter-suffixed, and decimal phase IDs. Resolution order: `planMap` → `canonicalToId` → `shortFormToId`
|
||||
- diagnostic invariant: when a `depends_on` reference cannot be resolved, the resolver MUST emit `unresolvedDeps[]` warnings (`Plan X: unresolved depends_on reference 'NN' — no matching plan in phase`). Without this, dropped edges hide behind a downstream wave-mismatch warning that misleadingly points at the wave declaration
|
||||
- reporter-claims-narrower-scope pattern: #3488 reporter believed integer-phase short-form worked; regression test proved it was equally broken — write the test for BOTH claimed-working and claimed-broken cases
|
||||
|
||||
### 2026-05-14 — Nested git worktree detection in `new-project` bootstrap (#3491, PR #3502)
|
||||
|
||||
- `pathExistsInternal(cwd, '.git')` is the wrong primitive for detecting an existing git context — it only sees `.git` directly in cwd, missing the case where cwd is a subdirectory of an outer worktree
|
||||
- right primitive: `git rev-parse --is-inside-work-tree` + `git rev-parse --show-toplevel`, surfaced through the existing `execGit` seam in `get-shit-done/bin/lib/core.cjs`
|
||||
- new helper `gitWorktreeInfoInternal(cwd)` returns `{ has_git, git_worktree_root, in_nested_subdir }`; surfaced via `cmdInitNewProject` and `cmdInitIngestDocs` (`bin/lib/init.cjs`) and mirrored in SDK `sdk/src/query/init.ts` / `init-complex.ts`
|
||||
- workflow gate: `new-project.md` and `ingest-docs.md` now refuse `git init` when `in_nested_subdir: true` and surface "planning files will track to the outer worktree" warning instead of creating nested `.git`
|
||||
- when adding a new large workflow that crosses the prompt-injection-scan size threshold, append to ALLOWLIST in `tests/prompt-injection-scan.test.cjs` alongside `discuss-phase.md` / `execute-phase.md` / `plan-phase.md`
|
||||
|
||||
### 2026-05-14 — `extractCurrentMilestone` generic Phase Details continuation (#3493, PR #3500)
|
||||
|
||||
- `nextMilestoneRegex` in `sdk/src/query/roadmap.ts` only treats same-version sub-headings as continuations (from #2455 / #2422). Generic non-version-prefixed `## Phase Details` placed AFTER a `### 📋 vX.Y+ (Planned)` sibling fell outside the milestone slice — `phase.insert N` then reported "Phase N not found in ROADMAP.md" despite the heading being unambiguously present
|
||||
- fix: after the initial boundary scan, look forward for a literal `^#{1,3}\s+Phase\s+Details\b` heading past `sectionEnd` and append that block (up to next real milestone boundary or EOF). Intervening planned-milestone content is skipped, not concatenated — cannot leak across milestones. Bounded to a single append
|
||||
- do not regress: same-version `## v2.0 Phase Details` continuation from #2455 must still work; the existing `bug-2422` regression test pins this
|
||||
|
||||
### 2026-05-14 — Path-replacement homedir check needs trailing-slash anchor (#3503, PR #3504)
|
||||
|
||||
- `tests/path-replacement.test.cjs:163` used `content.includes(normalizedHomedir)` — substring match false-positives when homedir is short and appears inside an identifier (e.g. Docker `/root` matches `</root_cause_analysis>` in agent markdown)
|
||||
- fix: `content.includes(normalizedHomedir + '/')` — real path leaks always have a trailing `/`; identifier substrings don't
|
||||
- general pattern: substring-containment checks on short absolute prefixes (`/root`, `/tmp`, `/opt`) need delimiter anchors
|
||||
|
||||
### 2026-05-14 — Codex AoT hooks migration: leaf TOML key must be event name (#3346, PR #3505)
|
||||
|
||||
- `migrateCodexHooksMapFormat` in `bin/install.js` used `s.path.slice('hooks.'.length)` as the leaf key for emitted `[[hooks.<EVENT>]]` blocks. When the legacy table key was a `<file>:<event>:<line>:<col>` diagnostic location identifier and the real event lived in the body's `event = "..."` field, the location tuple passed through verbatim as the leaf — Codex 0.124.0+ refuses to load such keys (`:` and `\` collide with TOML's `.`-separated key syntax)
|
||||
- fix: the `mapOnlyBlocks` and `staleNamespacedAotBlocks` branches now mirror the flat-AoT branch and prefer `extractFlatHookEventName(body)` over the raw path segment when the body declares `event = "..."`
|
||||
- post-write validator message "expected `[[...]]` array-of-tables" was a downstream symptom — the bracket form is correct, the leaf key chain was the bug. When validator-style errors describe a fix shape, verify the actual emitted string first; the validator may be misdiagnosing the cause
|
||||
- regression test fixture: legacy `[hooks]` block with a location-identifier table key → assert parsed result has `hooks.state.session_start` (event name) as the leaf path
|
||||
|
||||
### 2026-05-14 — Label-scoped stale-bot sub-job pattern (#3506, PR #3507)
|
||||
|
||||
- two `actions/stale` jobs in one workflow do not conflict when each scopes via `only-issue-labels` (tight job) plus the other's labels in its `exempt-issue-labels` (broad job)
|
||||
- tight job pattern for "we asked, no response" enforcement:
|
||||
- `only-issue-labels: 'awaiting-retest,needs-reproduction'`
|
||||
- `days-before-issue-stale: 5`
|
||||
- `days-before-issue-close: 0` (stale + close in same run)
|
||||
- `days-before-pr-stale: -1` / `days-before-pr-close: -1` (disable PR side)
|
||||
- `remove-stale-when-updated: true`
|
||||
- broad job (28+14) must add `awaiting-retest,needs-reproduction` to its `exempt-issue-labels` so the two jobs do not race or double-close
|
||||
- maintainers dry-run via `gh workflow run "Stale Cleanup"` before next cron tick; the action publishes a closes-summary
|
||||
|
||||
### 2026-05-14 — Multi-PR triage operational lessons
|
||||
|
||||
- GitHub auto-close-keyword parsing: `Fix #N` / `Fixes #N` / `Closes #N` in PR body prose (e.g. "Suggested fix #2 — `stopped_at` derivation") triggers spurious closes. Neutralise with backticks or parens: `` `(2)` `` instead of `#2`. After every `gh pr create`, verify `gh pr view <n> --json closingIssuesReferences -q '[.closingIssuesReferences[].number]'`
|
||||
- `gh-templates-first` hook gates `gh pr create` / `gh pr edit` / `gh issue create|edit` until every PR/issue template has been Read in-session. Pre-empt with one parallel-Read of `CONTRIBUTING.md`, all `.github/PULL_REQUEST_TEMPLATE/*.md`, `.github/pull_request_template.md`, and all `.github/ISSUE_TEMPLATE/*.yml` before the first `gh` invocation
|
||||
- sub-agent capacity: 4 parallel test-running debugger agents @ ~4 GB each overflow a 24 GB system. For multi-bug triage waves, run isolated-worktree agents **sequentially in foreground**, not in parallel — the worktree isolation still buys clean branch separation
|
||||
- trust-but-verify on every sub-agent return: confirm PR opened (`gh pr view <n>`), `closingIssuesReferences` lists exactly the intended issue, regression test path exists, scope is clean. Agents describe intent, not action
|
||||
- when auto-generated PR-template branch (`feat/NNNN-...` from `github-actions` bot) is heavily diverged from main (hundreds of files, large negative diff), discard it and branch fresh off main — the auto-branch is a courtesy reference, not a base
|
||||
- escalation pattern: an issue asking for a release publish or backport (e.g. #3340 — fix already in main, awaiting npm publish) is `ready-for-human`, not `ready-for-agent`. No branch can resolve it
|
||||
|
||||
Reference in New Issue
Block a user