138 lines
6.9 KiB
Markdown
138 lines
6.9 KiB
Markdown
# Cross-Code Review Report — Job 785aa8a3
|
|
|
|
- **Job ID**: 785aa8a3
|
|
- **Target**: `fix(lib): restore env_flags/final_cmd initialization before kind-detection block` (uncommitted working-tree change on `.agents/skills/lib.sh`, on top of commit `b0c2c08`)
|
|
- **Reviewer**: cline
|
|
- **Output Report Path**: `.mam/jobs/785aa8a3/cline-reports/report-final.md`
|
|
|
|
---
|
|
|
|
## 1. Delta Description
|
|
|
|
This changeset is the corrective fix for the critical regression identified in job 7479a065 (review of commit `b0c2c08`). The prior commit accidentally **replaced** the `env_flags`/`final_cmd` initialization block with the `kind`-detection block, orphaning `$parsed` and leaving `$env_flags`/`$final_cmd` permanently empty.
|
|
|
|
The fix is a **7-line uncommitted working-tree addition** to `.agents/skills/lib.sh` (6 init lines + 1 blank separator), restoring the deleted block in its correct position — *before* the `kind` detection, *after* the `parsed` assignment:
|
|
|
|
```bash
|
|
env_flags=""
|
|
final_cmd="$run_cmd"
|
|
if [ -n "$parsed" ]; then
|
|
env_flags=$(echo "$parsed" | head -n 1 | tr '\t' ' ')
|
|
final_cmd=$(echo "$parsed" | tail -n +2)
|
|
fi
|
|
```
|
|
|
|
The shim (`.mam/shim/herdr`, untracked by git) contains the identical restoration at lines 141-146.
|
|
|
|
**Diff (git diff .agents/skills/lib.sh):**
|
|
```
|
|
@@ -257,6 +257,13 @@ for tok in tokens:
|
|
print('\t'.join(env_flags) + '\n' + ' '.join(binary_tokens))
|
|
" "$run_cmd" 2>/dev/null || echo "")
|
|
|
|
+ env_flags=""
|
|
+ final_cmd="$run_cmd"
|
|
+ if [ -n "$parsed" ]; then
|
|
+ env_flags=$(echo "$parsed" | head -n 1 | tr '\t' ' ')
|
|
+ final_cmd=$(echo "$parsed" | tail -n +2)
|
|
+ fi
|
|
+
|
|
kind="cline"
|
|
if echo "$name" | grep -qi "agy"; then
|
|
kind="agy"
|
|
```
|
|
|
|
The diff also includes the bookkeeping addition of the prior review report file (`.agents/reports/.../report-83181aad.md`) — not a code change.
|
|
|
|
---
|
|
|
|
## 2. Lint
|
|
|
|
| Check | Result |
|
|
|-------|--------|
|
|
| `bash -n .agents/skills/lib.sh` | ✅ PASS |
|
|
| Inline-Python (strip block, lib.sh:283-294) `compile()` | ✅ PASS |
|
|
| `set -u` / `set -o nounset` enabled | ❌ NO (pre-existing; not introduced by this change) |
|
|
| shellcheck | Covered by CI (`deploy/gitea-ci.yml:31`); not installed locally |
|
|
|
|
---
|
|
|
|
## 3. Behavior (동작성)
|
|
|
|
### 3.1 Initialization restored correctly
|
|
|
|
The restored block (lib.sh:260-265) is the exact 6-line sequence deleted by `b0c2c08`, confirmed identical to the parent commit `20e2e9b` via `git show 20e2e9b:.agents/skills/lib.sh`. Placement is correct:
|
|
|
|
| Position | Before this fix (b0c2c08) | After this fix |
|
|
|----------|--------------------------|----------------|
|
|
| `parsed` (lib.sh:246-258) | computed, never consumed (orphaned) | ✅ consumed at lib.sh:263-264 |
|
|
| `env_flags` (lib.sh:260,263) | never set → empty in eval | ✅ set from `parsed` line 1 |
|
|
| `final_cmd` (lib.sh:261,264) | never set → empty in strip & eval | ✅ set from `run_cmd` / `parsed` line 2+ |
|
|
| `kind` detection (lib.sh:267-280) | `$final_cmd` empty → grep no-ops | ✅ `$final_cmd` populated → grep works |
|
|
| strip block (lib.sh:283-294) | operates on empty → empty output | ✅ operates on real binary+args → strips correctly |
|
|
| dual-syntax fallback (lib.sh:324-328) | `-- ` (no command) | ✅ `-- $final_cmd` with real command |
|
|
|
|
### 3.2 End-to-end simulation (executed)
|
|
|
|
Input: `run_cmd = 'FOO=bar cline --flag value'`
|
|
|
|
```
|
|
parsed = '--env FOO=bar\ncline --flag value'
|
|
env_flags = '--env FOO=bar' ← restored: parsed line 1
|
|
final_cmd (before strip) = 'cline --flag value' ← restored: parsed line 2+
|
|
final_cmd (after strip) = '--flag value' ← strip removes leading 'cline'
|
|
agent start: _real_herdr agent start NAME --kind cline --workspace WS --cwd WS $split_flag --env FOO=bar -- --flag value
|
|
```
|
|
|
|
All three features from `b0c2c08` now function correctly:
|
|
1. **Env-var forwarding**: `--env FOO=bar` is passed to `herdr agent start`. ✅
|
|
2. **Kind detection**: `$final_cmd` = `'cline --flag value'` → `grep -qi "cline"` matches → `kind="cline"` (or from `$name`). ✅
|
|
3. **Strip**: leading `cline` token removed → `--flag value` passed after `--`. ✅
|
|
4. **Dual-syntax fallback**: first try uses `--kind cline -- --flag value`; fallback uses `-- cline --flag value`. ✅
|
|
|
|
### 3.3 Shim consistency
|
|
|
|
`.mam/shim/herdr` (untracked) contains the identical restoration and all downstream blocks:
|
|
|
|
| Block | lib.sh lines | shim lines | Match |
|
|
|-------|-------------|------------|-------|
|
|
| init (restored) | 260-265 | 141-146 | ✅ IDENTICAL (diff) |
|
|
| kind detection | 267-280 | 148-161 | ✅ IDENTICAL (diff) |
|
|
| strip block | 282-294 | 163-175 | ✅ IDENTICAL (diff) |
|
|
| agent-start (dual syntax) | 324-328 | 205-209 | ✅ IDENTICAL (diff) |
|
|
|
|
Full `diff` of lib.sh:260-294 vs shim:141-175 = IDENTICAL.
|
|
|
|
---
|
|
|
|
## 4. Loss / Hygiene (유실)
|
|
|
|
| Item | Status |
|
|
|------|--------|
|
|
| `$parsed` — now consumed at lib.sh:263-264 (no longer orphaned) | ✅ Fixed |
|
|
| `$env_flags` — now set from `parsed` line 1 | ✅ Fixed |
|
|
| `$final_cmd` — now initialized from `run_cmd`/`parsed` before strip & eval | ✅ Fixed |
|
|
| `$final_cmd`-based kind detection (lib.sh:274-279) — now operates on non-empty `final_cmd` | ✅ Fixed |
|
|
| Change is purely additive (7 lines added, 0 removed) — no collateral deletion | ✅ Surgical |
|
|
| Shim (untracked) updated identically | ✅ Consistent |
|
|
| Working tree: only `lib.sh` modified + untracked report file | ✅ No stray artifacts |
|
|
| New tests for new-session flow | ❌ None (pre-existing limitation — requires real `herdr` binary; unchanged by this fix) |
|
|
|
|
---
|
|
|
|
## 5. Test Results
|
|
|
|
| Suite | Result | Time |
|
|
|-------|--------|------|
|
|
| `test_workspace_scope.py` + `test_tier1_unit.py -k 'resolve_herdr_session or derive_session_name'` | **5 passed** | 0.65s |
|
|
| `test_orc_onboard.py` (cross-regression) | **40 passed** | 6.35s |
|
|
|
|
No cross-regression. The `new-session` flow (env_flags/final_cmd/strip/dual-syntax) remains untested (requires a real `herdr` binary — pre-existing limitation, unchanged by this fix). The regression it fixes was likewise silent; the simulation in §3.2 provides the behavioral verification that tests cannot.
|
|
|
|
---
|
|
|
|
## 6. Verdict
|
|
|
|
The fix is a surgical, purely additive restoration of the 6-line `env_flags`/`final_cmd` initialization block that was accidentally deleted by commit `b0c2c08`. The lines are restored in the correct position (after `parsed`, before `kind` detection), are byte-identical to the parent commit `20e2e9b`, and resolve all four consequences of the prior regression: `$parsed` is consumed, `$env_flags` is populated, `$final_cmd` is initialized before the strip block, and the `$final_cmd`-based kind-detection branches are live. The end-to-end simulation confirms the full agent-start command is now correct with env-var forwarding and binary stripping both functioning. The shim (untracked) is updated identically across all four blocks. `bash -n` passes, the strip-block Python compiles, and 45 tests pass with no cross-regression. No design-level rework is needed.
|
|
|
|
[VERDICT: PASS] |