[devswha]

Change

Follow-ups from #1399's review (#1445), for Command Code (cmd) panes. Per item:

• 2, the footer check was strict (server/dialogs/cmd.ts, server/dialogs/live.ts): a permission prompt now gets its card when its footer wraps (split hint, word or character wrap) or uses the ctrl+e / ctrl+y variants. These are the shapes the published command-code 1.88.0 renderer draws (read statically from the npm package; the CLI was never run or logged into). A stale or incomplete footer, or anything drawn below it, still gives no card. Normal status chrome is hidden while a permission dialog is open, so it cannot sit under the footer.
• LOW, an ordinary numbered menu was labelled an approval card: a menu is an approval card only when its rows are Command Code's own Yes … No choices, so a theme picker with the same footer is not one.
• 4, herdr's model stuck in the transcript cache (server/conversation.ts, cmd metadata branch only): the served model follows the pane's current model, and the ETag changes with it. The response is a new object, so the cached page is never mutated. herdr's live model now also replaces a model recorded in the transcript, which is what the item asks for. This branch is only reached where herdr reports the pane's Command Code session, which no released herdr does yet (docs(guide): Command Code chat needs the session herdr reports, and none does yet #1647), so the changelog entry says so.
• 5, store validation copied from server/pi.ts: both now use server/transcript-store.ts. Behaviour is identical, and the old names are re-exported.
• 6, test gaps: tests for the cmd metadata branch, for the keys each answer sends from every cursor position, and for refusing a stale card over HTTP (409 prompt_changed, no keys sent). Session resolution and the skills source were already covered.
• LOW: the unused CmdTranscriptOptions.home is removed.

Not fixed

• 1, the card id covers everything above the rows, so a card goes stale when output above an open dialog redraws, as before. Narrowing the id to the dialog was tried and reverted: two review rounds showed that command text (a heredoc) can imitate the dialog's frame, including its real layout (rule, blank line, heading, subtitle). Then two different commands got the same id, and a stale card could be answered. Doing this safely needs Command Code to expose the dialog, or a capture proving a boundary that command text cannot imitate. A test pins the safe behaviour.
• LOW, a multi-line command's title: left as main draws it, for the same reason (it would need the same boundary guess).
• 3, the shell's cwd won over the agent's: already fixed on main by fix(chat): find Command Code's session in its own project when the shell is elsewhere #1745.
• LOW, the comment saying the body is part of the id: it is accurate with main's id, which covers everything above the rows, the body included.

Fixes #1445

Verification

• New tests fail on origin/main and pass here: footer shapes, the Yes/No menu check, the model and ETag refresh without a transcript append, and the answer keys. Tests from the reviews are kept as regressions. Heredoc and indentation changes give different ids, a quoted dialog frame in the real layout gives different ids, and file-permission card bodies equal main's. The stale-card route test gets 409 with no keys sent.
• Single-file runs after the rebase onto main: server/cmd.test.ts 56 pass, server/prompt.test.ts 357 pass, server/cmd-conversation.test.ts 2 pass, server/pi.test.ts 31 pass, server/dialogs/registry.test.ts 55 pass, server/dialogs/interaction-plan.test.ts 23 pass, server/dialogs/claude.test.ts 13 pass.
• Live check over HTTP, against a real headless herdr session of its own (not a fake socket) and the server's /api/pane/prompt routes. A pane reported as cmd drew Command Code's shell permission dialog at 80 columns, where the terminal wraps the footer mid-word (… bypass all perm / issions (Docs ↗)):
  • this branch: an approval card with Yes / Yes, don't ask again / No. origin/main: only the generic fallback card (↑ ↓ Enter Esc).
  • a theme picker with the same footer: this branch gives the generic menu card; origin/main labels it an approval card titled "Theme".
  • a line redrawn above the open dialog gives a new id; answering the old id gets 409 prompt_changed and no byte reaches the pane; answering the new id sends only Enter.
• Reviews:
  • Round 1: omo-native-code-reviewer and Codex (gpt-6.1-sol, xhigh) each found a HIGH in the narrowed card id (a heredoc hiding the command prefix).
  • Round 2: both found more ways around the narrowing, and file cards showing the whole screen. That led to restoring main's id.
  • Round 3: both APPROVE with no HIGH or MEDIUM.
  • After the rebase (one import conflict in server/pi.ts, where main's fix(pi): read the context window from the pane's own PI_CODING_AGENT_DIR #1899 added fileURLToPath): Codex (gpt-6.1-sol, xhigh) reviewed the rebase against the new main (fix(pi): read the context window from the pane's own PI_CODING_AGENT_DIR #1899's store and agent directory code, fix(chat): follow-ups from the agent command forms review #1893's cache ordering, fix(prompt): drop the unused rewind record, pin dialog reader order and remove dead registry paths #1890's dialog reader order) and APPROVED with no findings.
• bun run check fast on this head: workflow syntax, generated types and themes, typecheck, build and unit tests ok (6800 pass, 0 fail, 13 skip).

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: devswha/ovis
• Branch: fix/cmd-support-followups
• File: .coderabbit.yaml
• Commit: e7707c2
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: a5399676-f102-442d-8d68-4f6bd6181e72

Commits

Reviewing files that changed from the base of the PR and between ddee8be and e7707c2.

Files selected for processing: 11

• changelog.d/cmd-support-followups.md
• server/cmd-conversation.test.ts
• server/cmd.test.ts
• server/cmd.ts
• server/conversation.ts
• server/dialogs/cmd.ts
• server/dialogs/fallback.ts
• server/dialogs/live.ts
• server/pi.ts
• server/prompt.test.ts
• server/transcript-store.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Summary

Summary by CodeRabbit

• Bug Fixes
  • Permission prompts with wrapped footers or alternate explain-key options now display correctly in Chat, while unrelated numbered menus remain unaffected.
  • Approval cards are rejected if the command or heredoc details change after the card appears, preventing responses to outdated prompts.
  • Chat now shows the pane’s current model after switching models when session information is available.

The changes update Command Code permission-card detection and stale-card handling, refresh pane model metadata in conversation responses, and centralize transcript-path validation for Command Code and Pi.

Changes: Command Code support follow-ups

Layer / File(s)	Summary
Permission-card detection and identity server/dialogs/cmd.ts, server/dialogs/live.ts, server/dialogs/fallback.ts, server/cmd.test.ts, server/prompt.test.ts, changelog.d/cmd-support-followups.md	Command Code prompts now require a complete recognized footer and matching approval-menu labels. Cmd card identity hashes the raw screen. Tests cover parsing, answer keys, and rejection of stale cards.
Conversation model metadata server/conversation.ts, server/cmd-conversation.test.ts, changelog.d/cmd-support-followups.md	A nonempty pane model reported by Herdr updates the copied cmd-transcript response metadata and its version. Tests cover model changes and unchanged requests.
Shared transcript-path validation server/transcript-store.ts, server/cmd.ts, server/pi.ts	Transcript path validation helpers are added to transcript-store.ts and re-exported from cmd.ts and pi.ts. CmdTranscriptOptions no longer includes home.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

• devswha/ovis#1745: This change replaces the transcript validators used by the cwd lookup added in that PR.
• devswha/ovis#1761: This change replaces the Command Code footer-tail check that PR moved into server/dialogs/live.ts.

Suggested reviewers: diegolhambi

Merge Risk: Minimal · up to e7707

The reported transcript-path escape was not reproduced. No actionable merge-blocking risk remains after normal checks.

Pre-merge checks: 3 · 2

Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Linked Issues check		#1445 requirements are partly implemented. cmdFooterStart validates wrapped and Ctrl+E/Ctrl+Y footers, rejects trailing content, and restricts approval cards to Yes through No rows. `paneConvers…	Implement and test a safe dialog-boundary or equivalent card-ID scope for #1445 item 1. Preserve the stale-card rejection guarantee. Implement and test complete multi-line command title handling, or record that objective as separately defer…
Docstring Coverage		Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 10 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 3 passed

Check name	Status	Explanation
Title check		The title clearly and concisely identifies the Command Code follow-up fixes covered by the pull request.
Description check		The description includes the required Change section and provides detailed behavior, scope, limitations, tests, HTTP validation, and check results under a Verification heading. The heading differs fro…
Out of Scope Changes check		The changed production code, shared transcript-store helpers, cache-version update, dialog parsing, fallback identity, changelog, and regression tests all support the Command Code follow-ups in #1445.…

Full details: Description check

Explanation

The description includes the required Change section and provides detailed behavior, scope, limitations, tests, HTTP validation, and check results under a Verification heading. The heading differs from the template's Validation heading, but the required information is present.

Full details: Out of Scope Changes check

Explanation

The changed production code, shared transcript-store helpers, cache-version update, dialog parsing, fallback identity, changelog, and regression tests all support the Command Code follow-ups in #1445. No unrelated change is demonstrated.

Full details: Linked Issues check

Explanation

#1445 requirements are partly implemented. cmdFooterStart validates wrapped and Ctrl+E/Ctrl+Y footers, rejects trailing content, and restricts approval cards to Yes through No rows. paneConversation returns the live model with a changed version without mutating the cache. Store validation is shared, and the new tests cover metadata, answer keys, and stale-card rejection. The cwd, session-resolution, and skills-source items are already covered by the reviewed head. However, #1445's screen-wide card-ID issue remains: server/dialogs/cmd.ts still hashes all content above the rows, so redraws above an open dialog still invalidate the card. The multi-line command title issue also remains explicitly unresolved. The live check and regression tests prove safe rejection, but they do not complete those two coding objectives.

Resolution

Implement and test a safe dialog-boundary or equivalent card-ID scope for #1445 item 1. Preserve the stale-card rejection guarantee. Implement and test complete multi-line command title handling, or record that objective as separately deferred in #1445.

Full details: Docstring Coverage

Explanation

Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 10 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[devswha]

Merge pass review-work report. Overall: PASSED. This is an ordinary server behaviour change, so it was gated by Codex Sol, an omo code reviewer and the merge pass's own checks.

Lane	Verdict
Codex (gpt-6.1-sol, xhigh)	APPROVE
Code review (omo-native-code-reviewer)	PASS, only LOW findings

Tests: 591 cmd, pi, prompt and dialog tests pass, and tsc is clean. With each fixed file reverted to main's version, its new tests fail:

• conversation.ts: 2 fail.
• dialogs/cmd.ts and live.ts: 7 fail.
• fallback.ts: the heredoc test fails.

Footer check: the reviewer read the published command-code@1.88.0 package and found the same footer shapes the new code expects.

• Any text below the footer gives no card.
• An ordinary numbered menu gives no approval card.
• Answers still go out as arrows then Enter. A stale card gets a 409 and sends nothing.

Model: only the cmd branch changes in conversation.ts. It returns new objects without mutating the cached page, and the ETag changes with the model (pane-routes.ts:260-265).

transcript-store.ts: the code is identical to the copies it replaces. Every old name is re-exported, and it has no import cycle.

Overlaps:

• Against feat(chat): show pictures Claude Code reads, and a picture-only prompt as a turn #1935 and fix(outcome): one process read per Chat poll, the stale-session minute pinned, and pi sessions over 16 MiB #1936, the maintainer PRs sharing server/conversation.ts, git merge-tree is clean.
• Against the outside PRs it overlaps with, fix(chat): recognize Pi's thinking selector #1670, fix(prompt): support GJC custom answers with guarded delivery #1669, fix(codex): recognize the Ultra main prompt under a collapsed question queue #1930 and fix(prompt): support Pi rpiv question cards and safe navigation #1714 are clean. fix(chat): keep Pi sends visible until transcript append #1795 conflicts; whichever merges second rebases.

LOW follow-ups:

• The context % is computed from the recorded model after a live model switch.
• Card matching is English-only.
• The alias re-export repeats.