Skip to content

Umbrella: openwrt/luci#8992 second review — resolve unwrap blocker + lock abort #242

Description

@lucas-albers-lz4

Goal

One umbrella for the second openwrt-ai review round on
openwrt/luci#8992 (tip 2defca544d,
cut from fwlive fix/wave3-rules-map-followups / #238).

Verdict: all 4 new inline findings are valid. One is a hard blocker for an
advertised feature; one is a hard correctness bug on the WAN-log lock path; two
are nits (one UI, one cut hygiene). The reviewer's commit-message note on
05a223d2 is also valid as history hygiene.

Blocks #209. This umbrella is facts only — evidence and sizing.
Fixes land in this repo, then re-cut into the luci PR (same path as #216#227/#228).

Prior round status (acknowledged fixed)

The reviewer confirms the first-round functional blockers are fixed on this tip:
rules map via redirect (not pipeline), BusyBox nslookup resolve backend,
poll_clamp_lines, single-awk classifier, and the stat -ctest -O +
find -perm swap (#232). Re-checked locally against the same tip; agreed.

How this was verified

Every new finding was re-checked by executing the shipped code (or the
exact rpc.js expect-unwrap path) under dash and BusyBox ash, not by
reading alone. The resolve unwrap was also simulated against luci-base
rpc.js expect handling (ret = ret[key] then type fallback).

Children

Issue Finding Severity Effort
#243 Client double-unwraps resolve — hostnames never display Blocker S · ~30–60 min
#244 acquire_wan_log_lock exec 9> aborts shell; success path silences stderr Blocker S · ~45–90 min
#245 rules reply error never reaches the UI Nit / Medium S · ~45–75 min
#246 Cut Makefile leaves double blank after dropping SOURCE_DATE_EPOCH Nit S · ~15–30 min

Plus umbrella-only process note: rewrite / replace inaccurate intermediate luci
commit message 05a223d2 on the next force-push/recut (claims pot refresh +
wave1 fixes it did not contain).

Total ≈ 2.5–4.5 h.

Severity vs. the upstream review

Agree with the reviewer: the resolve double-unwrap is the hard feature blocker
(backend now works; client discards the map). The lock finding is at least as
serious as they rated — under rpcd a failed open kills the plugin mid-call
(no lock_failed JSON); on the success path stderr is permanently discarded
for the rest of the process. The rules-error nit is real and reachable (~256
UCI rules trip the 512-key budget). The Makefile blank is cut-script residue
only (feed Makefile still has the block; upstream-cut.sh deletes it).

Where fixes land

Same as #216: edit openwrt-feed/luci-app-fwlive/ here (view JS, logging.sh,
and/or scripts/upstream-cut.sh), stack on #238 tip until that merges, then
re-cut into lucas-albers-lz4/luci luci-app-fwlive-add for #8992.

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

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions