mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 22:14:25 +01:00
B5: learnings callers report a failed search instead of reading as empty
Every generated gstack-learnings-search caller ended in 2>/dev/null || true, so a missing bun or a failed search looked like 'nothing recorded'. Callers now keep stdout, capture stderr and the exit status, and print 'LEARNINGS: unavailable (<first stderr line>)' on failure (resolver callers in learnings.ts and review-army.ts, plus investigate, qa and ship's adversarial section).
This commit is contained in:
1 parent
f238001db9
commit
f69f83165d
6 files changed
+95
-7
No files matched your search
@@ -106,7 +106,8 @@ Pick ONE keyword from the hypothesis. The keyword should be a noun: the failing
|
||||
Worked examples (investigate-specific): good keywords are `auth-cookie`, `session-expiry`, `redirect-loop`. Bad: `auth.ts:47`, `fix the auth bug`, `<hypothesis-keyword>`.
|
||||
|
||||
```bash
|
||||
~/.claude/skills/gstack/bin/gstack-learnings-search --query "<your-keyword>" --limit 5 2>/dev/null || true
|
||||
{ _LERR=$(~/.claude/skills/gstack/bin/gstack-learnings-search --query "<your-keyword>" --limit 5 2>&1 >&3 3>&-); _LRC=$?; } 3>&1
|
||||
[ "$_LRC" = 0 ] || { _LERR=${_LERR%%$'\n'*}; echo "LEARNINGS: unavailable (${_LERR:-exit $_LRC})"; }
|
||||
```
|
||||
|
||||
If any learnings come back, name which one applies to your investigation in one sentence. If none come back, continue without reference — the absence of a matching prior learning is itself useful information.
|
||||
|
||||
+2
-1
@@ -158,7 +158,8 @@ only letters, digits or hyphens (e.g., `checkout-button`, `payment`), never a pa
|
||||
quotes, whitespace or other punctuation; simplify to an alphanumeric stem if needed.
|
||||
|
||||
```bash
|
||||
~/.claude/skills/gstack/bin/gstack-learnings-search --query "<your-keyword>" --limit 5 2>/dev/null || true
|
||||
{ _LERR=$(~/.claude/skills/gstack/bin/gstack-learnings-search --query "<your-keyword>" --limit 5 2>&1 >&3 3>&-); _LRC=$?; } 3>&1
|
||||
[ "$_LRC" = 0 ] || { _LERR=${_LERR%%$'\n'*}; echo "LEARNINGS: unavailable (${_LERR:-exit $_LRC})"; }
|
||||
```
|
||||
|
||||
Name an applicable learning in one sentence, or continue if none applies.
|
||||
|
||||
@@ -21,6 +21,14 @@ import { getHostConfig } from '../../hosts/index';
|
||||
// future contributors writing dangerous values.
|
||||
const QUERY_SAFE_RE = /^[A-Za-z0-9 _-]+$/;
|
||||
|
||||
/**
|
||||
* B5 (#2790): run a learnings search with its stdout intact and its stderr and
|
||||
* exit status kept, so a missing bun or a failed script prints why instead of
|
||||
* reading as "nothing recorded". Two lines: the capture, then the verdict.
|
||||
*/
|
||||
export const learningsCapture = (command: string) => `{ _LERR=$(${command} 2>&1 >&3 3>&-); _LRC=$?; } 3>&1`;
|
||||
export const LEARNINGS_VERDICT = `[ "$_LRC" = 0 ] || { _LERR=\${_LERR%%$'\\n'*}; echo "LEARNINGS: unavailable (\${_LERR:-exit $_LRC})"; }`;
|
||||
|
||||
export function generateLearningsSearch(ctx: TemplateContext, args?: string[]): string {
|
||||
// Parse query= arg. Empty value falls through to no-query (principle of least surprise:
|
||||
// a stray {{LEARNINGS_SEARCH:query=}} placeholder gets today's behavior, not a build error).
|
||||
@@ -58,7 +66,8 @@ the current behavior. Reading old notes never requires writing new ones.`;
|
||||
Search for relevant learnings from previous sessions on this project:
|
||||
|
||||
\`\`\`bash
|
||||
$GSTACK_BIN/gstack-learnings-search --limit 10${queryFlag} 2>/dev/null || true
|
||||
${learningsCapture(`$GSTACK_BIN/gstack-learnings-search --limit 10${queryFlag}`)}
|
||||
${LEARNINGS_VERDICT}
|
||||
\`\`\`
|
||||
|
||||
If learnings are found, incorporate them into your analysis. When a ${findingKind} finding
|
||||
@@ -73,10 +82,11 @@ Search for relevant learnings from previous sessions:
|
||||
_CROSS_PROJ=$(${ctx.paths.binDir}/gstack-config get cross_project_learnings 2>/dev/null || echo "unset")
|
||||
echo "CROSS_PROJECT: $_CROSS_PROJ"
|
||||
if [ "$_CROSS_PROJ" = "true" ]; then
|
||||
${ctx.paths.binDir}/gstack-learnings-search --limit 10${queryFlag} --cross-project 2>/dev/null || true
|
||||
${learningsCapture(`${ctx.paths.binDir}/gstack-learnings-search --limit 10${queryFlag} --cross-project`)}
|
||||
else
|
||||
${ctx.paths.binDir}/gstack-learnings-search --limit 10${queryFlag} 2>/dev/null || true
|
||||
${learningsCapture(`${ctx.paths.binDir}/gstack-learnings-search --limit 10${queryFlag}`)}
|
||||
fi
|
||||
${LEARNINGS_VERDICT}
|
||||
\`\`\`
|
||||
|
||||
If \`CROSS_PROJECT\` is \`unset\` (first time): ${ctx.skillName === 'plan-eng-review' ? 'Build a full decision brief from these facts and options using the preamble format, then ask and wait:' : 'Use AskUserQuestion:'}
|
||||
|
||||
@@ -11,6 +11,7 @@
|
||||
*/
|
||||
import type { TemplateContext } from './types';
|
||||
import { CC_BACKGROUND_DEFAULT_SINCE } from './constants';
|
||||
import { learningsCapture, LEARNINGS_VERDICT } from './learnings';
|
||||
|
||||
function generateSpecialistSelection(ctx: TemplateContext): string {
|
||||
const isShip = ctx.skillName === 'ship';
|
||||
@@ -100,7 +101,8 @@ Construct the prompt for each specialist. The prompt includes:
|
||||
3. Past learnings for this domain (if any exist):
|
||||
|
||||
\`\`\`bash
|
||||
${ctx.paths.binDir}/gstack-learnings-search --type pitfall --query "{specialist domain}" --limit 5 2>/dev/null || true
|
||||
${learningsCapture(`${ctx.paths.binDir}/gstack-learnings-search --type pitfall --query "{specialist domain}" --limit 5`)}
|
||||
${LEARNINGS_VERDICT}
|
||||
\`\`\`
|
||||
|
||||
If learnings are found, include them: "Past learnings for this domain: {learnings}"
|
||||
|
||||
@@ -14,7 +14,8 @@ be alphanumeric or hyphen only; simplify other characters. For example, use
|
||||
`token-or-search`, not `feat: token-or search`.
|
||||
|
||||
```bash
|
||||
~/.claude/skills/gstack/bin/gstack-learnings-search --query "<your-keyword>" --limit 5 2>/dev/null || true
|
||||
{ _LERR=$(~/.claude/skills/gstack/bin/gstack-learnings-search --query "<your-keyword>" --limit 5 2>&1 >&3 3>&-); _LRC=$?; } 3>&1
|
||||
[ "$_LRC" = 0 ] || { _LERR=${_LERR%%$'\n'*}; echo "LEARNINGS: unavailable (${_LERR:-exit $_LRC})"; }
|
||||
```
|
||||
|
||||
Name an applicable learning and its effect on the version bump or CHANGELOG in
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
/**
|
||||
* B5 (#2790): generated learnings-search callers used `2>/dev/null || true`, so
|
||||
* a missing bun or a failed search read as "no learnings recorded". Every
|
||||
* caller fence now keeps stdout, and on failure prints
|
||||
* `LEARNINGS: unavailable (<first stderr line>)`. Executed with a stub search
|
||||
* binary on Claude (full mode) and Codex (basic mode, fresh-shell prelude).
|
||||
*/
|
||||
import { afterAll, beforeAll, describe, expect, test } from 'bun:test';
|
||||
import { spawnSync } from 'child_process';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { runGeneration } from '../scripts/gen-skill-docs';
|
||||
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-learnings-callers-'));
|
||||
const out = path.join(tmp, 'render');
|
||||
const BUN_MISSING = 'gstack-learnings-search: bun not found on PATH, so learnings could not be read. Fix: install Bun (https://bun.sh), then re-run ./setup.';
|
||||
|
||||
const CALLERS: Array<{ name: string; file: (host: string) => string; hosts: string[] }> = [
|
||||
{ name: 'Prior Learnings (review)', file: h => (h === 'claude' ? 'review/SKILL.md' : '.agents/skills/gstack-review/SKILL.md'), hosts: ['claude', 'codex'] },
|
||||
{ name: 'investigate hypothesis refresh', file: h => (h === 'claude' ? 'investigate/SKILL.md' : '.agents/skills/gstack-investigate/SKILL.md'), hosts: ['claude', 'codex'] },
|
||||
{ name: 'qa fix-loop search', file: h => (h === 'claude' ? 'qa/SKILL.md' : '.agents/skills/gstack-qa/SKILL.md'), hosts: ['claude', 'codex'] },
|
||||
{ name: 'ship adversarial search', file: h => (h === 'claude' ? 'ship/sections/adversarial.md' : '.agents/skills/gstack-ship/sections/adversarial.md'), hosts: ['claude', 'codex'] },
|
||||
{ name: 'review army specialist search', file: () => 'review/sections/review-army.md', hosts: ['claude'] },
|
||||
];
|
||||
|
||||
function searchFences(file: string): string[] {
|
||||
return [...fs.readFileSync(path.join(out, file), 'utf8').matchAll(/^```bash\n([\s\S]*?)\n```$/gm)]
|
||||
.map(m => m[1]).filter(body => body.includes('gstack-learnings-search'));
|
||||
}
|
||||
|
||||
function run(fence: string, host: string, stub: string) {
|
||||
const home = fs.mkdtempSync(path.join(tmp, 'home-'));
|
||||
const root = path.join(home, host === 'claude' ? '.claude/skills/gstack' : '.codex/skills/gstack');
|
||||
for (const dir of ['bin', 'lib']) fs.mkdirSync(path.join(root, dir), { recursive: true });
|
||||
fs.writeFileSync(path.join(root, 'bin', 'gstack-learnings-search'), `#!/bin/sh\n${stub}\n`, { mode: 0o755 });
|
||||
fs.writeFileSync(path.join(root, 'bin', 'gstack-config'), '#!/bin/sh\necho false\n', { mode: 0o755 });
|
||||
const cwd = fs.mkdtempSync(path.join(tmp, 'cwd-'));
|
||||
return spawnSync('env', ['-i', `HOME=${home}`, 'PATH=/usr/bin:/bin', 'bash', '-c', fence], { cwd, encoding: 'utf8', timeout: 10_000 });
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
const r = await runGeneration({ host: 'all', outputRoot: out });
|
||||
if (r.exitCode !== 0) throw new Error(r.diagnostics.filter(d => d.kind === 'error').map(d => d.message).join('\n'));
|
||||
});
|
||||
afterAll(() => fs.rmSync(tmp, { recursive: true, force: true }));
|
||||
|
||||
describe('B5: learnings callers report a failed search', () => {
|
||||
for (const caller of CALLERS) {
|
||||
for (const host of caller.hosts) {
|
||||
test(`${host}: ${caller.name}`, () => {
|
||||
const fences = searchFences(caller.file(host));
|
||||
expect(fences.length).toBeGreaterThan(0);
|
||||
for (const fence of fences) {
|
||||
const missing = run(fence, host, `echo "${BUN_MISSING}" >&2\necho "second line" >&2\nexit 127`);
|
||||
expect(missing.stdout, missing.stderr).toContain(`LEARNINGS: unavailable (${BUN_MISSING})`);
|
||||
expect(missing.stdout).not.toContain('second line');
|
||||
|
||||
const silentFail = run(fence, host, 'exit 3');
|
||||
expect(silentFail.stdout).toContain('LEARNINGS: unavailable (exit 3)');
|
||||
|
||||
const found = run(fence, host, 'echo "LEARNING: retry-backoff (confidence 8)"');
|
||||
expect(found.stdout).toContain('LEARNING: retry-backoff (confidence 8)');
|
||||
expect(found.stdout).not.toContain('LEARNINGS: unavailable');
|
||||
|
||||
const none = run(fence, host, 'exit 0');
|
||||
expect(none.status).toBe(0);
|
||||
expect(none.stdout).not.toContain('LEARNINGS: unavailable');
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
});
|
||||
Reference in new issue
Block a user