Skip to content

Commit window got Title and Description boxes + keyboard shortcut. - #391

Open
Styrse wants to merge 4 commits into
dcouple:mainfrom
Styrse:commit-message-title
Open

Commit window got Title and Description boxes + keyboard shortcut.#391
Styrse wants to merge 4 commits into
dcouple:mainfrom
Styrse:commit-message-title

Conversation

@Styrse

@Styrse Styrse commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Description

Adds dedicated Title and optional Description fields to commit dialogs,
matching how GitHub displays commit messages. Also adds Ctrl+Enter /
Cmd+Enter as a keyboard shortcut for submitting commits.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the code style of this project

  • I have performed a self-review of my own code

  • My changes generate no new warnings

  • I have added tests that prove my fix is effective or that my
    feature works

  • New and existing unit tests pass locally with my changes

  • I have run pnpm typecheck and pnpm lint locally

  • I have tested the Electron app locally with pnpm electron-dev

Critical Areas Modified

None.

Screenshots

Screenshot of the updated commit dialog with separate Title and
Description fields.

Pane-commit-box

Additional Notes

Commit messages are still sent through the existing IPC contract. Pane
combines the title and description using Git’s conventional blank-line
separator.

Automated QA

Status: Passed at 3b391ee6.

  • Main repository commit dialog: verified empty-title disablement, separate Title and optional Description fields, exact composed message, and Ctrl+Enter submission.
  • Review-panel commit dialog: verified the generated Update 1 file title, optional description, composed message, and Ctrl+Enter submission.
  • Automated checks: Playwright 2/2, frontend Vitest 272/272, focused main IPC Vitest 12/12, repository typecheck and lint.
Surface Evidence
Main repository Main repository commit dialog with title and description
Review panel Review commit dialog with title and description

Remaining for human review: final platform-specific Cmd+Enter sanity check on macOS.

@parsakhaz parsakhaz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: Request changes - the shared dialog prop change leaves the main-repository call site uncompilable.
Counts: Must Fix: 1 (security: 0) · Should Fix: 0 · pass 1/3

Must Fix

  • MF-1 - Remove the stale setCommitMessage prop from the main-repository CommitMessageDialog call site · frontend/src/components/ProjectView.tsx:428 · update every call site of the changed props contract · violates the PR intent because the branch cannot build
    • Evidence: pnpm --filter frontend typecheck reports TS2322: Property setCommitMessage does not exist on type CommitMessageDialogProps at this line.
    • Failure scenario: CI or a local production build stops at TypeScript compilation, so the title/description UI cannot ship.

Praise

  • The shared compose/split utility preserves Git's conventional blank-line separator and normalizes CRLF input without changing the IPC contract.
  • No security defects were found in the changed message-handling path.

⚠️ Cannot verify

  • The required code-reviewer role file .claude/agents/code-reviewer.md is absent from the repository, so this review used the installed report contract and direct correctness/security inspection but could not apply the missing repository-specific role rubric.

Checks run: pnpm --filter frontend typecheck (failed as cited); source and call-site trace across both commit dialogs.

@parsakhaz
parsakhaz force-pushed the commit-message-title branch from a010951 to 9874279 Compare August 23, 2026 19:51

@parsakhaz parsakhaz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: Approve - MF-1 is fixed and the rebased branch passes the local review gates.
Counts: Must Fix: 0 (security: 0) · Should Fix: 0 · pass 2/3

  • MF-1 fixed: the stale setCommitMessage prop was removed from frontend/src/components/ProjectView.tsx; repository typecheck now passes.
  • Security recheck found no defects in commit-message composition or the new main-process validation boundary.

Checks: pnpm typecheck, pnpm lint, frontend Vitest 272/272, focused main IPC tests 12/12, commit-dialog Playwright 2/2.

@parsakhaz
parsakhaz force-pushed the commit-message-title branch from 9874279 to 3b391ee Compare August 23, 2026 19:54
@parsakhaz

parsakhaz commented Aug 23, 2026

Copy link
Copy Markdown
Member

Three-pass review complete

REVIEW

  • Rebased the PR onto current main, preserving main's focus-timer cleanup.
  • Posted a request-changes review for one real Must-Fix: ProjectView still passed the removed setCommitMessage prop and failed TypeScript compilation.
  • Fixed it in 3a3c1894 (fix(review): remove stale commit dialog prop), rechecked security, and posted an approval review. No security defects remain.

SIMPLIFY

  • Committed b203649c (refactor(simplify): reuse commit message predicates).
  • Removed redundant array mutation from message splitting.
  • Reused the dialog's existing canConfirm predicate for both action buttons.
  • Before and after results were identical or better: repository typecheck and lint passed, with no advisory anti-slop findings; frontend Vitest improved from 270 to 272 passing tests after the later deep-pass coverage.

REFACTOR

  • Committed 3b391ee6 (refactor: share commit message boundary).
  • Moved commit-message composition, splitting, title validation, and shortcut recognition into one shared renderer/main utility.
  • Enforced the non-empty title contract at all three main-process commit entry points: git:commit, sessions:git-stage-and-commit, and sessions:squash-and-rebase-to-main.
  • Unified Ctrl/Cmd+Enter behavior and blocked repeated or disabled submissions.
  • Removed dead main-repository commit-message state.
  • Added focused IPC coverage and two Playwright journeys for both commit dialog surfaces.

Verification

  • pnpm typecheck: passed.
  • pnpm lint: passed, including Oxlint, ESLint, boundary conformance, Knip, and zero advisory anti-slop findings.
  • Frontend Vitest: 27 files, 272 tests passed.
  • Main IPC Vitest: 1 file, 12 tests passed.
  • Playwright tests/commit-dialog.spec.ts: 2/2 passed at current head 3b391ee6.
  • An accidental main watch-mode invocation also exercised most of the broad suite; two unrelated SkillCacheManager network/cache tests timed out. The intended focused IPC run passed cleanly.
UI evidence
Journey Preview
Main repository commit Separate fields filled before verified Ctrl+Enter submission.
Main repository commit dialog with title and description
Review-panel commit Generated title and optional description before verified Ctrl+Enter submission.
Review commit dialog with title and description

Follow-ups

  • Consider unifying ui/Input.tsx's textarea and ui/Textarea.tsx before extracting a shared visual commit-message field component. They currently have intentionally different presentation contracts, so this pass kept shared logic below the UI.
  • Restore the missing .claude/agents/code-reviewer.md role file so future code-reviewer runs can apply the complete repository-specific rubric.

Left for parsa

  • Fresh CI passed at 3b391ee6 across macOS, Ubuntu, Windows, quality/smoke, wrapper, security, and React checks.
  • Optionally do one native macOS Cmd+Enter sanity check.
  • Merge when satisfied. I did not merge.

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.

2 participants