Skip to content

Act on review findings from CODEREVIEW-2026-06-25-1 #43

Description

@mnsmarko

A new code review has been opened on branch CODEREVIEW-2026-06-25-1.

  • Report: docs/reviews/2026-06/CODEREVIEW-2026-06-25-1.md
  • TODO: docs/reviews/2026-06/TODO-CODEREVIEW-2026-06-25-1.md

Please review the findings and either fix, defer (ONHOLD), or reject (WONTFIX) each one. Record ONHOLD/WONTFIX in the report file itself so future reviews honor them — a GitHub comment alone is not enough. Only WONTFIX/ONHOLD findings are suppressed later; anything you leave unmarked will be re-reported by the next review if it's still present in the code.


Please leave this issue open until you're done

While this issue or its review PR stays open, the scheduler will not open another code review of this repo — so you won't get a second review stacked on top of one you're still working through. To free the repo for its next review you must resolve both:

  • Close this issue, and
  • Merge or close the review PRmerge it (recommended) to land the report with your WONTFIX/ONHOLD markers on the default branch, the record future runs read to avoid repeating deferred/rejected findings; or close it without merging to abandon this cycle and get a fresh review of newer code (nothing is recorded, so still-present findings come back).
Handover prompt for an AI coding agent (click to expand — paste into Claude Code / Codex in a clone of this repo, with gh authenticated)

You are picking up a completed dual-agent review. There is this open review PR (it contains only the dated report + TODO — no source changes) and a companion issue. Act on the findings, record my defer/reject decisions in the report, then merge the PR and close the issue.

  • Report: docs/reviews/2026-06/CODEREVIEW-2026-06-25-1.md
  • TODO: docs/reviews/2026-06/TODO-CODEREVIEW-2026-06-25-1.md
  • Review branch: CODEREVIEW-2026-06-25-1 · PR base: the default branch

0. Orient. Read the report + TODO on the review branch. Each finding has an ID like 2026-06-03-1-7 and a severity. Check out the base branch and pull latest — the review was a snapshot and code may have moved since.

1. Re-anchor every finding against the CURRENT code (commits may have landed after the review). Report line numbers can be stale — locate each finding now by symbol/content, not by line number. Then: already fixed by a later commit → don't re-fix, leave it unmarked (it just won't be re-reported); no longer applicable → mark WONTFIX with a reason; still live → fix it. Never trust the report's line numbers blindly.

2. Fix the still-live findings via this repo's normal workflow (a fix branch off latest base, or direct commits per convention). Keep code fixes OUT of the review PR branch — it's report-only so it always merges cleanly. Reference the finding ID in commit messages.

3. Record defer/reject decisions IN THE REPORT FILE on the review branch (a GitHub comment is not enough — only the report is read by future runs): WONTFIX to reject, ONHOLD to defer, each with a brief reason. Anything left unmarked and still present will be re-reported next time. Commit + push these edits to the PR branch.

4. Close out with gh — the next review of this type is blocked while either the PR or issue is open, so resolve both:

  • gh pr merge <pr> --squash --delete-branch (recommended — lands the report + your markers; it's report-only so it merges into the advanced base. If GitHub says the branch is behind, gh pr update-branch <pr> first.)
  • gh issue close <issue> --comment "Fixed: <ids>. Deferred: <ids>. Rejected: <ids>. Already resolved by later commits: <ids>."

Abandoning instead? gh pr close <pr> (no merge) + close the issue — nothing is recorded, markers are lost, still-present findings come back. Only do this deliberately.

Report back: what you fixed, what you marked WONTFIX/ONHOLD (with reasons), what was already resolved by later commits, and the final PR/issue state.


Findings summary

16 issues found.

Severity Count
🟡 Medium 5
🟢 Low 11

🟡 Medium

  • 2026-06-25-1-1: expand_sockname does not check malloc result before use
  • 2026-06-25-1-2: pty_activity drops (or duplicates) live output to a slow/backpressured client
  • 2026-06-25-1-3: Scrollback ring can be overwritten while a slow client is mid-replay
  • 2026-06-25-1-7: tail -f stops showing output after a log rotation
  • 2026-06-25-1-11: Signal handlers call non-async-signal-safe functions

🟢 Low

  • 2026-06-25-1-4: Legacy single-dash dispatch only reads the second character; trailing letters are silently ignored
  • 2026-06-25-1-5: parse_size can integer-overflow on the k/m multiplier
  • 2026-06-25-1-6: Detach character with the high bit set can never match (signedness)
  • 2026-06-25-1-8: tail -f never exits when its output pipe is closed
  • 2026-06-25-1-9: On-disk log can grow to ~2× the configured cap before rotating
  • 2026-06-25-1-10: scrollback_append copies byte-by-byte instead of using memcpy
  • 2026-06-25-1-12: Duplicated "write failed" reporting block in write_buf_or_fail and write_packet_or_fail
  • 2026-06-25-1-13: "Invalid number of arguments" + "Try --help" block repeated across many command handlers
  • 2026-06-25-1-14: Scattered magic-number path buffers risk silent truncation and drift
  • 2026-06-25-1-15: config.h defines HAVE_SYS_TIME_H and HAVE_LIBUTIL that are never referenced
  • 2026-06-25-1-16: Unreachable return 0 after master_process in master_main

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions