Merge pull request #3205 from 0xdhx/fix/3174-quick-verification-status-query
fix(#3174): read quick's verification status via the verification.status query
This commit is contained in:
5
.changeset/3174-quick-verification-status-query.md
Normal file
5
.changeset/3174-quick-verification-status-query.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3205
|
||||
---
|
||||
**`/gsd-quick --validate` no longer trusts a verification result it cannot actually read** — quick parsed the verifier's status by grepping the whole report rather than its frontmatter, so a `status:` line in the report's prose could be picked up alongside or instead of the real one, staleness was never detected at all, and a range of valid and malformed reports alike resolved to a value no routing arm matched — leaving the orchestrator to improvise at the moment the pipeline had failed. Quick now reads the same frontmatter-anchored, staleness-aware `verification.status` query that `execute-phase`, `verify-work` and `progress` already use, and routes `missing` / `unknown` / `stale` through an explicit arm instead of falling through. (#3174)
|
||||
@@ -32,15 +32,36 @@ Check must_haves against actual codebase. Create VERIFICATION.md at ${QUICK_DIR}
|
||||
|
||||
> **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available.
|
||||
|
||||
Read verification status:
|
||||
Read verification status via the canonical query (frontmatter-anchored, and total over its input space):
|
||||
```bash
|
||||
grep "^status:" "${QUICK_DIR}/${quick_id}-VERIFICATION.md" | cut -d: -f2 | tr -d ' '
|
||||
_GSD_SHIM_NAME="gsd-tools.cjs"; _GSD_RUNTIME_ROOT="${RUNTIME_DIR:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}"; GSD_TOOLS="${_GSD_RUNTIME_ROOT}/gsd-core/bin/${_GSD_SHIM_NAME}"; if [ -f "$GSD_TOOLS" ]; then gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${_GSD_RUNTIME_ROOT}/.claude/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${_GSD_RUNTIME_ROOT}/.claude/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${_GSD_RUNTIME_ROOT}/.codex/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${_GSD_RUNTIME_ROOT}/.codex/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif command -v gsd-tools >/dev/null 2>&1; then GSD_TOOLS="$(command -v gsd-tools)"; gsd_run() { "$GSD_TOOLS" "$@"; }; elif [ -f "${CLAUDE_CONFIG_DIR:-$HOME/.claude}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CLAUDE_CONFIG_DIR:-$HOME/.claude}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${HERMES_HOME:-$HOME/.hermes}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${HERMES_HOME:-$HOME/.hermes}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${CURSOR_CONFIG_DIR:-$HOME/.cursor}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CURSOR_CONFIG_DIR:-$HOME/.cursor}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${CODEX_HOME:-$HOME/.codex}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CODEX_HOME:-$HOME/.codex}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${GEMINI_CONFIG_DIR:-$HOME/.gemini}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${GEMINI_CONFIG_DIR:-$HOME/.gemini}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${COPILOT_CONFIG_DIR:-$HOME/.copilot}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${COPILOT_CONFIG_DIR:-$HOME/.copilot}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${WINDSURF_CONFIG_DIR:-$HOME/.codeium/windsurf}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${WINDSURF_CONFIG_DIR:-$HOME/.codeium/windsurf}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${AUGMENT_CONFIG_DIR:-$HOME/.augment}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${AUGMENT_CONFIG_DIR:-$HOME/.augment}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${TRAE_CONFIG_DIR:-$HOME/.trae}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${TRAE_CONFIG_DIR:-$HOME/.trae}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${QWEN_CONFIG_DIR:-$HOME/.qwen}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${QWEN_CONFIG_DIR:-$HOME/.qwen}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${CODEBUDDY_CONFIG_DIR:-$HOME/.codebuddy}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CODEBUDDY_CONFIG_DIR:-$HOME/.codebuddy}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${CLINE_CONFIG_DIR:-$HOME/.cline}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CLINE_CONFIG_DIR:-$HOME/.cline}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${GROK_AGENTS_HOME:-$HOME/.agents}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${GROK_AGENTS_HOME:-$HOME/.agents}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${ANTIGRAVITY_CONFIG_DIR:-$HOME/.gemini/antigravity}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${ANTIGRAVITY_CONFIG_DIR:-$HOME/.gemini/antigravity}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${OPENCODE_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/opencode}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${OPENCODE_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/opencode}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${KILO_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/kilo}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${KILO_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/kilo}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; else echo "ERROR: gsd-tools.cjs not found at $GSD_TOOLS and gsd-tools is not on PATH. Run: npx -y @opengsd/gsd-core@latest --claude --local" >&2; exit 1; fi; if [ -n "${CLAUDE_ENV_FILE:-}" ] && [ -n "${GSD_TOOLS:-}" ]; then printf "export PATH='%s':\"\$PATH\"\n" "${GSD_TOOLS%/*}" >> "$CLAUDE_ENV_FILE" 2>/dev/null || true; fi
|
||||
STATUS=$(gsd_run query verification.status "${QUICK_DIR}" --pick status 2>/dev/null)
|
||||
```
|
||||
|
||||
Store as `$VERIFICATION_STATUS`.
|
||||
`--pick status` returns the bare value, so this path needs **no `jq`**. That is deliberate: #2589
|
||||
established that a `| jq -r '.field'` pipe yields an **empty** variable with no diagnostic on any
|
||||
machine without jq — the default on Windows/Git-Bash — which here would route a perfectly good
|
||||
`passed` verification into the recovery arm below.
|
||||
|
||||
| Status | Action |
|
||||
Route on `$STATUS` and store the display string as `$VERIFICATION_STATUS` (consumed by the quick index row and the completion banner).
|
||||
|
||||
The query is **total**: beyond the verifier's own statuses it can also return `missing` (no `*-VERIFICATION.md`, or no `status` in its frontmatter), `unknown` (a value outside the verifier's schema) or `stale` (a summary newer than the verification file). Which one wins when more than one applies is the query's own precedence, not this table's concern — all three land in the same arm here. That arm is reachable in normal operation and must never be dropped.
|
||||
|
||||
| `$STATUS` | Action |
|
||||
|--------|--------|
|
||||
| `passed` | Store `$VERIFICATION_STATUS = "Verified"`, continue to step 7 |
|
||||
| `human_needed` | Display items needing manual check, store `$VERIFICATION_STATUS = "Needs Review"`, continue |
|
||||
| `gaps_found` | Display gap summary, offer: 1) Re-run executor to fix gaps, 2) Accept as-is. Store `$VERIFICATION_STATUS = "Gaps"` |
|
||||
| anything else — `missing`, `unknown`, `stale`, or empty | Do **not** improvise a result. Report that verification produced no usable status, naming `$STATUS`, then offer: 1) Re-run the verifier, 2) Accept as-is without verification. Store `$VERIFICATION_STATUS = "Unverified (${STATUS:-no result})"` |
|
||||
|
||||
> **Why the status only, and not `next_action` / `next_command`.** The query projects those two for
|
||||
> the phase pipeline — they name `execute-phase`, `plan-phase --gaps` and `verify-work`, and they
|
||||
> append a phase-number argument taken from the directory basename. A quick task directory is named
|
||||
> `${quick_id}-${slug}` with a date-derived `quick_id`, so that argument resolves to the date: for
|
||||
> `260808-abc-some-slug` the projected recovery command carries `260808` as its phase argument — a
|
||||
> date posing as a phase number.
|
||||
> Quick therefore supplies its own recovery actions above. The split is the point:
|
||||
> `readVerificationStatus` *discovers and parses* shape-agnostically — it scans whatever directory
|
||||
> it is given for `*-VERIFICATION.md` — which is what makes the status half correct for
|
||||
> `${QUICK_DIR}`; but it also reads that directory's basename as a phase token to build the
|
||||
> projected commands, and that is the half quick must not use.
|
||||
|
||||
128
tests/fix-3174-quick-verification-status-read.test.cjs
Normal file
128
tests/fix-3174-quick-verification-status-read.test.cjs
Normal file
@@ -0,0 +1,128 @@
|
||||
// allow-test-rule: source-text-is-the-product see #3174
|
||||
// Workflow .md / agent .md / command .md / reference .md files — their text
|
||||
// IS what the runtime loads. Testing text content tests the deployed contract.
|
||||
// Per CONTRIBUTING.md exception matrix.
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* quick verification-status read contract (#3174)
|
||||
*
|
||||
* quick's verification step used to read the verifier's result with a raw
|
||||
* `grep "^status:" F | cut -d: -f2 | tr -d ' '` and route it through arms
|
||||
* passed / human_needed / gaps_found only. That read failed two ways,
|
||||
* both measured against the old pipeline.
|
||||
*
|
||||
* Matched NO arm: a missing report; most off-schema values; a `status:` line
|
||||
* in BOTH the frontmatter and the prose (two lines); and — on a CRLF
|
||||
* checkout — a perfectly valid `passed`, which arrives as `passed\r`.
|
||||
*
|
||||
* Matched the SUCCESS arm when it should not have: a stale report still
|
||||
* reading `passed` (staleness was never evaluated); a report whose only
|
||||
* `status:` line sits in its prose; and an off-schema value carrying a colon
|
||||
* (`passed:bogus`), which `cut -d: -f2` splits at that colon, leaving the
|
||||
* pipeline to yield `passed` once `tr -d ' '` strips the leading space.
|
||||
*
|
||||
* The unanchored match is the DEFECT.FRONTMATTER-SCALAR-BROAD-GREP class the
|
||||
* code side already fixed by name.
|
||||
*
|
||||
* These tests pin the five properties that keep the replacement honest.
|
||||
*/
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const QUICK_VERIFICATION = path.join(
|
||||
__dirname, '..', 'gsd-core', 'workflows', 'quick', 'steps', 'quick-verification.md',
|
||||
);
|
||||
// The canonical launcher preamble. scripts/sync-runtime-launcher.cjs rewrites
|
||||
// every workflow's bootstrap from this file, so THIS is the authority — not
|
||||
// whichever sibling step file happens to carry a copy today.
|
||||
const LAUNCHER_SNIPPET = path.join(
|
||||
__dirname, '..', 'gsd-core', 'workflows', '_runtime-launcher.snippet.sh',
|
||||
);
|
||||
|
||||
const SHIM_ANCHOR = '_GSD_SHIM_NAME="gsd-tools.cjs"';
|
||||
|
||||
describe('quick verification status read (#3174)', () => {
|
||||
test('status is read through the canonical query, not a raw frontmatter grep', () => {
|
||||
const content = fs.readFileSync(QUICK_VERIFICATION, 'utf-8');
|
||||
const queryIdx = content.indexOf('gsd_run query verification.status "${QUICK_DIR}"');
|
||||
|
||||
assert.ok(queryIdx !== -1, 'quick-verification.md must read status via the verification.status query');
|
||||
assert.ok(
|
||||
!content.includes('grep "^status:"'),
|
||||
'the raw frontmatter-scalar grep must not return — it matches body lines too (DEFECT.FRONTMATTER-SCALAR-BROAD-GREP)',
|
||||
);
|
||||
});
|
||||
|
||||
test('the query call is preceded by the runtime shim bootstrap in this step file', () => {
|
||||
// Step files are read and executed as their own units, so quick.md's
|
||||
// bootstrap does not reach here. Without this the call resolves to
|
||||
// nothing, 2>/dev/null swallows it, and the default arm is taken forever.
|
||||
const content = fs.readFileSync(QUICK_VERIFICATION, 'utf-8');
|
||||
const shimIdx = content.indexOf(SHIM_ANCHOR);
|
||||
const queryIdx = content.indexOf('gsd_run query verification.status');
|
||||
|
||||
assert.ok(shimIdx !== -1, 'the step file must carry its own runtime shim bootstrap');
|
||||
assert.ok(queryIdx > shimIdx, 'the shim bootstrap must precede the gsd_run call');
|
||||
});
|
||||
|
||||
test('the shim bootstrap is the canonical launcher preamble, not a fork of it', () => {
|
||||
// Anchored on _runtime-launcher.snippet.sh rather than on a sibling step
|
||||
// file: sync-runtime-launcher.cjs regenerates every workflow from the
|
||||
// snippet, so a synchronized launcher update keeps this green (correct),
|
||||
// and a sibling that legitimately stops calling gsd_run cannot fail us.
|
||||
const lineWithShim = (file) => fs.readFileSync(file, 'utf-8')
|
||||
.split(/\r?\n/)
|
||||
.find((line) => line.startsWith(SHIM_ANCHOR));
|
||||
|
||||
const mine = lineWithShim(QUICK_VERIFICATION);
|
||||
const canonical = lineWithShim(LAUNCHER_SNIPPET);
|
||||
|
||||
assert.ok(canonical, '_runtime-launcher.snippet.sh must carry the canonical preamble');
|
||||
assert.equal(mine, canonical, 'the bootstrap must match the canonical launcher snippet verbatim');
|
||||
});
|
||||
|
||||
test('status extraction does not depend on jq', () => {
|
||||
// #2589: a `| jq -r '.field'` pipe yields an empty variable with no
|
||||
// diagnostic wherever jq is absent (the Windows/Git-Bash default), which
|
||||
// would route a passing verification into the recovery arm.
|
||||
//
|
||||
// Scoped to the executable fence on purpose: the surrounding prose cites
|
||||
// the jq form in order to explain why it is not used, and an assertion
|
||||
// over the whole file would fire on its own rationale.
|
||||
const content = fs.readFileSync(QUICK_VERIFICATION, 'utf-8');
|
||||
const fences = content.match(/```bash\r?\n[\s\S]*?```/g) || [];
|
||||
const statusFence = fences.find((f) => f.includes('gsd_run query verification.status'));
|
||||
|
||||
assert.ok(statusFence, 'the status read must live in a bash fence');
|
||||
assert.ok(
|
||||
statusFence.includes('--pick status'),
|
||||
'the bare status must be picked by the query itself',
|
||||
);
|
||||
assert.ok(!/\|\s*jq\b/.test(statusFence), 'the status-read fence must not pipe through jq');
|
||||
});
|
||||
|
||||
test('the routing table carries a terminal arm for missing / unknown / stale', () => {
|
||||
const content = fs.readFileSync(QUICK_VERIFICATION, 'utf-8');
|
||||
const gapsIdx = content.indexOf('| `gaps_found` |');
|
||||
const fallbackIdx = content.indexOf('| anything else');
|
||||
|
||||
assert.ok(gapsIdx !== -1, 'the three verifier-status arms must remain');
|
||||
assert.ok(fallbackIdx > gapsIdx, 'a terminal arm must follow the verifier-status arms');
|
||||
|
||||
const fallbackRow = content.slice(fallbackIdx, content.indexOf('\n', fallbackIdx));
|
||||
for (const sentinel of ['missing', 'unknown', 'stale']) {
|
||||
assert.ok(
|
||||
fallbackRow.includes(sentinel),
|
||||
`the terminal arm must name the ${sentinel} sentinel the query can return`,
|
||||
);
|
||||
}
|
||||
assert.ok(
|
||||
fallbackRow.includes('VERIFICATION_STATUS'),
|
||||
'the terminal arm must set the display string consumed by the quick index row and banner',
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user