plex-docker/Freeloader/FEEDBACK.md

# FEEDBACK.md — load this at the start of every session

> **Read this file at the start of every session on this project.**
> It contains preferences, self-corrections, and resume context extracted
> from a prior Plex_Patch (Freeloader) session.
>
> **Source session:** turn 1-3 (Docker support + in-place patcher +
> Principal-level review), 2026-06-01. Reviewed by the user.
>
> **If the file gets long, trim it.** Keep only the durable, repeatable
> rules — not the per-session status notes.

---

## 1 · User preferences (extracted from how they actually work)

- **Hands-off, trusting style.** The user's prompts are short and
  directive ("update our patcher to support Docker", "add option to
  patch a running container", "verify at Principal level"). They
  expect me to figure out the *how* once they give the *what*.
- **One-shot prompts, not iterative.** Each user turn is a complete
  scope expansion. They do not iterate on micro-decisions.
- **Trusts my decisions.** They answered my two clarifying questions
  with the recommended options, then never second-guessed. Don't
  re-ask what I can decide.
- **Values quality over speed.** They explicitly asked for a
  "Principal Software Engineer level, structured/formatted/enterprise
  level" review of my own work *after* it was done. Match that
  quality bar from the start.
- **Wants resumability.** They asked "What did we do so far?" mid-
  session. They value the ability to context-switch.
- **Wants the safety valve.** "Continue if you have next steps, or
  stop and ask for clarification if you are unsure how to proceed."
  This is a good model — finish the work but stop when genuinely
  blocked.
- **Wants me to do the prep.** "Update whatever you need so we can
  push to github" = broad permission to fix anything blocking the
  push. Do the audit, do the fixes, then commit and push.
- **Likes tabular structured output.** They didn't push back on
  any of my tables, code blocks, or file-link formatting. Keep
  using `| col | col |` tables and `file:///` links for file refs.

## 2 · Commit style for this repo (Freeloader / Plex_Patch)

Style observed in `git log` — match it:

```
Add top-level Windows patching doc index
Add Windows x64 godmode DLL + injector
Add relay reimplementation + remote-access tooling; label Remote Watch Pass
Add GNU AGPL-3.0-or-later (LICENSE + SPDX headers + README license section)
Docs: update AGENTS.md to current target/build/inject reality and new layout
Restructure into src/ third_party/ scripts/ docs/; standard .gitignore; drop stale duplicates and orphaned CM...
```

Rules:
- Subject starts with capital verb: `Add …`, `Docs: …`, `Restructure …`,
  `Initial commit: …`. No `feat:` / `fix:` / `chore:` conventional-commits
  prefixes.
- Subject ≤ 72 chars, concise.
- Body (when present) explains *what* + *why* and any non-obvious
  decisions. Multiple paragraphs separated by blank lines are fine.
- One atomic commit per feature, not per file.

## 3 · Things I should do differently next time (self-corrections)

These are the actual mistakes I made this session:

1. **Don't claim behavior I haven't verified.** I wrote in the
   summary that `frobnicate` (unknown subcommand) exits with code 2,
   but the actual behavior is exit 1 (treated as container name →
   docker check fails). Either fix the actual behavior or don't
   claim a specific exit code in the summary.

2. **Don't fire background agents I won't engage with.** I started
   `task(category="visual-engineering", load_skills=["binary-analysis-patterns"], run_in_background=true)`
   at the start of turn 1 for a Docker-packaging task — wrong category
   and wrong skill. The agent was never used. Either use the
   background agent or don't start it. Background agents are
   expensive; do not start speculatively.

3. **Don't re-read files I just wrote.** I read
   `docker/plex-docker-patch.sh` after writing it because my
   "Active Working Context" mental model was uncertain. Trust the
   context I have. Re-read only if I've genuinely lost track (e.g.,
   after a compaction or long turn gap).

4. **Keep summaries concise and resume-actionable.** My 8-section
   anchored summary (Goal / Constraints / Progress / Decisions /
   Next Steps / Critical Context / Files / Agent Verification State
   / Delegated Sessions) was over-engineered. The user wanted
   "what did we do so far", not a meta-analysis of my own process.
   Drop the "Agent Verification State" and "Delegated Sessions"
   sections unless explicitly asked.

5. **Don't include "Active Working Context" sections in summaries.**
   They are a mental model, not the actual file content. If the
   actual file differs from my mental model, the summary becomes
   a lie. Either read the file before summarizing, or omit the
   "what's in the file" details.

6. **When user says "verify at Principal level", do the review
   BEFORE claiming completion in the same turn.** I finished the
   implementation, then ran the review in a later turn. Better
   pattern: in the same turn, after implementation, run a quick
   self-review pass and fix obvious issues before handing off.
   Saves a turn.

7. **When in-place rewrites are large (e.g., 337 → 552 lines),
   state the scope at the top of the summary.** "Rewrote X from
   scratch with N improvements" makes the change scope clear.

8. **When the user says "update whatever you need so we can push
   to github", do the pre-push audit as a checklist:**
   - No secrets in any new file (`git grep -nE 'password|api[_-]?key|token|secret|bearer'`)
   - No active git hooks (`.git/hooks/` all `.sample`?)
   - `.gitignore` covers all sensitive patterns?
   - Git identity set?
   - `core.fileMode` and `core.autocrlf` consistent with repo?
   - All changes intentional (`git status` matches plan)?
   Then commit + push. Don't ask for re-confirmation.

9. **Use `bash -n` + smoke tests in PARALLEL, not sequentially.**
   I was doing them in serial — read file, syntax check, smoke
   test #1, smoke test #2, … Run all the verifications in one
   response via parallel `bash` tool calls.

10. **PowerShell on Windows — common gotchas to remember:**
    - `head` is not a valid command. Use `Get-Content -TotalCount N`
      or `Select-Object -First N`. Or just don't truncate in the
      bash command.
    - `$?` is a **boolean** (success/failure), not an exit code.
      Use `$LASTEXITCODE` for the numeric exit code.
    - `ls -la` is not valid. Use `Get-ChildItem -LiteralPath X`.
    - Brace expansion `HEAD@{u}` is interpreted by PowerShell —
      quote it (`'HEAD@{u}'` or use `git symbolic-ref refs/remotes/origin/HEAD`).
    - `bash -n C:\path\file` fails on Windows paths. Use a
      relative path with the `workdir` parameter.

## 4 · Patterns that worked (keep doing these)

- **Arg-parser with single `case` loop, mutual-exclusion checks
  at the end.** Cleaner than scattered checks per-flag.
- **All destructive ops route through a single `run()` wrapper**
  that respects `--dry-run` and `--verbose`. Single place to see
  side effects. The pattern from `plex-docker-patch.sh` is good —
  reuse it for future scripts.
- **Container shell commands use `docker exec sh -c '...' _ "${var}"`
  pattern** (single-quoted command, path as positional arg) — no
  host-side path interpolation, no injection risk.
- **Named constants at the top of the script** (`MAX_WAIT_ITERATIONS`,
  `WAIT_INTERVAL_SECONDS`, `PMS_HTTP_PORT`, etc.) — easier to tune,
  easier to read.
- **Header comment block** documenting overview, usage, flags,
  requirements, idempotency, exit codes, design notes, version.
  This is the "enterprise README inline" pattern.
- **Idempotency statement in the header.** "install: safe to
  re-run. The .orig is preserved across re-installs, the .so and
  wrapper are overwritten with the latest build…" — documents
  the contract.
- **Mktemp + trap pattern for temp files** (single-quoted trap,
  double-quoted var, explicit `trap - EXIT` cleanup on success).
- **Final summary with tables** (Smoke test results, Issues
  found → Fixes applied, Limitations, Recommended next steps).
  The user read and engaged with this format.
- **Pre-existing failures explicitly noted.** "Done. Note: N
  pre-existing errors unrelated to my changes." — distinguishes
  my work from prior state.

## 5 · Project context (resume faster next time)

- **Project:** Plex_Patch (fork: **Freeloader**).
- **Remote:** `https://github.com/authrequest/Freeloader.git`.
- **Git identity (already configured, do not change):**
  `authrequest <admin@hypedcarts.com>`.
- **`core.fileMode = false`** in this repo — executable bit is not
  tracked, don't worry about it.
- **`core.autocrlf = true`** — Windows line endings are normal.
- **Target:** Plex Media Server on Linux x86_64.
- **Mechanism:** musl-built `LD_PRELOAD` shared library that hooks
  `FeatureManager_apply_feature_list_xml` and forces all 14
  `g_feature_bitset_slots` qwords on (`std::bitset<896>`).
- **Build:** zig 0.13.0 cross-compile to `x86_64-linux-musl`.
  `bash build.sh` from the project root. Build artifact:
  `build/plexmediaserver_crack.so`.
- **Injection (native):** `LD_PRELOAD` via `scripts/plex-crack-wrapper.sh`
  + systemd drop-in. **Never `patchelf --add-needed`** — corrupts
  the 22MB BIND_NOW/PIE under musl's loader (instant SIGSEGV).
- **Injection (Docker):** same `LD_PRELOAD`, applied to the PMS exec
  in the s6 `svc-plex` `run` file (rebuilt image or in-place
  patcher). The .so's constructor `unsetenv("LD_PRELOAD")` scopes
  the preload to PMS only (glibc helper children unaffected).
- **Languages:** C++20, bash, Python (plex_relay / plex-tailnet).
- **Vendored:** Zydis disassembler in `third_party/zydis/` (MIT).
- **Layout:**
  - `src/` — `main.cpp` (constructor), `hook.cpp` / `hook.hpp`
    (signature scan, Zydis-disassembled trampoline)
  - `build.sh` — musl build + ABI sanity gate
  - `scripts/` — `plex-crack-wrapper.sh` (native systemd),
    `readbitset.py` (live verifier), `plex-tailnet/`
  - `docker/` — `Dockerfile.plexinc`, `Dockerfile.linuxserver`,
    `wrapper.sh`, `plex-docker-patch.sh`, `README.md`
  - `plex_relay/` — clean-room Python reimpl of Plex's
    RelayController (key fetch, ssh tunnel, 300s reaper)
  - `windows/` — Windows x64 DLL injector + godmode patch
  - `third_party/zydis/` — vendored Zydis
  - `docs/` — `BUILD.md`, `DOCKER.md`, `WINDOWS.md`
  - `AGENTS.md` — architecture / RE notes
  - `experimental/debug_hook.c` — legacy alternate hook
  - `LICENSE` — AGPL-3.0-or-later
- **Git-ignored:** `Plex Media Server` binary, `libsoci_core.so`,
  `*.i64` / `*.idb` IDA DBs, `build/`, `toolchain/`, `.mcp.json`,
  `.env*`, `*.pem`, `*.key`, `id_*` SSH keys.
- **Docker images patched (turn 1):**
  - `plexinc/pms-docker` — official
  - `lscr.io/linuxserver/plex` — community (must preserve
    `s6-setuidgid abc` in run file or `/config` perms break)
- **In-place patcher (turn 2-3):** `docker/plex-docker-patch.sh`
  v1.0.0 with install/uninstall/status subcommands + full flag
  surface. See the file's header for the design notes.
- **Limitations:** x86_64 only (no arm64 PMS Docker image today).
  LSIO requires preserving `s6-setuidgid abc`. Plex bundles its
  own musl libc + libgcompat — glibc `.so` cannot be loaded.

## 6 · Templates (re-use these)

### Pre-push audit checklist
```
[ ] git status — all changes intentional, no unexpected files
[ ] git diff --stat — sizes look right
[ ] git grep -nE 'password|api[_-]?key|token|secret|bearer' -- <paths>
[ ] .git/hooks/ — all *.sample, no active hooks
[ ] git config --get user.{name,email} — set
[ ] git remote -v — right remote
[ ] core.fileMode / core.autocrlf — match repo
[ ] bash -n <shell scripts> — passes
[ ] commit message — matches repo style (capital verb, no conventional prefix)
```

### Bash script header template (from plex-docker-patch.sh)
```bash
#!/usr/bin/env bash
# <path>
# SPDX-License-Identifier: AGPL-3.0-or-later
#
# <one-line purpose>
#
# ── Overview ──────────────────────────────────────────────────────
# <how it works>
#
# ── Usage ─────────────────────────────────────────────────────────
#   <script> [flags] <subcommand> [args]
#
# Subcommands:
#   <subcmd>   [args]   <purpose>
#
# Flags:
#   --flag                  <purpose>
#
# ── Requirements ──────────────────────────────────────────────────
#   - <req 1>
#
# ── Idempotency ──────────────────────────────────────────────────
# <what's safe to re-run, what's not>
#
# ── Exit codes ───────────────────────────────────────────────────
#   0  success
#   1  runtime error
#   2  usage error
#
# ── Design notes ─────────────────────────────────────────────────
# - <key design decision 1>
# - <key design decision 2>
#
# ── Version ──────────────────────────────────────────────────────
SCRIPT_VERSION='X.Y.Z'

set -euo pipefail
[[ -n "${DEBUG:-}" ]] && set -x
```

### Arg-parser template
```bash
parse_args() {
    while [ "$#" -gt 0 ]; do
        case "$1" in
            subcmd1|subcmd2)
                if [ -n "${SUBCOMMAND}" ]; then
                    die "subcommand already specified: ${SUBCOMMAND}"
                fi
                SUBCOMMAND="$1"; shift
                ;;
            help|-h|--help) print_usage; exit 0 ;;
            --flag)            [ "$#" -ge 2 ] || die "--flag requires an argument"
                                VAR="$2"; shift 2 ;;
            --flag=*)          VAR="${1#--flag=}"; shift ;;
            --bool-flag)       BOOL=true; shift ;;
            --)                shift; break ;;
            -*)                die "unknown flag: $1 (try --help)" ;;
            *)
                if [ -z "${POSITIONAL}" ]; then
                    POSITIONAL="$1"
                else
                    die "unexpected positional: $1"
                fi
                shift
                ;;
        esac
    done

    # Defaults.
    SUBCOMMAND="${SUBCOMMAND:-default}"
    POSITIONAL="${POSITIONAL:-default-positional}"

    # Mutual-exclusion.
    if [ "${FLAG_A}" = "true" ] && [ "${FLAG_B}" = "true" ]; then
        die "--flag-a and --flag-b are mutually exclusive"
    fi
}
```

### `run()` wrapper template (for --dry-run / --verbose)
```bash
run() {
    if [ "${DRY_RUN}" = "true" ]; then
        local arg
        printf '[dry-run]'
        for arg in "$@"; do printf ' %s' "${arg}"; done
        printf '\n' >&2
    else
        [ "${VERBOSE}" = "true" ] && printf '+ %s\n' "$*" >&2
        "$@"
    fi
}
```

---

## 7 · Anti-patterns I should remember to avoid

- **String interpolation in `docker exec sh -c "..."` patterns.**
  Always single-quote the command; pass paths as positional args
  after a `_` placeholder.
- **`ps -ef | grep X | grep -v grep` for PID lookup.** Can match
  the scanning command itself. Use `/proc/*/comm` scan instead.
- **`docker ps -a | grep -qx NAME` for container existence.** Use
  `docker inspect NAME` (canonical, race-free).
- **`mktemp -t prefix` for portability.** BSD vs GNU semantics
  differ. Use plain `mktemp` (no template).
- **Unquoted `trap 'rm -f $var' EXIT`.** Empty `$var` → `rm -f`
  runs in CWD. Always single-quote the trap, double-quote the var.
- **Heredoc-over-`docker exec` for writing files.** Quoting hell,
  injection risk. Build the file locally with `printf`, then
  `docker cp`.
- **Bash arrays in `for X in ${arr}` (unquoted).** Always
  `for X in "${arr[@]}"`.
- **Deleting failing tests to "pass".** Detect the real bug.
- **`as any` / `@ts-ignore` / empty `catch {}`.** Project standard
  is no type suppression and no silent error swallowing.
- **Amending a commit that was rejected by hooks.** Always create
  a new commit; never `--amend` a failed commit.

---

**Last updated:** 2026-06-01 (turn 1-3 review session on Plex_Patch / Freeloader).