#183 feat: Sprint 1+2 — session lifecycle, auth UX, credential & trust model (#147, #160, #148, #106, #157, #158, #163, #54)

[Anonymous]

Originally created by: kumaakh

Summary

Combined Sprint 1 (Cluster A + D) and Sprint 2 (Cluster C) delivering session reliability and security hardening across 8 core issues, with post-review fixes and two additional features (#191, [#192]).

---

Sprint 1 — Session Lifecycle + Auth UX

• #147 — execute_prompt: kill previous agent instances — PID capture via shell wrapper; existing session killed before new one starts. Prevents zombie agents accumulating across retries.
• #160 — Activity-aware timeout — Rolling inactivity timer resets on each output event; timeout_ms is now an inactivity timeout, not wall-clock. Added max_total_ms as hard ceiling.
• #148 — Background agent cancellation — stop_prompt kills the member-side LLM process and sets a stopped flag; subsequent dispatches to a stopped member are rejected with a clear error.
• #106 — OOB SSH/headless fix — Environment detection before attempting GUI terminal spawn. SSH sessions receive actionable instructions with the real member name. Additional ProcessStartInfo fix for headless stdout pipe inheritance on Windows.

Sprint 2 — Credential & Trust Model

• #157 — Credential scoping — credential_store_set accepts members param. Fleet server enforces access at resolution time using calling member identity from request context. Rejection error names the allowed members.
• #158 — Credential TTL — credential_store_set accepts ttl_seconds. Expiry stored as absolute timestamp; rejected at resolution time with clear error; purged at server startup. credential_store_list shows remaining time.
• #163 — provision_vcs_auth isolation — Added label + scope_url params. Each call writes ~/.fleet-git-credential-<label> and a scoped gitconfig entry. Multiple identities coexist. revoke_vcs_auth removes by label and now accepts scope_url for full cleanup. Windows backslash-in-gitconfig bug fixed. Legacy single-file credentials auto-migrated.
• #54 — Unattended mode — dangerously_skip_permissions deprecated in execute_prompt (warning + ignored). unattended: false | 'auto' | 'dangerous' added to register_member / update_member / Agent type. Claude/Codex map to correct CLI flags; Gemini/Copilot log warnings. SKILL.md updated.

Post-review Fixes

• Windows PID fix — pidWrapWindows previously captured PowerShell's own $PID; fixed to capture the real Claude CLI PID via Start-Process -PassThru -NoNewWindow. taskkill /F /T /PID now terminates the full process subtree.
• SECURITY: Label injection hardened — label param now validated against /^[a-zA-Z0-9_-]{1,64}$/ in both provisionVcsAuthSchema and revokeVcsAuthSchema. Windows credFileName path also sanitised.
• Race condition fixed — In-flight Set guard in execute_prompt prevents two concurrent calls for the same agent from both passing the stopped check.
• revoke_vcs_auth scope_url — Schema now accepts and propagates scope_url, so credentials deployed with a custom scope can be fully revoked. Orphaned gitconfig entries no longer occur.
• Duplicated PROVIDER_HOSTS — Extracted to a single shared constants file; both provision-vcs-auth.ts and revoke-vcs-auth.ts import from it.
• Dead fields removed — dangerouslySkipPermissions removed from PromptOptions; activePid removed from Agent interface.
• LocalStrategy hardened — child.kill('SIGKILL') (was SIGTERM); both inactivityTimer and maxTotalTimer now call .unref() matching the SSH path.
• Test cleanup — Empty afterEach in credential scoping tests now performs proper credential cleanup; no credential leaks on test failure.
• Docs cleanup — feedback.md moved to docs/reviews/feedback-b238154.md; .gitignore CLAUDE.md entry annotated with explanation.

Additional Features (#191, [#192])

• #191 — credential_store_update — New tool to update an existing credential's value, members scope, or TTL without delete-and-recreate.
• #192 — Dangerous mode for Gemini/Codex — unattended='dangerous' now wired to correct provider flags for Gemini and Codex (previously logged a warning only).
• Skill file improvements — Session resume, stop_prompt, unattended modes, timeout semantics, and credential scoping all documented in SKILL.md.

---

Test plan

• 1009 tests passing, 0 failures, 7 skipped (up from 906 baseline; 11 duplicate tests removed in Sprint 3 audit)
• 17 credential scoping+TTL tests, 7 VCS isolation tests, 16 unattended mode tests, 4 Windows-specific tests
• Integration test plan (28 test cases) covering all Sprint 2 issues end-to-end
• Security audit: member identity spoofing, TTL bypass, label injection, unattended self-escalation — all SECURE

Sprint 3 — Stop-Prompt Fix, Provider Abstraction, Structured Logging & Test Cleanup

Bug fixes:

• T7 — stop_prompt PID fix — FLEET_PID: now extracted from stdout data stream in real-time (not post-close) in both LocalStrategy (strategy.ts) and RemoteStrategy (ssh.ts). clearStoredPid called in close/error handlers. stop_prompt now actually kills the correct LLM process.
• T8 — Provider flag abstraction — windows.ts was hardcoding --permission-mode auto for all providers. New permissionModeAutoFlag(): string | null method added to ProviderAdapter interface; all 4 providers implement it (Claude → --permission-mode auto, Gemini → null, Codex → --ask-for-approval auto-edit, Copilot → null + warning). windows.ts now delegates.
• T11 — windowsHide:true — spawn() in LocalStrategy defaulted windowsHide to false (unlike exec which defaulted to true), causing flashing cmd.exe/PowerShell windows on Windows members. Fixed with windowsHide: true in spawn options.

Structured logging (T9):

• New src/utils/log-helpers.ts — maskSecrets() (redacts {{secure.*}} and sec:// patterns before any log write), truncateForLog() (80 chars, collapses whitespace), logLine() (JSON to stderr).
• execute_prompt logs entry + exit (exit code + elapsed ms). execute_command logs entry. Local strategy logs child.pid at spawn. All via console.error (stderr) — nothing to stdout.

Skill doc review fixes (T1–T6):

• T1 — SKILL.md: Blanket "unattended='auto' does not add any CLI flag" replaced with per-provider flag table covering auto + dangerous modes for all 4 providers.
• T2 — SKILL.md: credential_store_update added to Core Fleet Tools table.
• T3 — SKILL.md: Copilot unattended limitation documented in provider table (both modes unsupported).
• T4 — doer-reviewer.md: Gemini-specific auto-approval implementation detail replaced with SKILL.md cross-reference.
• T5 — doer-reviewer.md: Sub-bullet formatting fixed; checklist item 4 now has properly indented sub-bullets.
• T6 — linux.ts: All ${credFile} occurrences in shell strings double-quoted — defense-in-depth against paths with spaces (5 locations: >, chmod 600, chmod +x, git config --add, rm -f).

Test suite cleanup (T10):

• Audited all 61 test files; removed 11 duplicate tests across 3 locations:
• tests/unit/inactivity-timer.test.ts — deleted (3 tests, exact duplicate of session-lifecycle.test.ts inactivity describe block)
• 4 tests removed from unattended-mode.test.ts — exact duplicates of windows-pid-wrap.test.ts section 4
• 4 tests removed from tests/unit/pid-wrapper.test.ts — exact duplicates of windows-pid-wrap.test.ts sections 1–2
• Net: 1020 → 1009 tests, 61 → 60 test files. No security boundary tests deleted.

---

Advisory items (deferred — not blocking)

These were flagged by the expert reviewer as non-blocking advisory items and are deferred to a future sprint:

• Duplicated inactivity timer logic — rolling timer + max_total_ms pattern implemented independently in ssh.ts and strategy.ts; fixes must be applied to both. Extraction to a shared utility is the right fix but touches both execution paths.
• pidWrapUnix PID capture — captures the backgrounded subshell PID, not the actual LLM process; if cmd contains a pipeline, kill -9 kills only the outer shell. Fragile but acceptable for current provider command structures.
• credentialSet negative ttl guard — schema validates z.number().positive() at the API boundary; internal callers can bypass. No current internal caller passes negative values; add guard in a follow-up.
• console.warn in Gemini/Copilot providers — warnings are invisible in MCP server context; should surface through the tool's return value.

Deferred (backlog)

• No dedicated unit test for Windows forward-slash path fix (#163)
• No dedicated unit tests for log-helpers.ts (maskSecrets, truncateForLog) — Sprint 3 advisory
• _activePids is in-memory only, not queryable externally and lost on server restart — stop_prompt effectiveness is unverifiable without new tooling to expose it via fleet_status or member_detail
• Async execute_prompt returning a task handle immediately + local monitor_task parity — root fix for stream watchdog problem (PM agent emits zero tokens while blocked on long tool call, 600s watchdog fires)

Generated with Claude Code

[Anonymous]

Ticket changed by: kumaakh

• status: open --> closed