mirror of https://github.com/garrytan/gstack.git
fix: housekeeping sweep — telemetry integrity, persistent opt-out, context-bill accuracy, setup hang, dev-server discovery, model resolution (#2136 + v1.63 polish)
Seven small fixes, one theme (claims matching code): - telemetry-sync strips local-only fields with jq del() (structural) instead of quote-fragile sed regexes; unparseable lines are dropped, never forwarded unstripped. Sed survives only as a jq-less fallback. - telemetry-log rejects non-integer durations BEFORE the range caps, whose test(1) comparisons silently no-op on non-numerics — a malformed duration spliced raw text into the JSONL stream. - browse's local telemetry honors the persistent tier (config.yaml telemetry: off), not just the preamble's env hint — direct $B use and embedders now respect the opt-out. - gstack-context-bill --exact sees GSTACK_-promoted keys inside Conductor (conductor-env-shim wired at the CLI entry), and the TOTAL line no longer double-counts every nested skill through the root skill's walk (v1.63 deferred polish; the telemetry-sync HTTP-status outcome deferred alongside it turned out already shipped). - setup's Chromium probe is deadline-bounded (90s, background + poll-kill — macOS has no GNU timeout) and prefers Node for the launch probe everywhere (the bun --eval hang family behind #2136); the install is single-flight behind a lock dir with an actionable stale-lock message. Probe verified live on this Mac. - the review resolver's dev-server check reads CLAUDE.md and the plan file before falling back to an expanded port probe, and says how to make itself smarter next time. - eval/harness model IDs resolve through lib/eval-model.ts (GSTACK_EVAL_MODEL[_KIND] env overrides, per-kind defaults, tested) at the SDK-capture and PTY-warmup sites; the bash-embedded distill snippet mirrors the resolution inline. - memory-ingest's silent-zero shape (staged>0, imported+unchanged==0, errors==0) warns even under --quiet — a run that indexes nothing must never look healthy again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
parent
56d83c684c
commit
b9cd094981
|
|
@ -2,6 +2,7 @@
|
|||
// gstack-context-bill — token bill-of-materials for an installed skills tree.
|
||||
// All behavior lives in lib/context-bill.ts; this is the CLI shim.
|
||||
|
||||
import '../lib/conductor-env-shim'; // --exact needs GSTACK_ANTHROPIC_API_KEY promotion inside Conductor
|
||||
import { contextBillMain } from '../lib/context-bill';
|
||||
|
||||
process.exit(await contextBillMain(process.argv.slice(2)));
|
||||
|
|
|
|||
|
|
@ -197,8 +197,13 @@ RESULT=$(EVENTS_JSON="$EVENTS_JSON" DISTILL_PROMPT="$DISTILL_PROMPT" \
|
|||
const INPUT_PER_TOKEN = 1e-6;
|
||||
const OUTPUT_PER_TOKEN = 5e-6;
|
||||
|
||||
// Host-neutral model resolution (mirrors lib/eval-model.ts — this inline
|
||||
// bun -e script cannot import repo-relative libs from an arbitrary cwd).
|
||||
const distillModel = process.env.GSTACK_EVAL_MODEL_DISTILL
|
||||
|| process.env.GSTACK_EVAL_MODEL
|
||||
|| "claude-haiku-4-5-20251001";
|
||||
const resp = await client.messages.create({
|
||||
model: "claude-haiku-4-5-20251001",
|
||||
model: distillModel,
|
||||
max_tokens: 4096,
|
||||
messages: [{ role: "user", content: prompt }],
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1884,6 +1884,18 @@ async function ingestPass(args: CliArgs): Promise<BulkResult> {
|
|||
: ""),
|
||||
);
|
||||
}
|
||||
// Silent-zero pathology detector (#2144's other half): pages were staged
|
||||
// but NOTHING imported or skipped-as-unchanged. That shape hid the dead
|
||||
// ingest for months — it must be loud even under --quiet, because a run
|
||||
// that indexes nothing is otherwise indistinguishable from a healthy one.
|
||||
const importedCount = (importJson.imported ?? 0) + (importJson.skipped ?? 0);
|
||||
if (prep.prepared.length > 0 && importedCount === 0 && (importJson.errors ?? 0) === 0) {
|
||||
console.error(
|
||||
`[memory-ingest] WARNING: ${prep.prepared.length} page(s) staged but gbrain collected ZERO ` +
|
||||
`(no imports, no unchanged-skips, no errors). This is the #2144 silent-zero shape — ` +
|
||||
`check gbrain's import.collect_files log line and your gbrain version.`,
|
||||
);
|
||||
}
|
||||
} finally {
|
||||
// #1802 D1: in remote-http mode `stagingDir` is the PERSISTENT transcript
|
||||
// dir (makePersistentTranscriptDir, under ~/.gstack/transcripts/) that
|
||||
|
|
|
|||
|
|
@ -210,6 +210,13 @@ fi
|
|||
STEP_FIELD="null"
|
||||
[ -n "$FAILED_STEP" ] && STEP_FIELD="\"$(json_safe "$FAILED_STEP")\""
|
||||
|
||||
# Integrity first: a non-numeric duration would splice raw text into the
|
||||
# JSON line ("duration_s":%s) and corrupt the whole JSONL stream — the range
|
||||
# caps below silently no-op on non-integers because both test(1) comparisons
|
||||
# fail. Reject anything that isn't a plain integer.
|
||||
case "$DURATION" in
|
||||
''|*[!0-9]*) DURATION="" ;;
|
||||
esac
|
||||
# Cap unreasonable durations
|
||||
if [ -n "$DURATION" ] && [ "$DURATION" -gt 86400 ] 2>/dev/null; then
|
||||
DURATION="" # null if > 24h
|
||||
|
|
|
|||
|
|
@ -81,15 +81,27 @@ while IFS= read -r LINE; do
|
|||
[ -z "$LINE" ] && continue
|
||||
echo "$LINE" | grep -q '^{' || continue
|
||||
|
||||
# Strip local-only fields (keep v, ts, sessions as-is for edge function)
|
||||
CLEAN="$(echo "$LINE" | sed \
|
||||
-e 's/,"_repo_slug":"[^"]*"//g' \
|
||||
-e 's/,"_branch":"[^"]*"//g' \
|
||||
-e 's/,"repo":"[^"]*"//g')"
|
||||
|
||||
# If anonymous tier, strip installation_id
|
||||
if [ "$TIER" = "anonymous" ]; then
|
||||
CLEAN="$(echo "$CLEAN" | sed 's/,"installation_id":"[^"]*"//g; s/,"installation_id":null//g')"
|
||||
# Strip local-only fields (keep v, ts, sessions as-is for edge function).
|
||||
# jq del() is structural — a value containing an escaped quote (repo names,
|
||||
# branch names) can't smuggle the field past a regex or corrupt the strip.
|
||||
# The sed path stays only as a jq-less fallback.
|
||||
if command -v jq >/dev/null 2>&1; then
|
||||
if [ "$TIER" = "anonymous" ]; then
|
||||
CLEAN="$(printf '%s' "$LINE" | jq -c 'del(._repo_slug, ._branch, .repo, .installation_id)' 2>/dev/null)" || CLEAN=""
|
||||
else
|
||||
CLEAN="$(printf '%s' "$LINE" | jq -c 'del(._repo_slug, ._branch, .repo)' 2>/dev/null)" || CLEAN=""
|
||||
fi
|
||||
# A line jq can't parse is malformed telemetry — drop it rather than
|
||||
# forwarding bytes the strip never touched.
|
||||
[ -z "$CLEAN" ] && continue
|
||||
else
|
||||
CLEAN="$(echo "$LINE" | sed \
|
||||
-e 's/,"_repo_slug":"[^"]*"//g' \
|
||||
-e 's/,"_branch":"[^"]*"//g' \
|
||||
-e 's/,"repo":"[^"]*"//g')"
|
||||
if [ "$TIER" = "anonymous" ]; then
|
||||
CLEAN="$(echo "$CLEAN" | sed 's/,"installation_id":"[^"]*"//g; s/,"installation_id":null//g')"
|
||||
fi
|
||||
fi
|
||||
|
||||
if [ "$FIRST" = "true" ]; then
|
||||
|
|
|
|||
|
|
@ -50,8 +50,23 @@ function isDisabled(): boolean {
|
|||
telemetryDisabled = true;
|
||||
return true;
|
||||
}
|
||||
// Persistent tier: gstack-config set telemetry off must hold even when the
|
||||
// daemon is spawned outside a skill preamble (direct $B use, embedders) and
|
||||
// the env hint was never set (fork port wave 2 polish).
|
||||
try {
|
||||
const fs = require('fs') as typeof import('fs');
|
||||
const path = require('path') as typeof import('path');
|
||||
const os = require('os') as typeof import('os');
|
||||
const home = process.env.GSTACK_HOME || path.join(os.homedir(), '.gstack');
|
||||
const yaml = fs.readFileSync(path.join(home, 'config.yaml'), 'utf-8');
|
||||
if (/^\s*telemetry\s*:\s*['"]?off['"]?\s*(?:#.*)?$/m.test(yaml)) {
|
||||
telemetryDisabled = true;
|
||||
return true;
|
||||
}
|
||||
} catch { /* no config — fall through to default */ }
|
||||
// Conservative default: telemetry ON unless explicitly off. Users opt out via
|
||||
// gstack-config set telemetry off (preamble reads this; we trust the env hint).
|
||||
// gstack-config set telemetry off (env hint from the preamble OR the
|
||||
// persistent tier read above).
|
||||
telemetryDisabled = false;
|
||||
return false;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -222,6 +222,20 @@ function totalMd(dir: string, tokensOf: TokensOf): { bytes: number; tokens: numb
|
|||
let bytes = 0;
|
||||
let tokens = 0;
|
||||
for (const p of walkMd(dir)) {
|
||||
// A skill dir that CONTAINS other skill dirs (the gstack root skill wraps
|
||||
// the whole tree) must not swallow its children's files: each nested
|
||||
// skill reports its own totalMd, and the grand total sums per-skill
|
||||
// figures — counting them here again double-counted every nested skill
|
||||
// in the TOTAL line (v1.63 deferred polish, fixed in fork port wave 2).
|
||||
const rel = path.relative(dir, p);
|
||||
const topSeg = rel.split(path.sep)[0];
|
||||
if (
|
||||
topSeg &&
|
||||
topSeg !== rel && // p is inside a subdirectory
|
||||
fs.existsSync(path.join(dir, topSeg, "SKILL.md"))
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
const b = bytesOf(p) ?? 0;
|
||||
bytes += b;
|
||||
tokens += tokensOf(p, b);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,40 @@
|
|||
/**
|
||||
* Host-neutral eval/harness model resolution (fork port wave 2, G cluster).
|
||||
*
|
||||
* Model IDs were hardcoded at six call sites across the eval helpers and the
|
||||
* distill bin, so an environment that pins a different model (CI cost
|
||||
* control, a model migration, an air-gapped proxy alias) had to patch source.
|
||||
* One resolution point, env-overridable:
|
||||
*
|
||||
* GSTACK_EVAL_MODEL_<KIND> (e.g. GSTACK_EVAL_MODEL_WARMUP) — per-kind
|
||||
* GSTACK_EVAL_MODEL — global
|
||||
* explicit argument — caller wins
|
||||
* per-kind default — last resort
|
||||
*
|
||||
* Kinds and their defaults:
|
||||
* capture — AskUserQuestion SDK capture runs (quality matters): opus
|
||||
* warmup — PTY warm-up ping (cheapest thing that answers): haiku
|
||||
* distill — free-text distillation (cheap, structured): haiku (pinned)
|
||||
*/
|
||||
|
||||
const DEFAULTS: Record<string, string> = {
|
||||
capture: "claude-opus-4-7",
|
||||
warmup: "claude-haiku-4-5",
|
||||
distill: "claude-haiku-4-5-20251001",
|
||||
};
|
||||
|
||||
export type EvalModelKind = keyof typeof DEFAULTS & string;
|
||||
|
||||
export function resolveEvalModel(
|
||||
kind: EvalModelKind,
|
||||
explicit?: string | null,
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
): string {
|
||||
if (explicit) return explicit;
|
||||
const perKind = env[`GSTACK_EVAL_MODEL_${kind.toUpperCase()}`];
|
||||
if (perKind) return perKind;
|
||||
if (env.GSTACK_EVAL_MODEL) return env.GSTACK_EVAL_MODEL;
|
||||
const fallback = DEFAULTS[kind];
|
||||
if (!fallback) throw new Error(`resolveEvalModel: unknown kind "${kind}"`);
|
||||
return fallback;
|
||||
}
|
||||
|
|
@ -1147,16 +1147,23 @@ Using the plan file already discovered in Step 8, look for a verification sectio
|
|||
|
||||
### 2. Check for running dev server
|
||||
|
||||
Before invoking browse-based verification, check if a dev server is reachable:
|
||||
Before invoking browse-based verification, find the dev-server URL the way the
|
||||
project declares it — never trust a hardcoded port list alone:
|
||||
|
||||
1. **CLAUDE.md first:** look for a documented dev URL or dev command (a
|
||||
\`## Development\`/\`## Testing\` section naming a port or URL). Use it.
|
||||
2. **The plan file:** if the plan's verification section names a URL, use it.
|
||||
3. **Fallback probe** (common ports, only when 1-2 found nothing):
|
||||
|
||||
\`\`\`bash
|
||||
curl -s -o /dev/null -w '%{http_code}' http://localhost:3000 2>/dev/null || \\
|
||||
curl -s -o /dev/null -w '%{http_code}' http://localhost:8080 2>/dev/null || \\
|
||||
curl -s -o /dev/null -w '%{http_code}' http://localhost:5173 2>/dev/null || \\
|
||||
curl -s -o /dev/null -w '%{http_code}' http://localhost:4000 2>/dev/null || echo "NO_SERVER"
|
||||
for _p in 3000 8080 5173 4000 4321 8000; do
|
||||
_code=$(curl -s -o /dev/null -w '%{http_code}' "http://localhost:$_p" 2>/dev/null)
|
||||
[ -n "$_code" ] && [ "$_code" != "000" ] && { echo "DEV_SERVER: http://localhost:$_p ($_code)"; break; }
|
||||
done
|
||||
[ -z "\${_code:-}" ] || [ "\${_code:-000}" = "000" ] && echo "NO_SERVER"
|
||||
\`\`\`
|
||||
|
||||
**If NO_SERVER:** Skip with "No dev server detected — skipping plan verification. Run /qa separately after deploying."
|
||||
**If NO_SERVER:** Skip with "No dev server detected (checked CLAUDE.md, the plan, and common ports) — skipping plan verification. Run /qa separately after deploying, or document the dev URL in CLAUDE.md so this step finds it next time."
|
||||
|
||||
### 3. Invoke /qa-only inline
|
||||
|
||||
|
|
|
|||
66
setup
66
setup
|
|
@ -249,20 +249,45 @@ if [ "$INSTALL_CODEX" -eq 1 ]; then
|
|||
migrate_direct_codex_install "$SOURCE_GSTACK_DIR" "$CODEX_GSTACK"
|
||||
fi
|
||||
|
||||
# Deadline-bounded wait for a background probe. macOS ships no GNU timeout;
|
||||
# poll the PID and SIGKILL past the deadline. Returns the probe's exit code,
|
||||
# or 124 on timeout.
|
||||
_wait_with_deadline() {
|
||||
local pid="$1" deadline_s="$2" waited=0
|
||||
while kill -0 "$pid" 2>/dev/null; do
|
||||
if [ "$waited" -ge "$deadline_s" ]; then
|
||||
kill -9 "$pid" 2>/dev/null || true
|
||||
wait "$pid" 2>/dev/null || true
|
||||
return 124
|
||||
fi
|
||||
sleep 1
|
||||
waited=$((waited + 1))
|
||||
done
|
||||
wait "$pid"
|
||||
}
|
||||
|
||||
ensure_playwright_browser() {
|
||||
if [ "$IS_WINDOWS" -eq 1 ]; then
|
||||
# On Windows, Bun can't launch Chromium due to broken pipe handling
|
||||
# (oven-sh/bun#4253). Use Node.js to verify Chromium works instead.
|
||||
(
|
||||
cd "$SOURCE_GSTACK_DIR"
|
||||
node -e "const { chromium } = require('playwright'); (async () => { const b = await chromium.launch(); await b.close(); })()" 2>/dev/null
|
||||
)
|
||||
# #2136: fresh installs hung forever at this probe (macOS arm64) and
|
||||
# re-runs stacked stuck process trees, so skills never got linked. Two
|
||||
# fixes: prefer Node for the launch probe everywhere it exists (the
|
||||
# bun --eval launch is the same pipe-bug family already worked around on
|
||||
# Windows), and bound the probe with a 90s deadline — a wedged probe now
|
||||
# reports failure (which routes to the install path) instead of hanging
|
||||
# setup.
|
||||
local probe_cmd
|
||||
if command -v node >/dev/null 2>&1; then
|
||||
probe_cmd='node -e "const { chromium } = require((process.cwd()) + \"/node_modules/playwright\"); (async () => { const b = await chromium.launch(); await b.close(); })().then(() => process.exit(0), () => process.exit(1))"'
|
||||
elif [ "$IS_WINDOWS" -eq 1 ]; then
|
||||
echo "gstack setup failed: Node.js is required on Windows" >&2
|
||||
return 1
|
||||
else
|
||||
(
|
||||
cd "$SOURCE_GSTACK_DIR"
|
||||
bun --eval 'import { chromium } from "playwright"; const browser = await chromium.launch(); await browser.close();'
|
||||
) >/dev/null 2>&1
|
||||
probe_cmd="bun --eval 'import { chromium } from \"playwright\"; const browser = await chromium.launch(); await browser.close();'"
|
||||
fi
|
||||
(
|
||||
cd "$SOURCE_GSTACK_DIR"
|
||||
eval "$probe_cmd"
|
||||
) >/dev/null 2>&1 &
|
||||
_wait_with_deadline $! 90
|
||||
}
|
||||
|
||||
# Ensure a color-emoji font is installed (Linux only).
|
||||
|
|
@ -478,10 +503,21 @@ fi
|
|||
# 2. Ensure Playwright's Chromium is available
|
||||
if ! ensure_playwright_browser; then
|
||||
echo "Installing Playwright Chromium..."
|
||||
(
|
||||
cd "$SOURCE_GSTACK_DIR"
|
||||
bunx playwright install chromium
|
||||
)
|
||||
_PW_LOCK="${TMPDIR:-/tmp}/gstack-playwright-install.lock"
|
||||
if mkdir "$_PW_LOCK" 2>/dev/null; then
|
||||
trap 'rmdir "$_PW_LOCK" 2>/dev/null || true' EXIT
|
||||
(
|
||||
cd "$SOURCE_GSTACK_DIR"
|
||||
bunx playwright install chromium
|
||||
)
|
||||
rmdir "$_PW_LOCK" 2>/dev/null || true
|
||||
trap - EXIT
|
||||
else
|
||||
echo " another gstack setup is already installing Chromium (lock: $_PW_LOCK)." >&2
|
||||
echo " Wait for it to finish, then re-run ./setup. If no other setup is running," >&2
|
||||
echo " remove the stale lock: rmdir \"$_PW_LOCK\"" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
if [ "$IS_WINDOWS" -eq 1 ]; then
|
||||
# On Windows, Node.js launches Chromium (not Bun — see oven-sh/bun#4253).
|
||||
|
|
|
|||
|
|
@ -0,0 +1,23 @@
|
|||
/** OV11: the host-neutral eval-model resolver's contract. */
|
||||
import { describe, test, expect } from "bun:test";
|
||||
import { resolveEvalModel } from "../lib/eval-model";
|
||||
|
||||
describe("resolveEvalModel", () => {
|
||||
test("explicit argument wins over everything", () => {
|
||||
expect(resolveEvalModel("capture", "my-model", { GSTACK_EVAL_MODEL: "x" } as never)).toBe("my-model");
|
||||
});
|
||||
test("per-kind env beats the global env", () => {
|
||||
expect(resolveEvalModel("warmup", null, { GSTACK_EVAL_MODEL_WARMUP: "w", GSTACK_EVAL_MODEL: "g" } as never)).toBe("w");
|
||||
});
|
||||
test("global env beats the default", () => {
|
||||
expect(resolveEvalModel("distill", null, { GSTACK_EVAL_MODEL: "g" } as never)).toBe("g");
|
||||
});
|
||||
test("defaults per kind", () => {
|
||||
expect(resolveEvalModel("capture", null, {} as never)).toBe("claude-opus-4-7");
|
||||
expect(resolveEvalModel("warmup", null, {} as never)).toBe("claude-haiku-4-5");
|
||||
expect(resolveEvalModel("distill", null, {} as never)).toBe("claude-haiku-4-5-20251001");
|
||||
});
|
||||
test("unknown kind throws instead of silently defaulting", () => {
|
||||
expect(() => resolveEvalModel("banana" as never, null, {} as never)).toThrow();
|
||||
});
|
||||
});
|
||||
|
|
@ -11,6 +11,7 @@
|
|||
* zero rendering loss. The TTY rendering layer is identical for fat and slim
|
||||
* skills, so it is not where token-reduction degradation can hide.
|
||||
*/
|
||||
import { resolveEvalModel } from '../../lib/eval-model';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
|
|
@ -191,7 +192,7 @@ This is a capture test, not an interactive session. Skip any system-audit / envi
|
|||
timeout: 240_000,
|
||||
testName: opts.testName,
|
||||
runId: opts.runId,
|
||||
model: opts.model ?? 'claude-opus-4-7',
|
||||
model: resolveEvalModel('capture', opts.model),
|
||||
});
|
||||
|
||||
try {
|
||||
|
|
@ -253,7 +254,7 @@ Rules for this run:
|
|||
timeout: opts.timeout ?? 300_000,
|
||||
testName: opts.testName,
|
||||
runId: opts.runId,
|
||||
model: opts.model ?? 'claude-opus-4-7',
|
||||
model: resolveEvalModel('capture', opts.model),
|
||||
});
|
||||
|
||||
const readSections = new Set<string>();
|
||||
|
|
@ -334,7 +335,7 @@ Write the verbatim text of that AskUserQuestion (the full decision brief: title,
|
|||
timeout: 240_000,
|
||||
testName: opts.testName,
|
||||
runId: opts.runId,
|
||||
model: opts.model ?? 'claude-opus-4-7',
|
||||
model: resolveEvalModel('capture', opts.model),
|
||||
});
|
||||
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@
|
|||
* tests don't need it).
|
||||
*/
|
||||
|
||||
import { resolveEvalModel } from '../../lib/eval-model';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
|
|
@ -456,7 +457,7 @@ ${tail}
|
|||
try {
|
||||
const result = nodeSpawnSync(
|
||||
'claude',
|
||||
['-p', '--model', 'claude-haiku-4-5', '--max-turns', '1'],
|
||||
['-p', '--model', resolveEvalModel('warmup'), '--max-turns', '1'],
|
||||
{
|
||||
input: prompt,
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
|
|
|
|||
Loading…
Reference in New Issue