mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-13 03:48:06 +01:00
fix: adversarial review fixes
Trailer fix after a skipped upgrade is one runnable command (the refusal prose stays on the WARN behind row); the floor reader is whitespace-insensitive and names a non-version value instead of calling it absent. Found by /ship Step 11 (Claude adversarial, Codex adversarial + structured). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
fc69d3ff0b
commit
761a22b38b
5 files changed
+23
-6
No files matched your search
+3
-1
@@ -14,10 +14,12 @@
|
||||
- **A Node-only pin mismatch is advisory** (`bin/gstack-doctor-check.sh`): when the project's Node pin is unmet by the Node on PATH while every Bun pin is met, `pins` is `WARN pins — ... (node pin only; bun pins pass); fix: ... (PROJECT_NODE_PIN_UNMET)` and the trailer stays ok. gstack runs on Bun and the project's tests choose their Node (nvm, the CI matrix). A Bun pin that is unmet stays `FAIL pins` (`PROJECT_PIN_MISMATCH`).
|
||||
- **WARN rows print their action as given** (`_dc_row`): `owner action: ...` for the Capy skill copy, `fix: ...` for the floor, a skipped upgrade and a Node pin, so a lane knows which WARNs it can clear itself.
|
||||
- **A Drive copy below the wrapper minimum is a WARN in a direct run too** (`bin/gstack-doctor-check.sh`): with no bootstrapper provenance (`bin/gstack-upgrade`'s installer run, a direct `gstack-doctor --check`), every Drive copy at a bootstrapper revision below `GSTACK_CAPY_MIN_WRAPPER` is `WARN wrapper — every Capy skill copy is below the minimum wrapper 3 (it still works): ...` with the owner action and an ok trailer; only a pre-bootstrapper copy (no `GSTACK_CAPY_WRAPPER=` line) still fails. Found by /ship's red team: raising the minimum to 3 would otherwise have failed every direct run on a machine whose Drive copy is the deployed wrapper 2 until the owner recopied the skill.
|
||||
- **A `--for none` session never self-upgrades** (`bin/gstack-capy-session`): an older bootstrapper (wrapper 1 or 2) hands `--for none` to the session in install mode; the session now treats it like `--check` (reports `behind=`, moves nothing) before the installer prints `ok not-needed`. Wrapper 3 answers `--for none` before the session. The session's header comment describes the `WARN behind` outcome (it said `fail behind`).
|
||||
- **A `--for none` session never self-upgrades** (`bin/gstack-capy-session`): an older bootstrapper (wrapper 1 or 2) hands `--for none` to the session in install mode; the session now treats it like `--check` (reports `behind=`, moves nothing) before the installer prints `ok not-needed`; the old bootstrapper's own clone or fast-forward of a below-minimum checkout, before the handover, is unchanged. Wrapper 3 answers `--for none` before the session. The session's header comment describes the `WARN behind` outcome (it said `fail behind`).
|
||||
- **/cso** (`cso/SKILL.md.tmpl`): the always-loaded startup block gains two rules. Environment hygiene: prerequisite checks read a variable as set or unset only, and `env`, `printenv`, `set -x`, a bare `set` or `export`, `export $(...)` and `echo "$NAME"` never appear in a tool fence. Tooling errors fail loudly: a non-zero exit with no helper status is retried at most twice (5 s, 30 s) and then ends **not assessed** for whatever the helper never recorded, quoting the command, exit code and stderr tail; never a quiet partial, never another review offered as the missing coverage.
|
||||
- **Capy skill and docs** (`capy/skill/gstack/SKILL.md`, `docs/capy-install.md`): the session-start steps cover `--for none`, `floor=`, `behind=` as informational, the Node WARN and the in-place review-voice upgrade; run directories and scratch on Capy live under `~/.capy/work/<name>`, never `/tmp`, which Capy wipes when a machine sleeps.
|
||||
- **Result codes** (`lib/result-codes-capy.ts`): `PROJECT_FLOOR_UNMET`, `PROJECT_NODE_PIN_UNMET` and `UPGRADE_SKIPPED`, each with a `docs/troubleshooting.md` anchor.
|
||||
- **The trailer's `fix:` after a skipped upgrade is one runnable command** (`bin/gstack-capy-install`): the session's rerun or `bin/gstack-upgrade` (which prints its own refusal), never the refusal's prose; that prose, with its `GSTACK_UPGRADE_FORCE=1` escape hatch for a live lease, stays on the `WARN behind` row (found by /ship's adversarial review: an agent told to run the `fix:` command was being handed prose with the note separator in it).
|
||||
- **The floor reader is whitespace-insensitive** (`bin/gstack-doctor-check.sh`): `min_gstack_version` on one line and its value on the next, which `lib/ship-policy.ts` accepts, is read; a present value that is not a dotted version SKIPs as such instead of as "null or absent" (found by Codex in /ship's adversarial review).
|
||||
|
||||
### Tests
|
||||
- `test/gstack-doctor.test.ts`: the `floor` row's four outcomes (absent file, null key, met, unmet with the wrapper fix and without it) and the Node-only WARN beside a Bun-plus-Node FAIL; every required-row list gains `floor`.
|
||||
|
||||
@@ -612,7 +612,10 @@ fi
|
||||
if [ -n "${GSTACK_CAPY_BEHIND:-}" ]; then
|
||||
note "behind=$GSTACK_CAPY_BEHIND (latest: ${GSTACK_CAPY_BEHIND_TO:-${REMOTE_VERSION:-unknown}})"
|
||||
[ -n "$BLOCKED" ] && note "upgrade skipped: $BLOCKED_REASON ($BLOCKED_CODE)"
|
||||
FIX_NOTE="${BLOCKED:+$BLOCKED_FIX}"; FIX_NOTE="${FIX_NOTE:-${GSTACK_CAPY_BEHIND_FIX:-bash <skill>/scripts/install.sh}}"
|
||||
# The trailer's fix is always one runnable command (the session's: the rerun
|
||||
# or bin/gstack-upgrade, which prints its own refusal); the refusal's own
|
||||
# prose stays on the WARN behind row.
|
||||
FIX_NOTE="${GSTACK_CAPY_BEHIND_FIX:-${BLOCKED:+$ROOT/bin/gstack-upgrade}}"; FIX_NOTE="${FIX_NOTE:-bash <skill>/scripts/install.sh}"
|
||||
elif [ -n "$REMOTE_VERSION" ] && [ "$REMOTE_VERSION" != "$INSTALLED" ] \
|
||||
&& [ "$(printf '%s\n%s\n' "$INSTALLED" "$REMOTE_VERSION" | sort -V | head -1)" = "$INSTALLED" ]; then
|
||||
note "latest: $REMOTE_VERSION available"
|
||||
|
||||
@@ -492,9 +492,14 @@ _dc_check_floor() {
|
||||
_dc_row SKIP floor "$project declares none ($GSTACK_PROJECT_FLOOR_FILE absent; set $GSTACK_PROJECT_FLOOR_FIELD there to require a gstack version)"
|
||||
return
|
||||
fi
|
||||
floor=$(sed -n 's/.*"'"$GSTACK_PROJECT_FLOOR_FIELD"'"[[:space:]]*:[[:space:]]*"\([0-9][0-9.]*\)".*/\1/p' "$file" 2>/dev/null | head -1 | clamp '0-9.' 32)
|
||||
# Whitespace-insensitive: the value may sit on the line after its field.
|
||||
floor=$(tr '\n\r\t' ' ' < "$file" 2>/dev/null | sed -n 's/.*"'"$GSTACK_PROJECT_FLOOR_FIELD"'" *: *"\([0-9][0-9.]*\)".*/\1/p' | head -1 | clamp '0-9.' 32)
|
||||
if [ -z "$floor" ]; then
|
||||
_dc_row SKIP floor "$file sets no $GSTACK_PROJECT_FLOOR_FIELD (null or absent)"
|
||||
if tr '\n\r\t' ' ' < "$file" 2>/dev/null | grep -q '"'"$GSTACK_PROJECT_FLOOR_FIELD"'" *: *"'; then
|
||||
_dc_row SKIP floor "$file sets $GSTACK_PROJECT_FLOOR_FIELD to something other than a dotted version (gstack-ship-queue rejects it)"
|
||||
else
|
||||
_dc_row SKIP floor "$file sets no $GSTACK_PROJECT_FLOOR_FIELD (null or absent)"
|
||||
fi
|
||||
return
|
||||
fi
|
||||
installed=$(head -1 "$ROOT/VERSION" 2>/dev/null | clamp '0-9A-Za-z.+-' 40)
|
||||
|
||||
@@ -287,7 +287,8 @@ describe('gstack-capy-install', () => {
|
||||
const busy = { GSTACK_CAPY_BEHIND: '3', GSTACK_CAPY_BEHIND_TO: '1.2.6', GSTACK_CAPY_BEHIND_FIX: `${f.root}/bin/gstack-upgrade`, GSTACK_CAPY_BEHIND_BLOCKED: 'live gstack sessions hold this installation: pid 379 (124m dbus-daemon); fix: wait for those sessions to finish, or re-run with GSTACK_UPGRADE_FORCE=1 when you know they are idle (UPGRADE_BUSY)' };
|
||||
const r = run(f, [], busy);
|
||||
expect(r.status, r.stdout + r.stderr).toBe(0);
|
||||
expect(lastLine(r.stdout)).toBe('gstack: ok 1.2.3 for=autoplan (behind=3 (latest: 1.2.6); upgrade skipped: live gstack sessions hold this installation: pid 379 (124m dbus-daemon) (UPGRADE_BUSY); fix: wait for those sessions to finish, or re-run with GSTACK_UPGRADE_FORCE=1 when you know they are idle)');
|
||||
// The trailer's fix is the session's one runnable command; the refusal's own prose (and its GSTACK_UPGRADE_FORCE=1 escape hatch) stays on the WARN row.
|
||||
expect(lastLine(r.stdout)).toBe(`gstack: ok 1.2.3 for=autoplan (behind=3 (latest: 1.2.6); upgrade skipped: live gstack sessions hold this installation: pid 379 (124m dbus-daemon) (UPGRADE_BUSY); fix: ${f.root}/bin/gstack-upgrade)`);
|
||||
expect(r.stdout).toMatch(/^WARN behind — installed 1\.2\.3 is 3 commits behind origin \(latest 1\.2\.6\) and the session-start upgrade was skipped: live gstack sessions hold this installation: pid 379 \(124m dbus-daemon\); fix: wait for those sessions to finish, or re-run with GSTACK_UPGRADE_FORCE=1 when you know they are idle \(UPGRADE_BUSY; .*#upgrade-busy\)$/m);
|
||||
expect(r.stdout).toMatch(/^GSTACK_LAUNCH_FILE: /m);
|
||||
const bare = run(f, [], { ...busy, GSTACK_CAPY_BEHIND_BLOCKED: 'gstack-upgrade exit 9' });
|
||||
@@ -296,7 +297,7 @@ describe('gstack-capy-install', () => {
|
||||
// A code the installer does not know collapses to UPGRADE_SKIPPED, whose anchor exists; the reason and fix are kept.
|
||||
const other = run(f, [], { ...busy, GSTACK_CAPY_BEHIND_BLOCKED: 'origin unreachable; fix: retry when online (UPGRADE_FETCH_FAILED)' });
|
||||
expect(other.status).toBe(0);
|
||||
expect(lastLine(other.stdout)).toBe('gstack: ok 1.2.3 for=autoplan (behind=3 (latest: 1.2.6); upgrade skipped: origin unreachable (UPGRADE_SKIPPED); fix: retry when online)');
|
||||
expect(lastLine(other.stdout)).toBe(`gstack: ok 1.2.3 for=autoplan (behind=3 (latest: 1.2.6); upgrade skipped: origin unreachable (UPGRADE_SKIPPED); fix: ${f.root}/bin/gstack-upgrade)`);
|
||||
expect(other.stdout).toMatch(/^WARN behind — .*; fix: retry when online \(UPGRADE_SKIPPED; .*#upgrade-skipped\)$/m);
|
||||
expect(lastLine(run(f, [], { ...busy, STUB_DOCTOR_FAIL: '1' }).stdout)).toBe('gstack: fail codex');
|
||||
});
|
||||
|
||||
@@ -672,9 +672,15 @@ describe('gstack-doctor --check', () => {
|
||||
expect(none.rows.floor).toEqual({ status: 'SKIP', rest: `(${project} declares none (.gstack/ship-policy.json absent; set min_gstack_version there to require a gstack version))` });
|
||||
write(path.join(project, '.gstack/ship-policy.json'), '{\n "bump": "patch",\n "min_gstack_version": null\n}\n');
|
||||
expect(row(f, ['--project', project]).rows.floor).toEqual({ status: 'SKIP', rest: `(${project}/.gstack/ship-policy.json sets no min_gstack_version (null or absent))` });
|
||||
// A present value that is not a dotted version is named as such, never read as "absent"; the shell never evaluates it.
|
||||
write(path.join(project, '.gstack/ship-policy.json'), '{\n "min_gstack_version": "1.2.3-rc1; $(touch /tmp/pwned)"\n}\n');
|
||||
expect(row(f, ['--project', project]).rows.floor).toEqual({ status: 'SKIP', rest: `(${project}/.gstack/ship-policy.json sets min_gstack_version to something other than a dotted version (gstack-ship-queue rejects it))` });
|
||||
write(path.join(project, '.gstack/ship-policy.json'), `{\n "min_gstack_version": "${VERSION}"\n}\n`);
|
||||
const met = row(f, ['--project', project]);
|
||||
expect(met.rows.floor).toEqual({ status: 'PASS', rest: `${VERSION} declared by ${project}/.gstack/ship-policy.json (installed ${VERSION})` });
|
||||
// Valid JSON may put the value on the line after its field (found by Codex in /ship's adversarial review).
|
||||
write(path.join(project, '.gstack/ship-policy.json'), `{\n "bump": "patch",\n "min_gstack_version":\n "${VERSION}"\n}\n`);
|
||||
expect(row(f, ['--project', project]).rows.floor.status).toBe('PASS');
|
||||
write(path.join(project, '.gstack/ship-policy.json'), '{\n "min_gstack_version": "999.0.0.0"\n}\n');
|
||||
const wrapper = '/drive/skills/gstack/scripts/install.sh';
|
||||
const unmet = row(f, ['--project', project, '--for', 'health'], { GSTACK_CAPY_WRAPPER_PATH: wrapper });
|
||||
|
||||
Reference in new issue
Block a user