Carry apply/rollback advisories as typed values: ApplyResult.error doubles as a warning string that three consumers decode by substring, and they disagree

Đang mở
  • Có gì không ổn?
#1,987 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức phù hợp với người mới
35/100
Loại issue
Tái cấu trúc
Độ rõ ràng
Khá rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
rust
Lĩnh vực
cli

Hướng nghiên cứu

Start in crates/socket-patch-core/src/patch/apply.rs, where ApplyResult.error gets advisories joined with "; " (join_notes) and advisory_warning_codes matches marker consts. Then read the CLI consumers in crates/socket-patch-cli/src/commands/apply.rs and rollback.rs, plus store_copies.rs fold. Done means a typed Advisory enum on the results, one mapping helper to RunWarning, and the listed tests asserting variants; the maintainer's open decision on rollback's fallback needs settling first.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

arch-audit refactor

[agent] Filed by the scheduled architecture audit (audit-core). Register: discussion #1830 register.

Kind: refactor. Register ID: A76.

Problem

ApplyResult.error (and the rollback result) means "why it failed" when success == false, but on a successful result it carries advisories instead: a --force skip note, an ownership restore the process could not make, or a failed stale-bytecode cleanup. These are joined with "; " into one string (apply.rs L121-L125, join_notes L1192). Consumers then get the warning codes back by substring-matching marker text (advisory_warning_codes L977-L989, which uses OWNERSHIP_NOT_RESTORED_MARKER and STALE_BYTECODE_NOT_REMOVED_MARKER).

The three consumers handle a note differently:

  • CLI apply (apply.rs L1411-L1419): a note that matches no marker, such as the --force skip note, produces no run warning.
  • CLI rollback (rollback.rs L2316-L2323):`` a note that matches no marker is labelled ownership_not_restored anyway.
  • Core store_copies::fold (store_copies.rs L65-L72): a successful store copy keeps its advisory only if it contains the ownership marker. A store copy's stale-bytecode advisory is dropped before the CLI sees it. This is read from the code; I did not execute it.

Refusals are matched by marker the same way, for example SHARED_STORE_REFUSAL_MARKER, so tests can only check them with .contains on a sentence (rollback.rs L3086).

Proposed change
  • Add a typed advisories: Vec<Advisory> to the apply and rollback results, with enum Advisory { ForceSkipped(..), OwnershipNotRestored(..), StaleBytecodeNotRemoved(..) }. Each variant has its warning code and display text.
  • Keep error for failures only.
  • Make store_copies::fold concatenate every copy's advisories.
  • Have both CLI commands map variants to RunWarning through one helper.
  • Delete advisory_warning_codes and the two marker consts, or make them private if a test still needs the text.
Size

About 6 files: core patch/{apply,rollback,store_copies}.rs and CLI commands/{apply,rollback}.rs, plus their tests. Estimated diff is ±250 lines.

Acceptance criteria
  • The human text and the --json warning codes stay the same for ownership and bytecode advisories.
  • Decide whether rollback's fallback to ownership_not_restored for unrecognized notes is intended. If not, delete it and add a test.
  • A store-copy stale-bytecode advisory now reaches the run warnings, with a test.
  • test_restore_file_permissions_ownership_failure_is_a_warning and the shared-store refusal tests assert variants instead of sentence substrings.
Ngôn ngữ chính
Rust
Star
9
Fork
2
Merge trung bình
18 giờ 47 phút
Pull request đã merge (30 ngày)
637

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của SocketDev/socket-patch

Tất cả issue của SocketDev/socket-patch

Issue tương tự

Thêm issue về Rust

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.