Feature Requirements: review-fixes-before-merge

Feature Requirements: review-fixes-before-merge

Metadata

• Feature: review-fixes-before-merge
• Status: APPROVED
• Created: 2026-02-15
• Author: Factory Plan Mode
• Parent: fix/codeql-scanning-alerts (addendum to existing branch)

1. Problem Statement

1.1 Background

Code review of the fix/codeql-scanning-alerts branch identified 3 issues: a double-sanitization bug causing display corruption, missing shlex.quote() on temp script paths, and unverified CodeQL alert dismissals.

1.2 Problem

• Double sanitization: _sanitize_pr_content() called at parse time (line 198) AND output time (line 370) in pr_engine.py. Causes <Tag> → <Tag> → <Tag> in PR bodies.
• Unquoted script path: GIT_SEQUENCE_EDITOR set via f-string without quoting at history_engine.py:573,664. Breaks if TMPDIR contains spaces.
• Unverified dismissals: FR-009 (cyclic imports) and FR-011 (undefined exports) used GitHub UI dismissal. Need confirmation they're resolved.

1.3 Impact

Without fixes: commit messages with <, >, & render as < in PR bodies; edge case failure with spaces in temp paths; incomplete audit trail for CodeQL dismissals.

2. Users

2.1 Primary Users

• ZERG maintainers creating PRs via zerg git pr
• ZERG maintainers using zerg git rewrite

2.2 User Stories

• As a maintainer, I want PR bodies to render commit messages correctly so angle brackets and ampersands display properly
• As a maintainer, I want rebase scripts to work regardless of temp directory path

3. Functional Requirements

3.1 Detailed Changes

FR-001 (pr_engine.py:198):

# BEFORE (line 198):
message=_sanitize_pr_content(message.strip()),

# AFTER:
message=message.strip(),

Store raw commit messages in CommitInfo. Sanitization happens at output boundary (lines 370, 381, 401, 411).

FR-002/FR-003 (history_engine.py:573, 664):

# BEFORE:
env["GIT_SEQUENCE_EDITOR"] = f"python3 {script_path}"

# AFTER:
env["GIT_SEQUENCE_EDITOR"] = f"python3 {shlex.quote(script_path)}"

Requires import shlex at top of file (or verify existing).

FR-004 (verification only):

• gh api repos/rocklambros/zerg/code-scanning/alerts confirms 0 open alerts for cyclic-import and undefined-export rules.
• No code changes needed. Document as verified in CHANGELOG.

3.2 Business Rules

• No behavioral changes to PR content beyond fixing double-encoding
• _parse_commit_type() operates on raw message (line 193), before sanitization was applied — unaffected by this change
• All 6 call sites of _sanitize_pr_content reviewed: only line 198 is redundant

4. Non-Functional Requirements

4.1 Performance

• No performance impact

4.2 Security

• Sanitization at output boundary is the standard secure pattern (OWASP)
• shlex.quote() prevents shell injection in edge cases

4.3 Reliability

• All existing tests must pass
• ruff check must show no new lint errors

5. Scope

5.1 In Scope

• pr_engine.py: Remove redundant parse-time sanitization
• history_engine.py: Add shlex.quote() to 2 locations
• Verify CodeQL alert status for FR-009/FR-011
• CHANGELOG.md update

5.2 Out of Scope

• Refactoring _sanitize_pr_content() itself
• Adding tests for shlex.quote edge case (TMPDIR with spaces is OS-level)
• Any other CodeQL fixes

5.3 Constraints

• Changes go on existing fix/codeql-scanning-alerts branch
• Must not break any existing tests

6. Dependencies

7. Acceptance Criteria

• PR body renders <Tag> correctly (no double-encoding)
• shlex.quote() wraps script_path at both locations
• python -m pytest tests/ --timeout=120 — green
• ruff check zerg/git/pr_engine.py zerg/git/history_engine.py — clean
• CodeQL cyclic-import/undefined-export alerts confirmed at 0

8. Open Questions

9. Approval

10. Documentation Impact Analysis

11. Estimated Effort

3 files changed, ~6 lines modified. Minimal risk. ~5 minutes implementation.