fix(skill): findings from round-5 team review
- Зачем:
- round-5 adversarial review (claude-team-review) нашёл 2 HIGH и 3 MEDIUM, которых не увидел round-4 self-review. Основное — регрессия в b213678, где при рефакторе primary-path silently дропнуло malformed-JSON-first-line handling, и противоречие в Step 7 step 4 между «same approach as Step 4 check 4» и «keep previous id».
- Что:
- SKILL.md Step 4 check 4: primary-path с явными ветками — valid UUID → save; любой другой случай (empty / malformed / missing thread_id / partial output) → fallthrough на secondary. Secondary описан как двухпричинный (sandbox suppression + format drift), не только «0 bytes».
- SKILL.md Step 7 step 4: явно разведено с Step 4 check 4 — zero-find в resume НЕ абортит round, а keep previous CODEX_SESSION_ID (§2.4.4 гарантирует что thread id не ротируется). Добавлен warning-сообщение.
- SKILL.md Step 2/4/7/Rules: убран `echo $(($(date +%s) - 1))` Bash-вызов, timestamp считается Opus'ом в reasoning и подставляется литералом. Убирает compound-command permission-матч проблему (`$()` + `- 1` арифметика) и один Bash-круг на раунд.
- SKILL.md Step 7 fresh-exec fallback: архивирует failed-resume артефакты через `mv` в `*-failed-resume.{jsonl,txt}` ПЕРЕД fresh exec. Step 9 cleanup glob расширен.
- DESIGN.md §9.5: cross-ref «Step 4 check 3» → «Step 4 check 4».
- DESIGN.md §4.1 trade-offs: описание CODEX_SESSIONS_BEFORE переписано под in-reasoning capture.
- README.md: убрана `Bash(date +%s)` permission, добавлены `Bash(mv ...)` для архивации.
- Проверка:
- Повторно прогнать self-review с фокусом на: (а) понятен ли novice reader malformed-case fallthrough, (б) не противоречит ли Step 7 step 4 Step 4 check 4 после правки.
This commit is contained in:
+9
-7
@@ -484,12 +484,14 @@ Each decision below follows the same template:
|
||||
only) — load-bearing for `§4.9`.
|
||||
- Secondary path introduces a filesystem race against parallel
|
||||
codex invocations (§9.1 scope). Mitigated (not eliminated) by
|
||||
the pre-exec timestamp (`CODEX_SESSIONS_BEFORE=$(($(date +%s) -
|
||||
1))`) narrowing the window to "files created within ~1-2 seconds
|
||||
of the exec start". The `-1` shift against `-newermt`'s strict-
|
||||
greater semantics prevents same-epoch miss; the race window is
|
||||
one second wider as a result, still negligible compared to a
|
||||
real codex exec duration.
|
||||
the pre-exec timestamp `CODEX_SESSIONS_BEFORE` (computed by the
|
||||
lead in-reasoning as "current Unix timestamp minus 1" and
|
||||
substituted as a literal integer — no Bash call), narrowing the
|
||||
window to "files created within ~1-2 seconds of the exec start".
|
||||
The `-1` shift against `-newermt`'s strict-greater semantics
|
||||
prevents same-epoch miss; the race window is one second wider as
|
||||
a result, still negligible compared to a real codex exec
|
||||
duration.
|
||||
- Session-id capture happens only after review-file sanity passes
|
||||
AND only when verdict is `REVISE` (Step 4 check order in
|
||||
`SKILL.md`). This avoids aborting a valid round-1 APPROVED over
|
||||
@@ -1204,7 +1206,7 @@ and `-printf`, both GNU extensions. On macOS (BSD `find`) the commands
|
||||
do not accept these flags. The skill does not detect the platform and
|
||||
does not translate commands automatically.
|
||||
|
||||
Mitigation today: `SKILL.md` Step 4 check 3 includes a one-paragraph
|
||||
Mitigation today: `SKILL.md` Step 4 check 4 includes a one-paragraph
|
||||
platform note that states the *goal* of the command ("list rollout
|
||||
files modified since `CODEX_SESSIONS_BEFORE`, pick newest, extract
|
||||
UUID from filename") and invites the operator or the lead to substitute
|
||||
|
||||
Reference in New Issue
Block a user