Skip to content

Review remediation: collapse duplications + harden soft edges #11

Description

@andreiRS

From the three-lens deep review (review-remediation). Theme: collapse duplications + harden soft edges. Land after the data-integrity work (the other four issues).

Problem

Logic that defines cross-command contracts is duplicated and will diverge: HEAD-sha lookup is triplicated, the deps_in/deps_out projection is copy-pasted, new reinlines the mutating preamble, the effort/attendance enums are redeclared in 3-4 files, and the editor-runner spawn is duplicated across new/edit. Plus two soft edges flagged but downgraded by verification (robustness, not security boundaries).

Slices

11. Single source of truth for the HEAD sha (finding #13)

Delete the duplicated private readHeadSha in export and summary; route both through the existing exported gitCapture(dir, ["rev-parse", "HEAD"]).

  • export and summary produce identical HEAD-sha behavior (including failure message) via gitCapture.
  • Both private readHeadSha definitions are gone.

12. Extract the shared dependency projection (finding #18)

Extract the copy-pasted deps_in/deps_out projection from show and next into one helper (e.g. resolveDeps(task, allTasks) in queries).

  • show and next emit byte-identical deps_in/deps_out JSON as before, via the shared helper.

13. Extract a shared editor runner (finding #20)

Pull the duplicated $EDITOR/$VISUAL resolution, NO_EDITOR check, and spawn into one module used by both new and edit.

  • new and edit share one editor-resolution + spawn module.
  • NO_EDITOR behavior (code and message) is unchanged.

14. Spawn the editor as argv, not via sh -c (finding #10) — blocked by #13

Tokenize the $EDITOR value and spawn it as argv with no shell, instead of sh -c "$EDITOR \"$1\"".

  • A multi-word $EDITOR (e.g. code --wait) still opens the file correctly.
  • The editor is spawned as argv; no sh -c remains in the editor path.

15. Import the shared effort/attendance enums (finding #17)

Replace locally redeclared VALID_EFFORT/VALID_ATTENDANCE consts with imports of the existing EFFORT_VALUES/ATTENDANCE_VALUES.

  • list/set/next/new use the imported constants; no local redeclarations remain.
  • Enum-validation behavior is unchanged.

16. Shared positional-argument helper (finding #16)

Add one positionals(rest, { valueFlags }) helper and use it across commands that hand-roll extraction. Note: the originally-reported mv misparse does not actually occur (the column allowlist catches it), so this is a consistency/reuse cleanup, not a bug fix.

  • One shared helper replaces the per-command ad-hoc positional extraction.
  • Behavior is unchanged for valid inputs; unknown trailing flags handled consistently.

17. Shared short-id/uuid resolution helper (finding #21)

Consolidate the duplicated "is this a short id or a uuid, resolve it" logic (in new's --deps resolution and show's archive fallback) into one helper.

  • new and show share one short-id-or-uuid resolution path.
  • UNKNOWN_UUID behavior on an unresolvable ref is unchanged.

18. Cap the --body - stdin read (finding #12)

Read the --body - stdin stream with a size cap, rejecting (or truncating with a clear error) beyond a sane limit instead of buffering unbounded.

  • An oversized --body - stream is rejected with a clear, documented error code rather than buffering unbounded.
  • Normal-sized bodies are unaffected.

Method

TDD outside-in at the CLI boundary. One commit per green (green: / refactor:). Most slices can start independently; #14 is blocked by #13.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestreview-remediationConformance fixes from the three-lens deep review

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions