auto-fix batch 2026-04-27-122744#432
Merged
Merged
Conversation
Master PR session-open marker for resolving-issues skill batch run. Sub-PRs target this branch; merging the master PR closes all linked issues.
Add willow uid 10001 + USER directive in runtime stage of relay, replay, storage Dockerfiles. chown writable dirs (/etc/willow, /shared, /var/lib/willow) before USER switch. Web: switch nginx:alpine to nginxinc/nginx-unprivileged:alpine (uid 101, listens 8080). Compose maps host 8080 to container 8080. Refs #314 https://claude.ai/code/session_016cmtqT7yEQUgjcLgz4pARP Co-authored-by: Claude <noreply@anthropic.com>
SetPermission.permission was String. Took anything. "FrobnicateWidgets" sail past apply, land in role.permissions set. Same role.permissions also String-typed — pollution sticks. Now: SetPermission.permission: Permission. Role.permissions: BTreeSet<Permission>. Type system bars unknown names from ever entering DAG. Back-compat: custom Deserialize maps unknown legacy string-form names (JSON / MCP boundary) to hidden __UnknownLegacy sentinel. apply_event drops sentinel as no-op + tracing::warn — DAG signature stays intact, bad name dropped. Bincode wire path emits typed enum directly; pre-1.0, no production storage round-trips legacy String form, so binary cutover safe (storage SQLite holds bincode bytes, not JSON). Agent MCP tool keeps string param on wire, parses via Permission::from_name at boundary, errors on unknown. Runner-up: hard cutover (Deserialize accepts only typed enum). Rejected — a rogue / future client emitting unknown name would currently break the chain on insert; sentinel keeps DAG resilient with no security loss. Refs #240 Co-authored-by: Claude <noreply@anthropic.com>
`IndexableMessage` in `app.rs` hard-coded `has_image: false`, `has_file: false`, `letter_id: None`, so `has:image` / `has:file` operators silently underreport. New `IndexableMessage::from_display_message` derives the flags from the body: `[file:NAME:b64]` → image if NAME ext is image, else file; `http(s)://...image-ext` URL → image. Web app uses the constructor. `letter_id` stays None: active-letter signal not plumbed into the indexer effect yet — follow-up. Refs #355 https://claude.ai/code/session_016cmtqT7yEQUgjcLgz4pARP Co-authored-by: Claude <noreply@anthropic.com>
Handler closures in `crates/web/src/handlers.rs` swallowed every async error with `let _ = h.<action>(...).await`. Send/edit/delete/react/ pin/unpin failures vanished — typed message disappeared from input, no log, no toast. Refs #350 [GEN-07]. New `warn_and_toast_with(action, e, toasts)` helper: - `tracing::warn!(error = ?e, action, "ui handler failed")` - Pushes `Toast::err("Couldn't {action}. Try again.")` with a per- action `dedup` key so back-to-back failures coalesce instead of stacking. Mirrors the lazy `use_context::<ToastStack>()` pattern PR #411 introduced for the inbound mark-read button. Each handler now captures the toast stack on the synchronous frame (reactive owner intact) before `spawn_local`, then forwards into the helper from the async path — `wasm_bindgen_futures::spawn_local` strips the owner, so context lookup must happen earlier. 7 sites patched: send_message, send_reply, edit_message, delete_ message, react, pin_message, unpin_message. switch_channel / switch_server stay as-is — they return `()` so there was nothing to swallow. Browser test `handler_error_toasts::*` covers helper happy-path, dedup coalescing, and missing-stack no-panic. Test exercises the exact production code path; the handler closures themselves are pinned to ClientHandle<IrohNetwork> with no trait seam to inject a failing double, so going one level deeper would buy nothing. `rg "let _ = h\." crates/web/src/handlers.rs` returns empty. fmt + clippy + native tests + wasm check all green; browser tier verified locally via `cargo check -p willow-web --tests` + manual review of the test mod (Firefox/wasm-pack not on this box, browser suite runs in CI). Co-authored-by: Claude <noreply@anthropic.com>
Owner
Author
|
go ahead and include the skill edit in this PR. and leave a note to do this in the future |
incorporate edits suggested in master PR #432 lessons learned. - empty session-open commit step added to master pr setup - sub-pr ci gating clarified: when base != main, gh actions workflows scoped to branches: [main] don't fire. local just check is the merge gate, do not park sub-pr open waiting. - implementer scope-creep guard: > 5 files or > 200 loc returns to coordinator before pushing. - merge gate step rewritten: covers both ci-runs and no-ci paths. - coordinator-never-codes exception: master pr session-open commit + lessons-learned skill edits exempt. - lessons learned now mandates skill edits applied in same master pr, not deferred. - quality section updated to match new merge gate.
Owner
Author
|
Let's also edit the skill so we don't create a PR immediately. We just use a branch, a master branch, to accept the sub-PR changes and then open a full PR, not a draft PR, when all that work is done. Just so I don't accidentally merge before the PR is ready. |
Owner
Author
|
And another addition we can make is to let's emphasize using superpowers and brainstorm but automated so it doesn't ask for my input. It uses those skills to come up with little plans for sufficiently complex work. We should determine if additional planning is required and use those skills to properly plan a solution for the issue. |
- master pr opens at end of run, ready (not draft), so human can't accidentally merge mid-flight. branch + sub-pr merges live without a pr until end. if anything unfinished, leave branch un-pr'd. - complexity gate added to implementer step 3: trigger automated brainstorm + writing-plans when fix spans crates / touches state / has multiple approaches / non-obvious root cause / > 5 files OR > 200 loc / 'it depends' scope. skip for one-liners + clear suggested-fix issues. - brainstorm runs automated, single-actor — implementer plays both roles, never asks the human (runs are unattended). - writing-plans drops a docs/plans/ file when brainstorm yields a multi-step plan. small fixes skip the plan file. - brainstorm + plan folded into sub-pr body for review trail. - core loop step 9 codifies: skill commits land before pr open. - master pr setup split into 'branch setup (start)' + 'pr open (end)'. - 'master pr branch' references swept to 'master branch'.
intendednull
commented
Apr 27, 2026
| 1. After all sub-PRs merge, all skill edits applied, and Lessons Learned drafted, open the PR — **non-draft, ready for review**. | ||
| 2. Title: `auto-fix batch YYYY-MM-DD-HHMMSS`. Base: `main`. Apply label `auto-fix-batch`. | ||
| 3. PR body: running list of `Fixes #N` lines (one per resolved issue) + `## Skill Evolution` (if skill commits landed) + `## Lessons Learned` section. | ||
| 4. **Never open the master PR as a draft.** Open as ready or not at all. If something is unfinished, leave the branch un-PR'd and surface what's left so the human can decide. |
Owner
Author
There was a problem hiding this comment.
I would rather reopen issues and open the PR with what can be merged. If further work is needed on something, a follow-up issue should be raised. Nothing should be continued in session. I don't want to have to chase down a session to continue work; we should automatically pick it up in the next run.
reviewer feedback on PR #432: never leave a session 'to be resumed'. the issue queue is the only durable handoff. - master pr open (end of run): always open with what landed, even if some attempts hit blockers. blockers move to follow-up issues so next scheduled run picks them up. only skip pr open if zero sub-prs landed. - new 'parked' section in pr body cites follow-up issues per row. - no in-session continuation. sessions don't get resumed. - implementer step 10 rewritten: ci red / just check red / mid-fix block → file follow-up issue, close sub-pr (not park draft). follow-up is the durable handoff for next run. - sub-pr rules tail: ci red after one fix attempt → follow-up issue + close sub-pr. - autonomy section: same shift, draft sub-pr park gone. - quality section: master pr opens with whatever landed; blocked work goes to follow-up issues.
Owner
Author
|
fix conflicts |
…DvNnk # Conflicts: # crates/web/tests/browser.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
auto-fix batch run. sub-PRs target this branch. merge master PR → all linked issues auto-close.
Resolved
/etc/willowpaths in clap default_value flags #324 — platform-aware worker default paths viadirs(sub-PR chore(workers): platform-aware default paths #439)SetPermission.permissionis an unvalidatedString, not a typedPermission#240 — typedPermissiononSetPermission+Role.permissions(sub-PR fix(state): typed Permission on SetPermission #440)letter_id,has_image,has_file— operators silently underreport #355 — plumbhas_image/has_fileinto search index (sub-PR fix(web): plumb has_image/has_file into search index #442;letter_idparked as Search index: plumb active letter_id into IndexableMessage #441)Skill Evolution
Three commits land lessons + reviewer feedback into
.claude/skills/resolving-issues/SKILL.md:95d007f— initial lessons (session-open commit, sub-PR CI gating, scope-creep guard, merge gate, in-PR skill edits).37988dd— master PR opens at end of run (ready, not draft); automated complexity gate triggerssuperpowers:brainstorming+superpowers:writing-plansfor non-trivial fixes.db74ac3— always ship what landed, blockers → follow-up issues, sub-PR closed (not draft-parked). Next scheduled run picks up the follow-up issue from the queue. No in-session continuation — sessions don't get resumed; the issue queue is the only durable handoff. Master PR body gains## Parkedsection when any rows blocked.Lessons Learned
what worked:
--scopefilter early kept dispatch to obvious-fix issues. zero scope-creep flame-outs across 6 sub-PRs.cargo fmt+clippy+test substitute fine when sub-PR base ≠ main suppresses CI workflows. master PR (base=main, opened at end of run) re-runs full CI before human merge — catches anything stale. now codified.letter_id,has_image,has_file— operators silently underreport #355 small. follow-up issue link in sub-PR body = clear audit trail.what didn't:
just checkIS the gate when CI doesn't run.SetPermission.permissionis an unvalidatedString, not a typedPermission#240) widened scope to also retypeRole.permissions: BTreeSet<String>→BTreeSet<Permission>. justified (root cause) but ballooned 10 files / 299+ LOC. now codified: complexity gate (step 3) + scope-creep guard (step 6) require brainstorm checkpoint before pushing.37988dd).db74ac3). issue queue is the durable handoff, no chasing sessions.## Implementer Boilerplatesection in next iteration if it bites again.