mirror of
https://github.com/garrytan/gstack.git
synced 2026-07-17 13:07:22 +02:00
merge: origin/main into garrytan/multi-checkpoint-resume
Catches up 7 commits from main: -c6e6a21drefactor: AI slop reduction with cross-model quality review (v0.16.3.0) -7e96fe29fix: security wave 3 — 12 fixes, 7 contributors (v0.16.4.0) -23000672feat: UX behavioral foundations + ux-audit command (v0.17.0.0) -b805aa01feat: Confusion Protocol, Hermes + GBrain hosts, brain-first resolver (v0.18.0.0) -6a785c57fix: ngrok Windows build + close CI error-swallowing gap (v0.18.0.1) -0cc830b6fix: avoid tilde-in-assignment to silence Claude Code permission prompts -cc42f14adocs: gstack compact design doc (tabled pending Anthropic API) Conflict resolution: - ship/SKILL.md.tmpl line 402: HEAD had "## Step 12: Version bump" from the renumber refactor; origin/main added {{GBRAIN_SAVE_RESULTS}} above "## Step 4: Version bump". Resolved by keeping origin/main's new placeholder AND my branch's "Step 12" heading. - ship/SKILL.md: regenerated from resolved template (per CLAUDE.md policy: never resolve generated files manually). All skill docs regenerated for all 9 hosts (claude, kiro, opencode, slate, cursor, openclaw, factory, hermes, gbrain) to reflect the merged template. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -20,6 +20,8 @@ bun run dev:skill # watch mode: auto-regen + validate on change
|
||||
bun run eval:list # list all eval runs from ~/.gstack-dev/evals/
|
||||
bun run eval:compare # compare two eval runs (auto-picks most recent)
|
||||
bun run eval:summary # aggregate stats across all eval runs
|
||||
bun run slop # full slop-scan report (all files)
|
||||
bun run slop:diff # slop findings in files changed on this branch only
|
||||
```
|
||||
|
||||
`test:evals` requires `ANTHROPIC_API_KEY`. Codex E2E tests (`test/codex-e2e.test.ts`)
|
||||
@@ -66,14 +68,15 @@ gstack/
|
||||
├── hosts/ # Typed host configs (one per AI agent)
|
||||
│ ├── claude.ts # Primary host config
|
||||
│ ├── codex.ts, factory.ts, kiro.ts # Existing hosts
|
||||
│ ├── opencode.ts, slate.ts, cursor.ts, openclaw.ts # New hosts
|
||||
│ ├── opencode.ts, slate.ts, cursor.ts, openclaw.ts # IDE hosts
|
||||
│ ├── hermes.ts, gbrain.ts # Agent runtime hosts
|
||||
│ └── index.ts # Registry: exports all, derives Host type
|
||||
├── scripts/ # Build + DX tooling
|
||||
│ ├── gen-skill-docs.ts # Template → SKILL.md generator (config-driven)
|
||||
│ ├── host-config.ts # HostConfig interface + validator
|
||||
│ ├── host-config-export.ts # Shell bridge for setup script
|
||||
│ ├── host-adapters/ # Host-specific adapters (OpenClaw tool mapping)
|
||||
│ ├── resolvers/ # Template resolver modules (preamble, design, review, etc.)
|
||||
│ ├── resolvers/ # Template resolver modules (preamble, design, review, gbrain, etc.)
|
||||
│ ├── skill-check.ts # Health dashboard
|
||||
│ └── dev-skill.ts # Watch mode
|
||||
├── test/ # Skill validation + eval tests
|
||||
@@ -136,6 +139,11 @@ SKILL.md files are **generated** from `.tmpl` templates. To update docs:
|
||||
To add a new browse command: add it to `browse/src/commands.ts` and rebuild.
|
||||
To add a snapshot flag: add it to `SNAPSHOT_FLAGS` in `browse/src/snapshot.ts` and rebuild.
|
||||
|
||||
**Token ceiling:** Generated SKILL.md files must stay under 100KB (~25K tokens).
|
||||
`gen-skill-docs` warns if any file exceeds this. If a skill template grows past the
|
||||
ceiling, consider extracting optional sections into separate resolvers that only
|
||||
inject when relevant, or making verbose evaluation rubrics more concise.
|
||||
|
||||
**Merge conflicts on SKILL.md files:** NEVER resolve conflicts on generated SKILL.md
|
||||
files by accepting either side. Instead: (1) resolve conflicts on the `.tmpl` templates
|
||||
and `scripts/gen-skill-docs.ts` (the sources of truth), (2) run `bun run gen:skill-docs`
|
||||
@@ -250,6 +258,62 @@ Examples of good bisection:
|
||||
When the user says "bisect commit" or "bisect and push," split staged/unstaged
|
||||
changes into logical commits and push.
|
||||
|
||||
## Slop-scan: AI code quality, not AI code hiding
|
||||
|
||||
We use [slop-scan](https://github.com/benvinegar/slop-scan) to catch patterns where
|
||||
AI-generated code is genuinely worse than what a human would write. We are NOT trying
|
||||
to pass as human code. We are AI-coded and proud of it. The goal is code quality.
|
||||
|
||||
```bash
|
||||
npx slop-scan scan . # human-readable report
|
||||
npx slop-scan scan . --json # machine-readable for diffing
|
||||
```
|
||||
|
||||
Config: `slop-scan.config.json` at repo root (currently excludes `**/vendor/**`).
|
||||
|
||||
### What to fix (genuine quality improvements)
|
||||
|
||||
- **Empty catches around file ops** — use `safeUnlink()` (ignores ENOENT, rethrows
|
||||
EPERM/EIO). A swallowed EPERM in cleanup means silent data loss.
|
||||
- **Empty catches around process kills** — use `safeKill()` (ignores ESRCH, rethrows
|
||||
EPERM). A swallowed EPERM means you think you killed something you didn't.
|
||||
- **Redundant `return await`** — remove when there's no enclosing try block. Saves a
|
||||
microtask, signals intent.
|
||||
- **Typed exception catches** — `catch (err) { if (!(err instanceof TypeError)) throw err }`
|
||||
is genuinely better than `catch {}` when the try block does URL parsing or DOM work.
|
||||
You know what error you expect, so say so.
|
||||
|
||||
### What NOT to fix (linter gaming, not quality)
|
||||
|
||||
- **String-matching on error messages** — `err.message.includes('closed')` is brittle.
|
||||
Playwright/Chrome can change wording anytime. If a fire-and-forget operation can fail
|
||||
for ANY reason and you don't care, `catch {}` is the correct pattern.
|
||||
- **Adding comments to exempt pass-through wrappers** — "alias for active session" above
|
||||
a method just to trip slop-scan's exemption rule is noise, not documentation.
|
||||
- **Converting extension catch-and-log to selective rethrow** — Chrome extensions crash
|
||||
entirely on uncaught errors. If the catch logs and continues, that IS the right pattern
|
||||
for extension code. Don't make it throw.
|
||||
- **Tightening best-effort cleanup paths** — shutdown, emergency cleanup, and disconnect
|
||||
code should use `safeUnlinkQuiet()` (swallows ALL errors). A cleanup path that throws
|
||||
on EPERM means the rest of cleanup doesn't run. That's worse.
|
||||
|
||||
### Utilities in `browse/src/error-handling.ts`
|
||||
|
||||
| Function | Use when | Behavior |
|
||||
|----------|----------|----------|
|
||||
| `safeUnlink(path)` | Normal file deletion | Ignores ENOENT, rethrows others |
|
||||
| `safeUnlinkQuiet(path)` | Shutdown/emergency cleanup | Swallows all errors |
|
||||
| `safeKill(pid, signal)` | Sending signals | Ignores ESRCH, rethrows others |
|
||||
| `isProcessAlive(pid)` | Boolean process checks | Returns true/false, never throws |
|
||||
|
||||
### Score tracking
|
||||
|
||||
Baseline (2026-04-09, before cleanup): 100 findings, 432.8 score, 2.38 score/file.
|
||||
After cleanup: 90 findings, 358.1 score, 1.96 score/file.
|
||||
|
||||
Don't chase the number. Fix patterns that represent actual code quality problems.
|
||||
Accept findings where the "sloppy" pattern is the correct engineering choice.
|
||||
|
||||
## Community PR guardrails
|
||||
|
||||
When reviewing or merging community PRs, **always AskUserQuestion** before accepting
|
||||
|
||||
Reference in New Issue
Block a user