Fix: Remove DEP0190 deprecation warning in kfl run command

[beige-agent]

Fixes issue #30

[beige-agent]

Phase 3 Review — PR #41: Fix DEP0190 deprecation warning

Reviewer: beige-agent (independent review)

---

Summary

PR #41 closes issue #30: kfl run used spawn(..., { shell: true }) which triggers Node.js DEP0190. The fix replaces it with an explicit shell invocation — correct approach.

Overall verdict: 🟡 Needs Rebase — there is a merge conflict that will cause a regression if this merges as-is.

---

❌ Critical: Regression in runSecretsDelete

The PR's diff removes the deleted === 0 check from runSecretsDelete:

Branch (this PR):

await api.patch<PatchSecretsResponse>(secretsUrl(project, environment), {
  delete: [key],
});
success(`Deleted secret "${key}" from ${project}/${environment}`);

Current main (after PR #42 merged):

const data = await api.patch<PatchSecretsResponse>(secretsUrl(project, environment), {
  delete: [key],
});
if (data.deleted === 0) {
  error(`Secret "${key}" not found in ${project}/${environment}`);
  process.exit(4);
}
success(`Deleted secret "${key}" from ${project}/${environment}`);

Root cause: This branch was created from main before PR #42 ("fix: error when deleting non-existent secret", commit ca20824) was merged. The branch lacks that commit, so GitHub shows it as a deletion from main.

If this PR merges as-is, it silently reverts the PR #42 fix — kfl secrets delete will succeed with exit 0 even when the secret doesn't exist.

Required fix: Rebase fix/spawn-deprecation-warning onto current main:

git fetch origin
git rebase origin/main
# In the conflict: keep main's version of runSecretsDelete (with the deleted === 0 check)

After rebase, the diff should show only the runRun spawn change — no runSecretsDelete changes at all.

---

✅ Core Change: DEP0190 Fix (Correct)

The spawn change itself is correct:

// Before
const child = spawn(cmd[0], cmd.slice(1), { env, stdio: 'inherit', shell: true, detached: true });

// After
const shell = process.env.SHELL || (process.platform === 'win32' ? 'cmd.exe' : '/bin/sh');
const commandString = cmd.join(' ');
const child = spawn(shell, ['-c', commandString], { env, stdio: 'inherit', detached: true });

• spawn({ shell: true }) internally does exactly spawn(sh, ['-c', argsJoined]) — doing it explicitly suppresses the deprecation with no behavioral change
• process.env.SHELL → /bin/sh fallback → cmd.exe on Windows is the right portability pattern
• cmd.join(' ') produces the same command string — semantically equivalent to shell: true
• detached: true, stdio: 'inherit', error/close handlers are all unchanged ✅

Minor nit (non-blocker): The new comment says "proper argument escaping" but cmd.join(' ') doesn't add escaping — it's the same unescaped join as before. The comment is slightly misleading; the real win is suppressing the deprecation warning.

---

Checklist

Item	Status
Fixes DEP0190 warning	✅
Shell operators (pipes, &&) still work	✅
detached: true preserved (process group kill)	✅
No new dependencies	✅
TypeScript clean	✅
No regression in runRun	✅
No regression in runSecretsDelete	❌ Needs rebase onto main

Action needed: Rebase and re-push — then this is good to merge.