Skip to content

feat(tui): warn when creating a panel with no experts#1754

Merged
pedrofuentes merged 4 commits into
mainfrom
feature/tui-create-noexperts
Jun 26, 2026
Merged

feat(tui): warn when creating a panel with no experts#1754
pedrofuentes merged 4 commits into
mainfrom
feature/tui-create-noexperts

Conversation

@pedrofuentes

Copy link
Copy Markdown
Owner

PanelCreateScreen now shows a clear teacher ('No experts yet — create an expert first') instead of a silent empty member list when there are no experts; Phase 11 polish — feedback #2

pedrofuentes and others added 2 commits June 26, 2026 10:17
Co-authored-by: Copilot <175574315+pedrofuentes@users.noreply.github.com>
Co-authored-by: Copilot <175574315+pedrofuentes@users.noreply.github.com>
@pedrofuentes

pedrofuentes commented Jun 26, 2026

Copy link
Copy Markdown
Owner Author

Status: REJECTED

Sentinel Review Report

Ref: feature/tui-create-noexperts → main
Report ID: SENTINEL-1754-eaa70f1
Reviewed SHA: eaa70f1
Sentinel ruleset: v1
Reviewed at: 2026-06-26T10:25:36-07:00
Mode: standard
Review depth: Tier 2 (full)
Required action: FIX_AND_REINVOKE
Persisted report: #1754 (comment)

Phase 1 — TDD / Test Evidence

  • Tests exist & meaningful: ❌ Partial. The warning render tests are meaningful and fail pre-implementation, but the focus-advance behavior is not tested (see 🔴 finding).
  • Test-first history verified: ✅ git log --oneline origin/main..origin/feature/tui-create-noexperts shows 5e48e2b test(tui): ... before eaa70f1 feat(tui): ...; per-commit diffs show the test commit changed only packages/cli/tests/unit/tui/panel-create-screen.test.tsx, and the feat commit changed only packages/cli/src/tui/screens/PanelCreateScreen.tsx.
  • Full suite green on SHA: ✅ Scoped final evidence at reviewed SHA: tests/unit/tui passed 95 files / 902 tests; focused panel-create-screen.test.tsx passed 12/12; lint and typecheck exited 0.
  • Coverage: N/A (coverage threshold not requested/enforced for this scoped review).
  • Test fails without implementation: ✅ At test commit 5e48e2b, panel-create-screen.test.tsx failed 2/12 on missing No experts yet assertions, proving the warning tests exercise the real screen.

Phase 1.5 — Fast-path Evaluation

🔴 count: 0 | LOC: 26 non-test insertions / 14 non-test deletions (≤150: Y) | Security paths: Y (terminal UI rendering surface) | New deps: N | Commit types qualify: N (feat)
→ Fast-path eligible: NO → Phase 2

Phase 2 — Execution Log

Commands run:

  • git fetch origin && git log --oneline origin/main..origin/feature/tui-create-noexperts && git diff origin/main...origin/feature/tui-create-noexperts
  • git show --patch 5e48e2ba1710708d625c023bdb2c04c6ee9f06ad -- packages/cli/tests/unit/tui/panel-create-screen.test.tsx
  • git show --patch eaa70f18d880eca3e60376fb0da2964567d13133 -- packages/cli/src/tui/screens/PanelCreateScreen.tsx
  • git worktree add --detach .worktrees/sentinel-1754-review eaa70f18d880eca3e60376fb0da2964567d13133
  • npx --yes pnpm@10.30.1 install --frozen-lockfile (needed after initial run failed with missing kysely module in the review worktree)
  • cd packages/cli && npx --yes pnpm@10.30.1 exec vitest run tests/unit/tui → ✅ 95 test files passed; 902 tests passed
  • cd packages/cli && npx --yes pnpm@10.30.1 lint → ✅ eslint src tests, exit 0
  • cd packages/cli && npx --yes pnpm@10.30.1 typecheck → ✅ tsc --noEmit, exit 0
  • git checkout 5e48e2b && cd packages/cli && npx --yes pnpm@10.30.1 exec vitest run tests/unit/tui/panel-create-screen.test.tsx → ✅ expected RED: 1 file failed; 2 failed / 10 passed / 12 total
  • git checkout eaa70f1 && cd packages/cli && npx --yes pnpm@10.30.1 exec vitest run tests/unit/tui/panel-create-screen.test.tsx → ✅ 1 file passed; 12 tests passed
  • gh pr comment 1754 --body-file - → persisted report: feat(tui): warn when creating a panel with no experts #1754 (comment)

Dimension dispatch:

Dim Tool Call Agent ID / Ref Status
Quick scan task(agent_type="general-purpose", model="claude-haiku-4.5", name="sentinel-fast-scan") N/A (sync task tool did not expose an agent id) ✅ 0 🔴; escalated to Tier 2 because fast-path criteria failed
A1 task(agent_type="general-purpose", name="sentinel-dim-a1") N/A (sync task tool did not expose an agent id) ✅ No findings
A2 task(agent_type="general-purpose", name="sentinel-dim-a2") N/A (sync task tool did not expose an agent id) ✅ No findings
B task(agent_type="general-purpose", name="sentinel-dim-b") N/A (sync task tool did not expose an agent id) ✅ No findings
C task(agent_type="general-purpose", name="sentinel-dim-c") N/A (sync task tool did not expose an agent id) ✅ No findings
D task(agent_type="general-purpose", name="sentinel-dim-d") N/A (sync task tool did not expose an agent id) ✅ No findings
E N/A N/A N/A (no dependency surface changed)
F task(agent_type="general-purpose", name="sentinel-dim-f") N/A (sync task tool did not expose an agent id) ✅ Returned focus-control issue; orchestrator reclassified as correctness/TDD blocker

Findings

  • 🔴 CRITICAL: 1
  • 🟡 IMPORTANT: 0 new / 0 known
  • 🟢 MINOR: 0

Details (ordered by severity)

  1. 🔴 Empty-experts focus guard is incomplete and untested — packages/cli/src/tui/screens/PanelCreateScreen.tsx:88-90,130-133; packages/cli/tests/unit/tui/panel-create-screen.test.tsx:257-287
    • Evidence:
      if (key.tab) {
        setField((current) => (current === "name" ? "members" : "name"));
      }
      if (!hasNoExperts) {
        setField("members");
      }
      expect(lastFrame()).toContain("No experts yet");
      expect(lastFrame()).toContain("create an expert");
    • Impact: Trigger: a user creates a panel while experts.loadList() returns [] and presses Tab (the UI still advertises Tab focus). Mechanism: the global useInput Tab handler still sets field to "members" without checking hasNoExperts, while the new guard only covers TextInput.onSubmit. Consequence: focus can still advance to an empty/nonexistent Members control, violating the PR intent and leaving the new focus behavior without a regression test.
    • Remediation: Gate the Tab focus transition when hasNoExperts is true (or keep focus on name whenever no member list is rendered) and add a no-experts keyboard test that writes \t and/or \r and asserts focus does not move to Members.

Follow-ups & Actions

  • REJECTED → FIX_AND_REINVOKE: fix 🔴 blockers only, re-commit, re-invoke Sentinel. Do not merge.

Decision rationale

  • TDD ordering and commit purity are compliant.
  • The warning copy is a trusted literal; terminal sanitization is N/A for that new string, and existing dynamic terminal strings in this screen use toSingleLineDisplay.
  • The submit guard requiring at least one selected expert remains unchanged.
  • Final scoped tests, lint, and typecheck pass at eaa70f18d880eca3e60376fb0da2964567d13133.
  • The PR’s stated focus behavior is incomplete because Tab still reaches the empty Members field and no test covers that path.

Verdict: REJECTED (reviewed SHA: eaa70f1)

pedrofuentes and others added 2 commits June 26, 2026 10:34
Co-authored-by: Copilot <175574315+pedrofuentes@users.noreply.github.com>
Co-authored-by: Copilot <175574315+pedrofuentes@users.noreply.github.com>
@pedrofuentes

pedrofuentes commented Jun 26, 2026

Copy link
Copy Markdown
Owner Author

Status: APPROVED

Sentinel Review Report

Ref: feature/tui-create-noexperts → main
Report ID: SENTINEL-1754-9a49a15
Reviewed SHA: 9a49a15
Sentinel ruleset: v1
Reviewed at: 2026-06-26T10:43:42-07:00
Mode: standard
Review depth: Tier 2 (full)
Required action: MERGE
Persisted report: #1754 (comment)

Phase 1 — TDD / Test Evidence

  • Tests exist & meaningful: ✅ New regression test exercises the Tab focus path by typing after Tab and asserting the name field still contains the new character: expect(lastFrame()).toContain("abcd"); (packages/cli/tests/unit/tui/panel-create-screen.test.tsx:291-305).
  • Test-first history verified: ✅ Chronological history shows e1fbf75 test(tui): assert Tab keeps focus on name when no experts before 9a49a15 fix(tui): keep focus on name when creating a panel with no experts; per-commit file lists confirm e1fbf75 changed only packages/cli/tests/unit/tui/panel-create-screen.test.tsx and 9a49a15 changed only packages/cli/src/tui/screens/PanelCreateScreen.tsx.
  • RED→GREEN proof: ✅ At e1fbf75, vitest run tests/unit/tui/panel-create-screen.test.tsx failed with AssertionError: expected 'Name: abc\n\u001b[7mMembers:\u001b[27…' to contain 'abcd' and 1 failed | 12 passed (13); at 9a49a15, the same file passed 13 passed (13).
  • Relevant suite green on SHA: ✅ tests/unit/tui passed 95 passed (95) test files and 903 passed (903) tests at 9a49a15c62afe58019e7145aa71c72aa69c43f6f; npx --yes pnpm@10.30.1 lint and npx --yes pnpm@10.30.1 typecheck both exited 0.
  • Coverage: N/A (not enforced for this scoped re-review; no coverage threshold command requested).

Phase 1.5 — Fast-path Evaluation

🔴 count: 0 | LOC: 4 non-test lines changed (≤150: Y) | Security paths: Y (terminal UI surface touched) | New deps: N | Commit types qualify: Y
→ Fast-path eligible: NO → Phase 2

Phase 2 — Execution Log

Dim Tool Call Agent ID / Ref Status
Quick scan task(agent_type="explore", name="sentinel-quick-scan", model="claude-haiku-4.5") N/A (tool returned sync result, no ID) ✅ 0 🔴
A1 task(agent_type="general-purpose", name="sentinel-dim-a1", model="claude-sonnet-4.6") N/A (tool returned sync result, no ID) ✅ No findings
A2 task(agent_type="general-purpose", name="sentinel-dim-a2", model="claude-sonnet-4.6") N/A (tool returned sync result, no ID) ✅ No findings
B task(agent_type="general-purpose", name="sentinel-dim-b", model="claude-sonnet-4.6") N/A (tool returned sync result, no ID) ✅ No material scoped findings
C task(agent_type="general-purpose", name="sentinel-dim-c", model="claude-sonnet-4.6") N/A (tool returned sync result, no ID) ✅ No findings
D task(agent_type="general-purpose", name="sentinel-dim-d", model="claude-sonnet-4.6") N/A (tool returned sync result, no ID) ✅ No material scoped findings
E N/A N/A ✅ Auto-skip: no dependency/supply-chain surface changed
F task(agent_type="general-purpose", name="sentinel-dim-f", model="claude-haiku-4.5") N/A (tool returned sync result, no ID) ✅ No material findings
Persist gh pr comment 1754 --body-file .sentinel/reports/SENTINEL-1754-9a49a15.md #1754 (comment) ✅ Full report posted to PR

Commands run:

  • git fetch origin && git --no-pager status --short --branch && git --no-pager log --oneline origin/main..origin/feature/tui-create-noexperts && git --no-pager diff --name-only eaa70f1...origin/feature/tui-create-noexperts && git --no-pager diff --stat eaa70f1...origin/feature/tui-create-noexperts
  • git --no-pager diff eaa70f1...origin/feature/tui-create-noexperts -- packages/cli/src/tui/screens/PanelCreateScreen.tsx packages/cli/tests/unit/tui/panel-create-screen.test.tsx
  • git --no-pager show --stat --patch --find-renames e1fbf75 and git --no-pager show --stat --patch --find-renames 9a49a15
  • git checkout e1fbf75 && cd packages/cli && npx --yes pnpm@10.30.1 exec vitest run tests/unit/tui/panel-create-screen.test.tsx → expected FAIL (1 failed | 12 passed (13))
  • git checkout 9a49a15 && cd packages/cli && npx --yes pnpm@10.30.1 exec vitest run tests/unit/tui/panel-create-screen.test.tsx → PASS (13 passed (13))
  • git rev-parse HEAD && cd packages/cli && npx --yes pnpm@10.30.1 exec vitest run tests/unit/tui && npx --yes pnpm@10.30.1 lint && npx --yes pnpm@10.30.1 typecheck → PASS (95 passed (95), 903 passed (903))
  • git --no-pager grep -n -E 'setField|"members"|hasNoExperts|noExperts|warning|No experts' origin/feature/tui-create-noexperts -- packages/cli/src/tui/screens/PanelCreateScreen.tsx

Findings

  • 🔴 CRITICAL: 0
  • 🟡 IMPORTANT: 0 new / 0 known
  • 🟢 MINOR: 0

Details (ordered by severity)

  1. [RESOLVED 🔴] Previous no-experts Tab focus leak — packages/cli/src/tui/screens/PanelCreateScreen.tsx:88-92
    • Evidence: const noExperts = expertState.status === "loaded" && expertState.data.length === 0; / if (!noExperts) { / setField((current) => (current === "name" ? "members" : "name"));
    • Impact: The prior rejected SHA allowed Tab to focus the absent members control when the expert list was loaded-empty; RED proof showed the new test failed before the fix.
    • Remediation: Resolved by guarding the Tab focus sink with the same loaded-empty source used by the no-experts render path.
  2. [RESOLVED 🔴] Exhaustiveness of focus-to-members sinks — packages/cli/src/tui/screens/PanelCreateScreen.tsx:133-137
    • Evidence: onSubmit={() => { / if (!hasNoExperts) { / setField("members");
    • Impact: The Enter/onSubmit focus sink remains guarded, and grep found no other setField sink to "members" beyond the guarded Tab and onSubmit paths.
    • Remediation: Verified no remaining unguarded loaded-empty focus path to members.
  3. [Verified] With-experts Tab behavior and warning/sanitization — packages/cli/src/tui/screens/PanelCreateScreen.tsx:143-147
    • Evidence: props.theme.warn( / "No experts yet — press Esc, then create an expert (Experts → n) before building a panel.",
    • Impact: Warning copy is a trusted literal; no new unsanitized dynamic terminal string was introduced. Existing with-experts test still passes through Tab/member selection, and the final TUI suite passed.
    • Remediation: None required.

Follow-ups & Actions

  • APPROVED → MERGE: no new 🟡/🟢 issues to file from this scoped re-review.
  • ⚠️ Do NOT fix 🟡/🟢 findings in this PR — none were accepted as material scoped findings.

Decision rationale

  • The new TDD pair is correctly ordered and pure: test-only commit e1fbf75 precedes implementation-only commit 9a49a15.
  • The new test genuinely fails without the fix and passes at the reviewed SHA, proving regression coverage for the rejected focus path.
  • Both loaded-empty focus-advance sinks to members are guarded (Tab via noExperts; Enter/onSubmit via hasNoExperts), and grep found no additional setField("members") path.
  • Relevant TUI tests, lint, and typecheck all pass at the reviewed SHA.
  • Phase 2 found no material in-scope security, resilience, performance, testing, dependency, or documentation blockers.

@pedrofuentes

Copy link
Copy Markdown
Owner Author

Sentinel re-review SENTINEL-1754-9a49a15 → APPROVED (0 🔴/🟡/🟢). Tab + onSubmit focus sinks both guarded against the no-experts case; RED→GREEN proven. Merging.

@pedrofuentes pedrofuentes merged commit 802845e into main Jun 26, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant