lekss361/gendesign
1
0
Fork 0
Code Issues 57 Pull requests Projects Releases Packages Wiki Activity Actions

feat(claude): env-vars refactor + stale-claim cron (Phase 2 final) #610

Merged
lekss361 merged 2 commits from feat/claude-envvars-and-stale-cron into main 2026-05-27 21:40:29 +00:00
Conversation 3 Commits 2 Files changed 9 +398 -70
lekss361 commented 2026-05-27 21:25:12 +00:00
Owner
Copy link

Summary

Финализация Phase 2 — закрывает #11 (env vars refactor) + #12 (stale-claim cron) одним PR.

#11: env vars вместо файлов

Slash-commands и _autonomous_pickup.md теперь читают persistent User-scope env vars напрямую ([System.Environment]::GetEnvironmentVariable("FORGEJO_TOKEN_BACKEND", "User")) — никаких Get-Content ~/.claude/secrets/*.token.

Setup: scripts/setup-bot-env.ps1 (user заменяет placeholders из vault, запускает один раз). После этого env vars в registry, доступны во всех новых shell.

#12: stale-claim cron

  • scripts/cleanup-stale-claims.sh — bash + curl + Python для JSON
  • .forgejo/workflows/stale-claims.yml — cron */30 * * * *

Освобождает issues застрявшие в status/wip >4h — clear assignees, +status/ready, post explanation comment.

Что нужно ПОСЛЕ merge

  1. Завести secret FORGEJO_BOT_QA_TOKEN в Forgejo Actions UI (git.gendsgn.ru/lekss361/gendesign settings → actions → secrets) — bot-qa PAT
  2. Опционально: верифицировать manual run workflow через UI workflow_dispatch

Test plan

  • Phase 2 finalized: claude window → /work-as-backend → pre-flight использует env vars
  • setup-bot-env.ps1 — placeholders detected при пустых tokens
  • Stale cron — manual trigger в Forgejo UI, верифицировать что не падает на empty result
  • Test stale flow: создать test issue со status/wip, поставить updated >4h назад, prove cron освободил

Refs

  • PR #608 (Phase 1 — agent stubs)
  • PR #609 (Phase 2 prep — slash-commands + labels + pre-flight)
  • vault: runbooks/multi_agent_autonomous_workflow.md
  • vault: meta/00_credentials.md (bot PATs)
## Summary Финализация Phase 2 — закрывает #11 (env vars refactor) + #12 (stale-claim cron) одним PR. ### #11: env vars вместо файлов Slash-commands и `_autonomous_pickup.md` теперь читают persistent User-scope env vars напрямую (`[System.Environment]::GetEnvironmentVariable("FORGEJO_TOKEN_BACKEND", "User")`) — никаких `Get-Content ~/.claude/secrets/*.token`. Setup: `scripts/setup-bot-env.ps1` (user заменяет placeholders из vault, запускает один раз). После этого env vars в registry, доступны во всех новых shell. ### #12: stale-claim cron - `scripts/cleanup-stale-claims.sh` — bash + curl + Python для JSON - `.forgejo/workflows/stale-claims.yml` — cron `*/30 * * * *` Освобождает issues застрявшие в `status/wip` >4h — clear assignees, +status/ready, post explanation comment. ### Что нужно ПОСЛЕ merge 1. Завести secret `FORGEJO_BOT_QA_TOKEN` в Forgejo Actions UI (git.gendsgn.ru/lekss361/gendesign settings → actions → secrets) — bot-qa PAT 2. Опционально: верифицировать manual run workflow через UI `workflow_dispatch` ## Test plan - [ ] Phase 2 finalized: `claude` window → `/work-as-backend` → pre-flight использует env vars - [ ] `setup-bot-env.ps1` — placeholders detected при пустых tokens - [ ] Stale cron — manual trigger в Forgejo UI, верифицировать что не падает на empty result - [ ] Test stale flow: создать test issue со status/wip, поставить updated >4h назад, prove cron освободил ## Refs - PR #608 (Phase 1 — agent stubs) - PR #609 (Phase 2 prep — slash-commands + labels + pre-flight) - vault: `runbooks/multi_agent_autonomous_workflow.md` - vault: `meta/00_credentials.md` (bot PATs)
lekss361 added 1 commit 2026-05-27 21:25:12 +00:00 
#11: slash-commands и _autonomous_pickup.md теперь читают persistent User-scope
env vars напрямую (FORGEJO_TOKEN_<ROLE>) вместо file-based из ~/.claude/secrets/.

Один раз: scripts/setup-bot-env.ps1 (запускается user'ом локально, кладёт
токены из vault в Windows User registry). После этого окна grab credentials
через [System.Environment]::GetEnvironmentVariable() — никаких файлов.

Все 5 work-as-*.md обновлены (analyst/backend/frontend/reviewer/qa).

#12: stale-claim cleanup — cron каждые 30 минут.

- scripts/cleanup-stale-claims.sh — bash, parses Forgejo API + Python для JSON
- .forgejo/workflows/stale-claims.yml — cron schedule `*/30 * * * *`

Освобождает issues застрявшие в status/wip >4h:
  - assignees=[]
  - +status/ready -status/wip
  - comment "stale claim released, worker likely crashed"

Использует secret FORGEJO_BOT_QA_TOKEN (минимум прав = write:issue).

Setup: secret нужно завести в Forgejo Actions UI после merge —
git.gendsgn.ru/lekss361/gendesign/-/settings/actions/secrets.

Refs: PRs #608, #609. Closes TODO с PR #608 (stale-claim cron).
lekss361 commented 2026-05-27 21:30:01 +00:00
Author
Owner
Copy link

Deep Code Review — verdict

Summary

  • Status: 🔴 BLOCK (changes requested)
  • Files: 9 (P0: 1 cron script, 1 setup script, 1 workflow · P1: 6 .claude/* docs)
  • Lines: +366 / -98 · PR: #610
  • 🚨 Self-extending — модифицирует .claude/agents/** (1 file) + .claude/commands/** (5 files). Human merge required — reviewer bot НЕ мержит per merge policy override.

🔴 Critical (BLOCK)

1. scripts/cleanup-stale-claims.sh:355 — CUTOFF never exported → Python reads "0" → cron never releases anything

NOW=$(now_utc_epoch)
CUTOFF=$((NOW - STALE_SECS))
# ... 
echo "$RESPONSE" | python3 <<PYEOF
cutoff = int(os.environ.get("CUTOFF", "0"))   # ← gets "0", not the real cutoff

CUTOFF is set as a shell var, не exported. Python heredoc child process не наследует. os.environ.get("CUTOFF", "0") всегда вернёт "0", поэтому updated_epoch >= cutoff всегда true → каждый issue считается fresh → нулевое действие.

Fix:

export CUTOFF=$((NOW - STALE_SECS))
export STALE_HOURS

Ilk: FORGEJO_URL/FORGEJO_REPO/FORGEJO_TOKEN приходят из workflow env: (уже exported в job env) — эти OK. Но shell-level CUTOFF нужен explicit export.

2. scripts/cleanup-stale-claims.sh:383 — datetime.UTC требует Python 3.11+

int(datetime.datetime.now(datetime.UTC).timestamp())

ubuntu-latest сейчас 22.04 LTS с системным python3 = 3.10. datetime.UTC появился в 3.11. На runner упадёт с AttributeError. Используется в двух местах (line 345 в now_utc_epoch() shell helper, line 383 в Python heredoc).

Fix: datetime.timezone.utc (works in 3.8+).

3. scripts/cleanup-stale-claims.sh:408-419 — label race: ready added before wip removed

Порядок:

  1. POST /labels add status/ready
  2. DELETE /labels/status%2Fwip

Если step 2 fails, issue имеет оба labels (status/ready + status/wip). Это нарушает status invariant — workers, проверяющие "is ready and not wip", получат inconsistent state.

Fix: invert order. Сначала remove wip, потом add ready. Если ready add fails — issue в neutral state без status, что менее опасно.

🟠 High

4. cleanup-stale-claims.sh:358 — pagination отсутствует

limit=50&page=1 — обрабатывается только первые 50 wip issues. Сейчас issue queue маленький, но cron */30 * * * * × 50 ceiling = at most 100/h освобождений. Если когда-нибудь будет burst (CI failure → multiple stuck) — silently truncated.

Fix: loop по page пока ответ не пустой.

5. setup-bot-env.ps1:496 — token остаётся в $tokens hashtable в memory + script остаётся on-disk

Комментарий говорит "можешь удалить локальную копию после запуска", но это manual step. Лучше:

  • Подсказать user'у запустить через Invoke-Expression после редактирования, или
  • В конце script: Remove-Variable tokens; [GC]::Collect() (минор — но соответствует security best-practice)
  • Ещё лучше: read tokens из vault file path через Read-Host -AsSecureString или из gpg --decrypt, без edit-in-place

Не блокер для merge, но worth для следующей итерации.

🟡 Medium

6. cleanup-stale-claims.sh:371 — updated_at ≠ heartbeat

Issue updated_at обновляется при любом edit (label, comment, assignee). Когда worker claim'ит issue (добавляет status/wip + assignee), updated_at = claim time. Если worker делает task >4h без касаний issue — будет released пока ещё жив. Для текущих small tasks 4h enough, но стоит:

  • задокументировать предположение в скрипте
  • или периодически постить heartbeat comment из worker'a (Phase 4)

7. cleanup-stale-claims.sh:348-350 — dead code: iso_to_epoch() shell helper определён, не используется

Вся iso→epoch конверсия делается inline в Python.

8. .forgejo/workflows/stale-claims.yml:23 — нет concurrency: group

Cron каждые 30 min на shared runner. Если предыдущий run ещё running (5min timeout — unlikely, но возможно при API latency), второй stomp'нет. Добавь:

concurrency:
  group: stale-claims
  cancel-in-progress: false

🟢 Low

9. cleanup-stale-claims.sh:439 — print(f"") → print() (cosmetic).

10. setup-bot-env.ps1:499 — последние 4 char токена в console output. Минорный info leak в shell history; приемлемо для UX verification но стоит upgrade на **** без suffix или показать только при flag -Verbose.

11. _autonomous_pickup.md:47 — Out-Null | if (-not $?) pattern

[System.Environment]::GetEnvironmentVariable("FORGEJO_TOKEN_BACKEND", "User") | Out-Null
if (-not $?) { Write-Error "❌ Запусти setup-bot-env.ps1 сначала" }

GetEnvironmentVariable НЕ throws при missing var — возвращает $null. $? checks last command success, и pipe в Out-Null всегда $true. Эта проверка молча проходит даже без env var. Используй: if (-not [System.Environment]::GetEnvironmentVariable(...)) { Write-Error ... }.

Cross-file impact

  • Smoke-test до merge: запустить cron вручную через workflow_dispatch в Forgejo UI после фиксов 1+2, проверить:
    • exit code 0 при 0 stale
    • exit code 0 при N stale (создать test issue с label status/wip + backdated update — нужен admin patch updated_at, или подождать 4h)
  • meta/00_credentials.md в vault — должен документировать FORGEJO_TOKEN_<ROLE> env var naming (sync с этим PR).
  • FORGEJO_BOT_QA_TOKEN secret в Forgejo Actions UI — указано в test plan как post-merge step. ОК.

Positive

  • Чёткий split: refactor (.md docs) + new feature (cron) + setup helper. Easy to revert.
  • setup-bot-env.ps1 validation REPLACE_ME_FROM_VAULT → fail-fast.
  • _autonomous_pickup.md теперь self-documenting с inline check.
  • Workflow timeout-minutes: 5 — sensible bound.
  • Сохранён старый pattern для bot-remote (нужен для git push под bot identity).

Recommended next steps

  1. MUST FIX before merge: export CUTOFF, fix datetime.UTC → datetime.timezone.utc, invert label order (remove wip → add ready).
  2. Smoke run через workflow_dispatch после fix — verify no traceback на 0 stale, no traceback на ≥1 stale.
  3. Добавь concurrency: group в workflow.
  4. Pagination loop в shell (опционально, низкий risk сейчас).
  5. Human merge. Self-extending → bot НЕ мержит.

Complexity / blast radius

  • Risk: medium. Cron на UTC runner, doc files affect только future autonomous workflows. Если cron упадёт — silently log, никаких prod-side effects (только stale queue растёт).
  • Reversibility: high. Revert PR + remove env vars [Environment]::SetEnvironmentVariable($k, $null, "User").
  • Merge window: anytime after fixes; не зависит от deploy cycle.

Bot status: changes requested, no merge (self-extending — human approval required + 3 critical bugs).

<!-- gendesign-review-bot: sha=297eb29 verdict=changes --> ## Deep Code Review — verdict ### Summary - Status: 🔴 BLOCK (changes requested) - Files: 9 (P0: 1 cron script, 1 setup script, 1 workflow · P1: 6 .claude/* docs) - Lines: +366 / -98 · PR: #610 - 🚨 **Self-extending** — модифицирует `.claude/agents/**` (1 file) + `.claude/commands/**` (5 files). **Human merge required** — reviewer bot НЕ мержит per merge policy override. ### 🔴 Critical (BLOCK) **1. `scripts/cleanup-stale-claims.sh:355` — `CUTOFF` never exported → Python reads `"0"` → cron never releases anything** ```bash NOW=$(now_utc_epoch) CUTOFF=$((NOW - STALE_SECS)) # ... echo "$RESPONSE" | python3 <<PYEOF cutoff = int(os.environ.get("CUTOFF", "0")) # ← gets "0", not the real cutoff ``` `CUTOFF` is set as a **shell var**, не exported. Python heredoc child process не наследует. `os.environ.get("CUTOFF", "0")` всегда вернёт `"0"`, поэтому `updated_epoch >= cutoff` всегда true → каждый issue считается fresh → нулевое действие. Fix: ```bash export CUTOFF=$((NOW - STALE_SECS)) export STALE_HOURS ``` Ilk: `FORGEJO_URL`/`FORGEJO_REPO`/`FORGEJO_TOKEN` приходят из workflow `env:` (уже exported в job env) — эти OK. Но shell-level `CUTOFF` нужен explicit `export`. **2. `scripts/cleanup-stale-claims.sh:383` — `datetime.UTC` требует Python 3.11+** ```python int(datetime.datetime.now(datetime.UTC).timestamp()) ``` `ubuntu-latest` сейчас 22.04 LTS с системным `python3` = 3.10. `datetime.UTC` появился в **3.11**. На runner упадёт с `AttributeError`. Используется в двух местах (line 345 в `now_utc_epoch()` shell helper, line 383 в Python heredoc). Fix: `datetime.timezone.utc` (works in 3.8+). **3. `scripts/cleanup-stale-claims.sh:408-419` — label race: ready added before wip removed** Порядок: 1. POST `/labels` add `status/ready` 2. DELETE `/labels/status%2Fwip` Если step 2 fails, issue имеет **оба** labels (`status/ready` + `status/wip`). Это нарушает status invariant — workers, проверяющие "is ready and not wip", получат inconsistent state. Fix: invert order. Сначала remove wip, потом add ready. Если ready add fails — issue в neutral state без status, что менее опасно. ### 🟠 High **4. `cleanup-stale-claims.sh:358` — pagination отсутствует** `limit=50&page=1` — обрабатывается только первые 50 wip issues. Сейчас issue queue маленький, но cron `*/30 * * * *` × 50 ceiling = at most 100/h освобождений. Если когда-нибудь будет burst (CI failure → multiple stuck) — silently truncated. Fix: loop по `page` пока ответ не пустой. **5. `setup-bot-env.ps1:496` — token остаётся в `$tokens` hashtable в memory + script остаётся on-disk** Комментарий говорит "можешь удалить локальную копию после запуска", но это manual step. Лучше: - Подсказать user'у запустить через `Invoke-Expression` после редактирования, или - В конце script: `Remove-Variable tokens; [GC]::Collect()` (минор — но соответствует security best-practice) - Ещё лучше: read tokens из vault file path через `Read-Host -AsSecureString` или из `gpg --decrypt`, без edit-in-place Не блокер для merge, но worth для следующей итерации. ### 🟡 Medium **6. `cleanup-stale-claims.sh:371` — `updated_at` ≠ heartbeat** Issue `updated_at` обновляется при любом edit (label, comment, assignee). Когда worker claim'ит issue (добавляет `status/wip` + assignee), `updated_at` = claim time. Если worker делает task >4h без касаний issue — будет released пока ещё жив. Для текущих small tasks 4h enough, но стоит: - задокументировать предположение в скрипте - или периодически постить heartbeat comment из worker'a (Phase 4) **7. `cleanup-stale-claims.sh:348-350` — dead code: `iso_to_epoch()` shell helper определён, не используется** Вся iso→epoch конверсия делается inline в Python. **8. `.forgejo/workflows/stale-claims.yml:23` — нет `concurrency:` group** Cron каждые 30 min на shared runner. Если предыдущий run ещё running (5min timeout — unlikely, но возможно при API latency), второй stomp'нет. Добавь: ```yaml concurrency: group: stale-claims cancel-in-progress: false ``` ### 🟢 Low **9. `cleanup-stale-claims.sh:439` — `print(f"")` → `print()`** (cosmetic). **10. `setup-bot-env.ps1:499` — последние 4 char токена в console output**. Минорный info leak в shell history; приемлемо для UX verification но стоит upgrade на `****` без suffix или показать только при flag `-Verbose`. **11. `_autonomous_pickup.md:47` — `Out-Null | if (-not $?)` pattern** ```powershell [System.Environment]::GetEnvironmentVariable("FORGEJO_TOKEN_BACKEND", "User") | Out-Null if (-not $?) { Write-Error "❌ Запусти setup-bot-env.ps1 сначала" } ``` `GetEnvironmentVariable` НЕ throws при missing var — возвращает `$null`. `$?` checks last command success, и pipe в `Out-Null` всегда $true. Эта проверка молча проходит даже без env var. Используй: `if (-not [System.Environment]::GetEnvironmentVariable(...)) { Write-Error ... }`. ### Cross-file impact - Smoke-test до merge: запустить cron вручную через `workflow_dispatch` в Forgejo UI **после** фиксов 1+2, проверить: - exit code 0 при 0 stale - exit code 0 при N stale (создать test issue с label status/wip + backdated update — нужен admin patch updated_at, или подождать 4h) - `meta/00_credentials.md` в vault — должен документировать `FORGEJO_TOKEN_<ROLE>` env var naming (sync с этим PR). - `FORGEJO_BOT_QA_TOKEN` secret в Forgejo Actions UI — указано в test plan как post-merge step. ОК. ### Positive - Чёткий split: refactor (.md docs) + new feature (cron) + setup helper. Easy to revert. - `setup-bot-env.ps1` validation `REPLACE_ME_FROM_VAULT` → fail-fast. - `_autonomous_pickup.md` теперь self-documenting с inline check. - Workflow `timeout-minutes: 5` — sensible bound. - Сохранён старый pattern для bot-remote (нужен для git push под bot identity). ### Recommended next steps 1. **MUST FIX before merge:** export CUTOFF, fix `datetime.UTC` → `datetime.timezone.utc`, invert label order (remove wip → add ready). 2. Smoke run через `workflow_dispatch` после fix — verify no traceback на 0 stale, no traceback на ≥1 stale. 3. Добавь `concurrency:` group в workflow. 4. Pagination loop в shell (опционально, низкий risk сейчас). 5. **Human merge.** Self-extending → bot НЕ мержит. ### Complexity / blast radius - **Risk:** medium. Cron на UTC runner, doc files affect только future autonomous workflows. Если cron упадёт — silently log, никаких prod-side effects (только stale queue растёт). - **Reversibility:** high. Revert PR + remove env vars `[Environment]::SetEnvironmentVariable($k, $null, "User")`. - **Merge window:** anytime after fixes; не зависит от deploy cycle. --- **Bot status:** changes requested, no merge (self-extending — human approval required + 3 critical bugs).
lekss361 commented 2026-05-27 21:33:52 +00:00
Author
Owner
Copy link

Fixup pushed: e4dc866

Все 🔴 BLOCK критичные закрыты + большая часть 🟠/🟡/🟢.

🔴 Critical (3/3 ✅)

# Fix
1 export CUTOFF + export STALE_HOURS — Python heredoc child process теперь получает реальные values. Plus added abort при CUTOFF=0 для defense-in-depth
2 datetime.UTC → datetime.timezone.utc (Python 3.8+ compat) — обе occurrences
3 Inverted label transition: DELETE wip → POST ready. При partial failure issue в neutral state, не оба status одновременно

🟠 + 🟡 + 🟢 (5/6 ✅, 1 skipped)

# Fix
4 Pagination loop через temp file + Python parsing (defensive cap 10×50=500)
5 Removed dead iso_to_epoch() shell helper
7 concurrency: group: stale-claims, cancel-in-progress: false в workflow
8 Documented updated_at != heartbeat assumption в script header
11 _autonomous_pickup.md: fixed broken Out-Null | if (-not $?) pattern → direct if (-not GetEnvironmentVariable(...)) check

Не сделано (skipped, low risk)

  • #5 setup-bot-env.ps1 tokens в memory — UX trade-off (manual edit-in-place easier than gpg-decrypt). Можно upgrade в Phase 4.
  • #10 token suffix в console — minor info leak в shell history (4 chars), не critical.
  • #9 print(f"") → print() — cosmetic, изменено в fixup автоматически.

Self-extending nota bene

Этот PR модифицирует .claude/agents/** + .claude/commands/** — попадает под self-extending guard. Bot НЕ мержит (per merge policy). Human merge required.

Готов к re-review.

## Fixup pushed: e4dc866 Все 🔴 BLOCK критичные закрыты + большая часть 🟠/🟡/🟢. ### 🔴 Critical (3/3 ✅) | # | Fix | |---|---| | 1 | `export CUTOFF` + `export STALE_HOURS` — Python heredoc child process теперь получает реальные values. Plus added abort при CUTOFF=0 для defense-in-depth | | 2 | `datetime.UTC` → `datetime.timezone.utc` (Python 3.8+ compat) — обе occurrences | | 3 | Inverted label transition: DELETE wip → POST ready. При partial failure issue в neutral state, не оба status одновременно | ### 🟠 + 🟡 + 🟢 (5/6 ✅, 1 skipped) | # | Fix | |---|---| | 4 | Pagination loop через temp file + Python parsing (defensive cap 10×50=500) | | 5 | Removed dead `iso_to_epoch()` shell helper | | 7 | `concurrency: group: stale-claims, cancel-in-progress: false` в workflow | | 8 | Documented `updated_at != heartbeat` assumption в script header | | 11 | `_autonomous_pickup.md`: fixed broken `Out-Null \| if (-not $?)` pattern → direct `if (-not GetEnvironmentVariable(...))` check | ### Не сделано (skipped, low risk) - **#5 setup-bot-env.ps1 tokens в memory** — UX trade-off (manual edit-in-place easier than gpg-decrypt). Можно upgrade в Phase 4. - **#10 token suffix в console** — minor info leak в shell history (4 chars), не critical. - **#9 `print(f"")` → `print()`** — cosmetic, изменено в fixup автоматически. ### Self-extending nota bene Этот PR модифицирует `.claude/agents/**` + `.claude/commands/**` — попадает под self-extending guard. Bot НЕ мержит (per merge policy). **Human merge required.** Готов к re-review.
lekss361 added 1 commit 2026-05-27 21:33:54 +00:00 
🔴 Critical (3):

1. cleanup-stale-claims.sh: export CUTOFF + STALE_HOURS
   Без `export` Python heredoc child process получал "0" вместо реального
   cutoff → cron всегда видел все issues как fresh → ничего не освобождал.
   Plus added abort при CUTOFF=0 в Python heredoc для defense-in-depth.

2. cleanup-stale-claims.sh: datetime.UTC → datetime.timezone.utc
   datetime.UTC требует Python 3.11+. Forgejo runner ubuntu-latest имеет 3.10.
   datetime.timezone.utc works в 3.8+.

3. cleanup-stale-claims.sh: invert label order — DELETE wip FIRST, потом
   POST ready. Если step 2 (add ready) fails — issue в neutral state без
   status/*, менее опасно чем оба status/ready+status/wip одновременно.

🟠 + 🟡 + 🟢 (5):

4. Added pagination loop (defensive cap 10 pages × 50 = 500 issues), но
   реализована через temp file + Python parsing (не bash JSON concat).

5. Removed dead code: iso_to_epoch() shell helper не использовался.

6. _autonomous_pickup.md: fix broken `Out-Null | if (-not $?)` pattern.
   GetEnvironmentVariable НЕ throws при missing — возвращает $null. $? у
   Get* всегда $true. Заменено на прямую проверку результата.

7. stale-claims.yml: добавлен concurrency group чтобы overlapping runs
   не stomp'ились (хотя 5-min timeout делает overlap unlikely, defensive).

8. Documented updated_at != heartbeat assumption в header script'а.

Не сделано (low/optional):

- Setup script tokens в memory (UX trade-off — не critical)
- Token suffix в console output (минор info leak в shell history)

Refs: PR #610 review by review-bot (sha=297eb29, verdict=changes)
lekss361 commented 2026-05-27 21:39:11 +00:00
Author
Owner
Copy link

Re-Review — verdict (delta 297eb29 → e4dc866)

Summary

  • Status: ✅ APPROVE (все 3 prior 🔴 BLOCK закрыты)
  • Files in fixup: 3 (scripts/cleanup-stale-claims.sh, .forgejo/workflows/stale-claims.yml, .claude/agents/_autonomous_pickup.md)
  • Fixup commit: e4dc866
  • 🚨 Self-extending — human merge required. Bot НЕ мержит per merge policy override (PR трогает .claude/agents/_autonomous_pickup.md + 5x .claude/commands/work-as-*.md).

🔴 Critical fixes verified

1. CUTOFF not exported → cron no-op — ✅ FIXED

  • scripts/cleanup-stale-claims.sh:34 — export STALE_HOURS="${STALE_HOURS:-4}"
  • scripts/cleanup-stale-claims.sh:50 — export CUTOFF=$((NOW - STALE_SECS))
  • Defense-in-depth: Python heredoc abort при cutoff == 0 (lines 112-114) — даже если кто-то снова забудет export в будущем рефакторинге, цикл остановится с error вместо silent no-op.
  • Heredoc остался quoted <<'PYEOF' (line 69) — env-var path корректен (Python читает через os.environ.get, не через shell interpolation).

2. datetime.UTC → Python 3.10 ImportError — ✅ FIXED

  • Все occurrences заменены на datetime.timezone.utc:
    • shell helper now_utc_epoch() (line 41)
    • Python heredoc now_epoch (line 76)
  • iso_to_epoch() helper (другая datetime.UTC точка) удалён целиком — мёртвый код после внутри-Python переноса парсинга.
  • Final grep datetime.UTC в head_sha=e4dc866 → 0 occurrences. ubuntu-latest python3.10 safe.

3. Label race (add ready перед remove wip) — ✅ FIXED

  • Order inverted в scripts/cleanup-stale-claims.sh:126-137:
    1. DELETE status/wip FIRST (line 130)
    2. POST status/ready (line 136)
  • Inline rationale (comment lines 126-127): partial-failure outcome = neutral state без status/* < both-statuses bad state. Корректная trade-off для idempotent retry на следующем cron tick.

🟠 / 🟡 / 🟢 follow-ups verified

# Issue Status
4 Pagination (только page=1 → defensive cap 10×50=500) ✅ for-loop + mktemp + per-page len check + break (lines 55-65)
5 Dead iso_to_epoch() shell helper ✅ removed
7 Workflow concurrency (overlapping cron при slow run) ✅ concurrency: group: stale-claims, cancel-in-progress: false (workflow lines 18-20)
8 updated_at != heartbeat assumption undocumented ✅ documented в script header (lines 19-23)
11 _autonomous_pickup.md broken Out-Null | if (-not $?) pattern ✅ заменено на direct if (-not GetEnvironmentVariable(...)) check

Skipped (acknowledged trade-offs)

  • #5 setup-bot-env.ps1 plaintext tokens — author заявил UX trade-off, "можно upgrade в Phase 4". Acceptable: tokens already live в registry, attack surface не расширяется этим PR.
  • #10 token suffix в console — 4-char info leak в shell history, minor.
  • #9 print(f"") → print() — author заявил cosmetic; в delta строка 191→192 changed.

Pre-flight checks

  • head.sha = e4dc866e2044ca9520cd0134c60524bf9c178849, mergeable=true, state=open
  • Branch: feat/claude-envvars-and-stale-cron
  • No 40-char hex / token literals в additions
  • Self-extending guard intact

Complexity / blast radius

  • Risk: low — cron is best-effort cleanup, idempotent, повторный запуск через 30 минут восстанавливает inconsistency
  • Reversibility: high — workflow disable через UI кнопку, script revert через standard revert PR
  • Merge window: anytime (cron runs */30, новый код активен через ≤30 минут после merge)

Recommended next steps

  1. Human merge (per self-extending guard) — squash recommended
  2. Post-merge: завести FORGEJO_BOT_QA_TOKEN secret в Forgejo Actions UI (упомянуто в PR body)
  3. Manual workflow_dispatch запуск на пустой status/wip queue — sanity check, что log output корректный + exit 0
  4. (опционально, не блокер) Phase 4: heartbeat comments от worker'а для accurate stale detection вне updated_at proxy

Positive

  • Defense-in-depth abort при cutoff == 0 — pattern, который стоит закрепить (memory: [heredoc-shell-var-not-exported] — guard превращает silent-no-op в loud-fail)
  • Explicit inline comment объясняющий label transition order — будущий читатель не "оптимизирует" обратно
  • Pagination cap (10 pages = 500 issues) — reasonable defense без infinite loop risk

🚨 Self-extending — human merge required. Bot НЕ мержит.

<!-- gendesign-review-bot: sha=e4dc866 verdict=approve --> ## Re-Review — verdict (delta 297eb29 → e4dc866) ### Summary - Status: ✅ APPROVE (все 3 prior 🔴 BLOCK закрыты) - Files in fixup: 3 (`scripts/cleanup-stale-claims.sh`, `.forgejo/workflows/stale-claims.yml`, `.claude/agents/_autonomous_pickup.md`) - Fixup commit: e4dc866 - 🚨 **Self-extending — human merge required.** Bot НЕ мержит per merge policy override (PR трогает `.claude/agents/_autonomous_pickup.md` + 5x `.claude/commands/work-as-*.md`). ### 🔴 Critical fixes verified **1. `CUTOFF` not exported → cron no-op** — ✅ **FIXED** - `scripts/cleanup-stale-claims.sh:34` — `export STALE_HOURS="${STALE_HOURS:-4}"` - `scripts/cleanup-stale-claims.sh:50` — `export CUTOFF=$((NOW - STALE_SECS))` - Defense-in-depth: Python heredoc abort при `cutoff == 0` (lines 112-114) — даже если кто-то снова забудет `export` в будущем рефакторинге, цикл остановится с error вместо silent no-op. - Heredoc остался quoted `<<'PYEOF'` (line 69) — env-var path корректен (Python читает через `os.environ.get`, не через shell interpolation). **2. `datetime.UTC` → Python 3.10 ImportError** — ✅ **FIXED** - Все occurrences заменены на `datetime.timezone.utc`: - shell helper `now_utc_epoch()` (line 41) - Python heredoc `now_epoch` (line 76) - `iso_to_epoch()` helper (другая `datetime.UTC` точка) **удалён целиком** — мёртвый код после внутри-Python переноса парсинга. - Final grep `datetime.UTC` в head_sha=e4dc866 → 0 occurrences. ubuntu-latest python3.10 safe. **3. Label race (add ready перед remove wip)** — ✅ **FIXED** - Order inverted в `scripts/cleanup-stale-claims.sh:126-137`: 1. DELETE `status/wip` FIRST (line 130) 2. POST `status/ready` (line 136) - Inline rationale (comment lines 126-127): partial-failure outcome = neutral state без `status/*` < both-statuses bad state. Корректная trade-off для idempotent retry на следующем cron tick. ### 🟠 / 🟡 / 🟢 follow-ups verified | # | Issue | Status | |---|---|---| | 4 | Pagination (только page=1 → defensive cap 10×50=500) | ✅ for-loop + mktemp + per-page len check + break (lines 55-65) | | 5 | Dead `iso_to_epoch()` shell helper | ✅ removed | | 7 | Workflow concurrency (overlapping cron при slow run) | ✅ `concurrency: group: stale-claims, cancel-in-progress: false` (workflow lines 18-20) | | 8 | `updated_at != heartbeat` assumption undocumented | ✅ documented в script header (lines 19-23) | | 11 | `_autonomous_pickup.md` broken `Out-Null \| if (-not $?)` pattern | ✅ заменено на direct `if (-not GetEnvironmentVariable(...))` check | ### Skipped (acknowledged trade-offs) - **#5 setup-bot-env.ps1 plaintext tokens** — author заявил UX trade-off, "можно upgrade в Phase 4". Acceptable: tokens already live в registry, attack surface не расширяется этим PR. - **#10 token suffix в console** — 4-char info leak в shell history, minor. - **#9 `print(f"")` → `print()`** — author заявил cosmetic; в delta строка 191→192 changed. ### Pre-flight checks - `head.sha = e4dc866e2044ca9520cd0134c60524bf9c178849`, `mergeable=true`, `state=open` - Branch: `feat/claude-envvars-and-stale-cron` - No 40-char hex / token literals в additions - Self-extending guard intact ### Complexity / blast radius - Risk: low — cron is best-effort cleanup, idempotent, повторный запуск через 30 минут восстанавливает inconsistency - Reversibility: high — workflow disable через UI кнопку, script revert через standard revert PR - Merge window: anytime (cron runs */30, новый код активен через ≤30 минут после merge) ### Recommended next steps 1. **Human merge** (per self-extending guard) — squash recommended 2. Post-merge: завести `FORGEJO_BOT_QA_TOKEN` secret в Forgejo Actions UI (упомянуто в PR body) 3. Manual `workflow_dispatch` запуск на пустой `status/wip` queue — sanity check, что log output корректный + exit 0 4. (опционально, не блокер) Phase 4: heartbeat comments от worker'а для accurate stale detection вне `updated_at` proxy ### Positive - Defense-in-depth abort при `cutoff == 0` — pattern, который стоит закрепить (memory: `[heredoc-shell-var-not-exported]` — guard превращает silent-no-op в loud-fail) - Explicit inline comment объясняющий label transition order — будущий читатель не "оптимизирует" обратно - Pagination cap (10 pages = 500 issues) — reasonable defense без infinite loop risk 🚨 **Self-extending — human merge required.** Bot НЕ мержит.
lekss361 merged commit 26559e16e5 into main 2026-05-27 21:40:29 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
admin

Admin UI / internal tools / pipeline tracker

  
analytics

Аналитика данных / matching

  
auth

Authentication / authorization / multi-tenancy

  
automation

Автоматизация / cron / scheduler

  
bug

Something isn't working

  
business

Коммерческая воронка / пилоты / pipeline / реестр ПО

  
chore

Operational housekeeping

  
ci

Forgejo Actions / build pipeline

  
compliance

152-ФЗ / реестр ПО / legal / TOS

  
data

Data sources, parsers, ETL

  
data-moat

Уникальные данные — sales tracker, CRM aggregation

  
docs

README / runbooks / onboarding

  
duplicate

This issue or pull request already exists

  
dx

Developer experience / build speed

  
enhancement

New feature or request

  
Fable 5 ревью

Находки полного мульти-агентного ревью (Claude Fable 5, 2026-06-09)

  
feedback/max

Feedback from Max (Senior partner)

  
generative

Generative Design — concept generation engine

  
GG-форсайт

Новое ТЗ от Ген Геныча (июнь 2026): Site Finder v2 — ИИ-прогноз продуктовой стратегии девелопера. Актуальный приоритет.

  
needs-discussion

Pending team sync / decision

  
needs-human

Human review/decision required

  
observability

Monitoring / metrics / logs / Sentry / Uptime

  
pause-bots

Kill-switch — all autonomous bots pause

  
performance

Latency / throughput / DB query tuning

  
priority/p0

Critical / production-blocker

  
priority/p1

High priority

  
priority/p2

Normal priority

  
priority/p3

Низкий приоритет (follow-up, tech-debt)

  
scope/backend

Backend Python — FastAPI, Celery, scrapers

  
scope/db

DB migrations, indexes, SQL

  
scope/devops

docker-compose, Caddy, workflows

  
scope/frontend

Frontend — Next.js, React, TypeScript

  
scope/qa

Tests, smoke, Playwright

  
scrapers

Скрапперы (Avito/Cian/Yandex/Rosreestr)

  
security

Security audit / secrets / vulnerabilities

  
site-finder

Site Finder scoring quality & data layer

  
stage/1

Stage 1: Generative MVP (нед 2-7)

  
stage/2

Stage 2: Site Finder MVP (нед 8-12)

  
status/blocked

Blocked — needs human intervention

  
status/done

Closed, completed

  
status/needs-analysis

Raw/fuzzy task filed by human — auto-analyst подхватывает: code-archeology + декомпозиция в actionable scope/* + status/ready (или split на под-issues)

  
status/needs-fix

Reviewer/QA вернул worker на доработку — worker сам чинит свой PR (fixup), НЕ требует human (в отличие от status/blocked)

  
status/qa

PR merged, awaiting smoke test

  
status/ready

Ready for pickup by worker bot

  
status/review

PR open, awaiting code review

  
status/wip

Work in progress (worker claimed)

  
tech-debt

Технический долг

  
tradein

Trade-In MVP проект

  
ux

User experience / responsive / conversion

  
week ревью 1

Week review 1 — находки multi-agent аудита проекта (2026-06-14)

  
wontfix

This will not be worked on

  
вторичка

Trade-In Estimator — проект оценки вторичного жилья

  
ИРД

Исходно-разрешительная документация — слой ограничений участка для analyze-таблицы

No labels
admin
analytics
auth
automation
bug
business
chore
ci
compliance
data
data-moat
docs
duplicate
dx
enhancement
Fable 5 ревью
feedback/max
generative
GG-форсайт
needs-discussion
needs-human
observability
pause-bots
performance
priority/p0
priority/p1
priority/p2
priority/p3
scope/backend
scope/db
scope/devops
scope/frontend
scope/qa
scrapers
security
site-finder
stage/1
stage/2
status/blocked
status/done
status/needs-analysis
status/needs-fix
status/qa
status/ready
status/review
status/wip
tech-debt
tradein
ux
week ревью 1
wontfix
вторичка
ИРД
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lekss361/gendesign#610
No description provided.
Delete branch "feat/claude-envvars-and-stale-cron"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?