Skip to content

[Duplicate] accidental resubmission of #5092 — please ignore #5094

Description

@JunZ-Leo

Problem / 问题

Three shipped readers parse git machine output with str.splitlines(). splitlines() treats U+0085 (NEL), U+2028 and U+2029 — plus U+000B, U+000C and U+001C–U+001E (measured; U+001F, the field separator this format itself uses, does not split) — as line breaks, while every --porcelain / --format= stream uses LF only as its record separator and emits the substituted values raw. So one record becomes several fragments, and the loop's own "skip malformed lines" guard silently swallows one of them.

This is the same root cause as #5057, which was fixed for the effect-runtime stderr channel in #5058: a record framed by a separator wider than the one the writer emits.

Reproduction 1 — a contributor's name is truncated, and the loss is silent

loopx/capabilities/issue_fix/reviewer_recommendation.py:631 _collect_history runs git log --format=%aN%x1f%aE (field separator \x1f, record separator LF) and frames it at :650 with output.splitlines(), then does if not separator: continue.

Measured with a commit whose author name is Zo + U+0085 + e:

raw stdout repr: 'Zo\x85e\x1fz@e.com\n'
records via splitlines(): 2
records via split(LF)   : 1
what the shipped loop sees: [("'Zo'", DROPPED - no separator), ("'e'", "HAS_SEP")]

The contributor is therefore published as e. That value is not cosmetic: the rows feed the recommendation filter at :839-850, where excluded_author_names (:848) and _normalise_author_name(row[0]) (:844) decide who stays eligible — so a person who was excluded by name can come back as a different string and remain eligible, and their real commits are attributed to a fragment.

Reproduction 2 — a worktree path is cut, and the wrong directory is written to

loopx/capabilities/integration_branch/core.py:523 _worktree_for_branch frames git worktree list --porcelain at :527 with result.stdout.splitlines(). Measured with a worktree at …/odd + U+0085 + dir:

real records (non-empty LF lines): 6
records via splitlines(): 9
parsed path: 'worktree /private/tmp/…/odd'      ← `dir` became a phantom record

Path(...).resolve() does not raise for a missing path, so nothing marks the truncation. The single caller (:660) then uses that value for _clean_worktree(checked_out_path) and, if it passes, _git(checked_out_path, "reset", "--hard", candidate_sha):

  • truncated path does not exist → git -C fails and the operator sees a worktree error (_git raises with git's own message), never "your path was mis-framed";
  • truncated path exists as another checkout → cleanliness is judged there and reset --hard is executed in the wrong working tree.

Desired outcome / 期望行为

Frame machine records by the separator the writer actually emits — LF — as #5058 did for the startup stderr channel, and keep the continue-on-malformed guard for genuinely malformed data instead of letting it absorb framing damage. Do not widen which characters are legal in a name or a path, and do not change what the recommendation logic excludes or what the integration sync writes.

Scope / 范围

Verified as machine-format streams (reproduced above): integration_branch/core.py:527 and issue_fix/reviewer_recommendation.py:650.

Same grep shape, not triaged — someone must read each before claiming it: issue_fix/acceptance_loop.py:156,243,246; change_quality/scope.py:58; doctor_git.py:177 (git remote get-url); canary/maintainability_ratchet.py:216 (git ls-files *.py); canary/runner.py:181 and cli_commands/canary.py:82; codex_cli_runtime_probe.py:234; dashboard_launcher.py:117; benchmark_toolkit/container_binding.py:95.

Explicitly excluded after checking: reviewer_recommendation.py:628 parses git diff --name-only, which cannot carry a raw U+0085 because core.quotePath octal-escapes non-ASCII pathnames by default. Any fix should state which streams are quoted and which are not, so the next reader does not re-report the safe one.

Acceptance / 验收

  • A commit whose author name contains U+0085 (and one containing U+2028) yields exactly one history row whose name is the full author name, and the DROPPED branch of the loop is not what consumes it.
  • A worktree whose path contains U+0085 resolves to the full path, and a torn prefix is never returned; the record count parsed equals the count of LF-terminated records.
  • Both regressions are shown to fail when framing is reverted to splitlines(), and the existing exclusion / integration-sync behaviours are unchanged for ordinary ASCII names and paths.
  • The excluded-but-adjacent git diff --name-only site is documented as safe rather than "fixed", with the quoting rule as the reason.

Related: #5057 / #5058 (same root cause, different channel). Not a duplicate of #4308 or #3558, which are about repository worktree selection and delivery verification, not record framing.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions