Repository navigation
chore: session hygiene, OWUI deployed-login smoke, password-leak guard - #559
Merged
Merged
Conversation
…leak guard Merges cross-session .wolf memory (anatomy, buglog, decisions, session hooks, memory log) that had been sitting uncommitted while five other PRs landed on main. Union-merges buglog.json entries from both sides (no entries lost) and renumbers this session's D-010 through D-019 decisions to D-015 through D-024 to resolve a numbering collision with PR #475's D-010 through D-014, already on main; no decision content changed, only the ID prefix on the colliding half. Adds the deployed-origin OIDC sign-in smoke spec for Open WebUI (apps/web-console/e2e/phase-19/owui/deployed-login.spec.ts) plus its companion Playwright project config, both real work from an earlier session that verifies sign-in against a live deployed origin without mutating it. Scrubs the password field immediately after submit in that spec so Playwright's error-context ARIA snapshot cannot serialize a live test password if the following visibility check times out while still on the login form. Addresses the specific leak vector in #554 for this spec; the broader CI-wide fix (scoping what the workflow uploads as an artifact) is out of scope here and stays tracked on that issue. Also carries forward a settings.json change adding SessionStart and PreToolUse(Agent) hook nudges for the wenyan-ultra subagent-dispatch convention, matching the equivalent hook already in fundmoreai. Root-level scratch screenshots and a stale local Playwright HTML report from prior sessions were moved out of the repo to the session scratchpad rather than committed or deleted.
…is commit OpenWolf's hooks are shared across the checkout and recorded another agent's live edit in a sibling worktree between the previous commit and this one. Captured here so the shared .wolf state stays in sync.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Same as the prior commit: another agent's live edits in a sibling worktree landed in the shared .wolf hook state while this PR was being opened.
…-2026-07-27 # Conflicts: # .wolf/buglog.json # .wolf/decisions.md # apps/web-console/e2e/phase-19/owui/deployed-login.spec.ts # apps/web-console/e2e/phase-19/owui/playwright.owui.config.ts
sakibsadmanshajib
added a commit
that referenced
this pull request
Jul 28, 2026
Closes #553. ## The defect `ci.yml` filtered on the trigger (`paths: [apps/**, packages/**, deploy/**, supabase/**, go.work*, .github/workflows/ci.yml]`). GitHub skips a whole workflow when a trigger path filter does not match, and a skipped workflow never creates its check runs, so every required status check stayed Pending and blocked the merge. The workaround was `ci-noop.yml`: a second workflow, also named `CI`, firing on the inverse `paths-ignore` set, publishing job names that matched `ci.yml`'s exactly (including the matrix-expanded `Go tests (<module>)` names) as `run: echo "Skipped (docs-only change)"`. GitHub runs a `paths-ignore` workflow whenever **any** changed file falls outside the ignore set. It does not care whether the same pull request also touches real code. So a pull request touching both code and docs fired **both** workflows, and each required context received two check runs: one real, one a two-second echo. Either satisfies branch protection. This repository's visual-proof rule commits screenshots under `docs/` in the same commit as the code they prove, so that mixed shape is the common case, not an edge case. This is not theoretical. PR #559, merged onto `main` twenty minutes before this branch was cut, touched `apps/web-console/e2e/**` plus `.wolf/**`. Both workflows fired on commit `d05b7ba1`: | Required context | `ci.yml` run [30301329356](https://github.com/sakibsadmanshajib/hive/actions/runs/30301329356) | `ci-noop.yml` run [30301329536](https://github.com/sakibsadmanshajib/hive/actions/runs/30301329536) | |---|---|---| | `Go tests (agent-engine)` | success, 49s | success, **3s** | | `Go tests (control-plane)` | success, 2m13s | success, **4s** | | `Go tests (edge-api)` | success, 1m49s | success, **4s** | | `Go tests (storage)` | success, 42s | success, **3s** | | `Repo policy lints (tenant + audit)` | success, 13s | success, **2s** | | `Web console (type + unit + build)` | success, 2m38s | success, **3s** | Both runs are literally named `CI`; only the `path` field of the run object distinguishes them. The echo run reported success on every required context in under five seconds. That is why `gh pr checks` had stopped being a usable merge signal. ## Approach Path filtering moves off the trigger and into the workflow, so `ci.yml` becomes the only producer of every required check name. - No `paths:` or `paths-ignore:` on any trigger. The workflow always starts, so it can never leave a required check Pending. - A leading `changes` job asks the GitHub API for the same changed-file list GitHub itself would have filtered on (`pulls/{n}/files` for pull requests, `compare/{before}...{after}` for pushes) and emits one boolean. - Polarity is inverted relative to the old filter. `changes` holds an allow-list of paths that cannot affect any build, test or lint (`*.md`, `LICENSE`, `NOTICE`, `docs/`, `.wolf/`, `.claude/`, `.vscode/`, `.cursor/`). Anything else means run, including an unrecognized path, an API error, or an event that carries no diff such as `schedule` and `workflow_dispatch`. A false "run" costs CI minutes; a false "skip" would defeat the gate, so the default is never "skip". - Jobs that publish a **required** check name (`go-tests`, `repo-policy-lints`, `web-unit`) carry `if: always()` and gate their individual **steps**. They always run to completion and always report their own conclusion. On a docs-only change they execute nothing and conclude `success` in a few seconds. - Jobs that are **not** required keep a plain job-level `if:` and are simply skipped, because a skipped non-required check cannot block anything. - The step condition is `!= 'false'`, not `== 'true'`, so a `changes` job that failed or was cancelled leaves an empty output and the real suite runs anyway. - `web-e2e-shim` is deleted. It duplicated the `Web E2E (full stack)` name held by `web-e2e`, which was the last remaining pair of jobs sharing a check name, and it was dead weight anyway: that context is not in `required_status_checks.contexts`. - `.github/ci/lint-workflow-check-names.mjs` runs inside the `Repo policy lints (tenant + audit)` required check and fails the build on (1) two pull-request jobs publishing the same check name, (2) a required context in `.github/branch-protection-main.json` that no job publishes, (3) a job publishing a required context without `if: always()`. This is the regression guard: reintroducing anything shaped like `ci-noop.yml` now fails CI. There is no longer any ordering dependence. Two workflows cannot both report a required name because only one workflow declares those names, and a re-run cannot rescue a failure because the same job either does the work or does not, decided by a single boolean. ### Rejected alternatives **Keep two workflows, make the path sets provably disjoint** (the "smallest fix" sketched in #553, for example an explicit docs allow-list on `ci-noop.yml`). Rejected: it leaves two producers for every required check name, so correctness rests on the two globs staying complementary forever. Any future path added to one set and not the other silently reopens the hole, and nothing fails when it does. **`dorny/paths-filter` for the detector.** Rejected: `gh api` plus a `case` statement is about twenty lines, needs no checkout, has no merge-base or clone-depth pitfalls, and adds no third-party action to the one code path that guards every merge. **Job-level `if:` on the required jobs, relying on a skipped job satisfying the gate.** This is what GitHub documents ("A job is skipped by a conditional" gives "The job reports Success", [troubleshooting required status checks](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/troubleshooting-required-status-checks)) and it was the first version of this branch. Rejected after building it: whether branch protection accepts a `skipped` conclusion for a required context cannot be verified on a pull request into a protected branch until the change is already on `main`, community reports contradict the docs, and this repository's own `web-e2e-shim` comment asserted the opposite. Guessing wrong deadlocks every docs-only pull request. `if: always()` plus step gating gives a job that concludes `success` on its own merits, which is unambiguous on every GitHub version. The guard now enforces that shape. **Merge queue (`merge_group`).** Not adopted. This repository does not use a merge queue, and `strict` is `false` deliberately. Adding one would be a separate change; note for later that any workflow feeding a merge queue must also list the `merge_group` trigger or the queue deadlocks. **Drop path filtering entirely and always run the full suite.** Simplest of all and fully correct, but it costs roughly four minutes of runner time on every documentation-only change. Kept the skip because the requirement was explicitly that a docs-only pull request go green without running the full suite. ## Verification Every case was exercised on real pull requests against this repository, not reasoned about. A pull request into `main` must carry `.github/workflows/ci.yml` itself in order to be evaluated by the new logic, and a workflow file is not a docs-only path. The `run=false` branch is therefore unreachable on a pull request into `main` until this is merged. To close that gap without weakening the allow-list, a throwaway harness (`probe/ci553-base`) carried a workflow whose `changes` job was copied **verbatim** out of this branch's `ci.yml` (sha256 `0e81807ca11ca5408e24c1ff1f91689f60d5d0ebf4d2e70ba8fabda3b2163b3a`, 76 lines) plus one job with the exact shape of a required job (`if: always()` and per-step gating). Pull requests into that base have clean single-purpose diffs, so both verdicts are reachable against the real decision logic. ### Case 1 — documentation only PR #567, run [30303490144](https://github.com/sakibsadmanshajib/hive/actions/runs/30303490144), diff = `docs/verification/ci-553-harness.md`. | Job | Conclusion | Duration | Evidence | |---|---|---|---| | `Detect changed paths` | success | 2s | `verdict: SKIP — every changed path is docs-only`, `run=false` | | `PROBE required-shaped job` | **success** | 4s | both real steps report conclusion `skipped`; the job itself concluded `success` | The required-shaped job is not a skipped job. It ran, executed nothing, and reported success, which is what makes the gate independent of skipped-check semantics. ### Case 2 — code only PR #563, run [30303342319](https://github.com/sakibsadmanshajib/hive/actions/runs/30303342319), head `32077382`. | Required context | Conclusion | Duration | |---|---|---| | `Detect changed paths` | success (`run=true`) | 3s | | `Go tests (agent-engine)` | success | 52s | | `Go tests (control-plane)` | success | 2m9s | | `Go tests (edge-api)` | success | 1m53s | | `Go tests (storage)` | success | 55s | | `Repo policy lints (tenant + audit)` | success | 16s | | `Web console (type + unit + build)` | success | 2m29s | ### Case 3 — code plus documentation, the case that was broken PR #564, run [30303353621](https://github.com/sakibsadmanshajib/hive/actions/runs/30303353621), head `98f44d76`, diff includes `docs/verification/ci-553-probe.md` alongside code. | Required context | Conclusion | Duration | |---|---|---| | `Detect changed paths` | success (`run=true`) | 5s | | `Go tests (agent-engine)` | success | 56s | | `Go tests (control-plane)` | success | 2m17s | | `Go tests (edge-api)` | success | 1m46s | | `Go tests (storage)` | success | 51s | | `Repo policy lints (tenant + audit)` | success | 17s | | `Web console (type + unit + build)` | success | 2m46s | Exactly one `CI` run per commit, where the same shape previously produced two. The docs half of the diff no longer buys the code half a two-second pass. The harness control for this case is PR #568, run [30303499232](https://github.com/sakibsadmanshajib/hive/actions/runs/30303499232): diff = `apps/edge-api/docs/swagger.go` plus `docs/verification/ci-553-harness.md`, detector 2s, `verdict: RUN the real suite — 'apps/edge-api/docs/swagger.go' is outside the docs-only allow-list`, and the required-shaped job executed its real step (all steps `success`, 7s). Same workflow, same commit shape, opposite verdict from Case 1, decided only by the file list. ### Case 4 — mixed pull request whose code deliberately fails PR #565, run [30303363079](https://github.com/sakibsadmanshajib/hive/actions/runs/30303363079), head `6f628256`. A `t.Fatal` test was added under `apps/edge-api/docs/` alongside a documentation change. | Required context | Check runs on the commit | Conclusion | Duration | |---|---|---|---| | `Go tests (edge-api)` | **1** | **failure** | 1m43s | | `Go tests (agent-engine)` | 1 | success | 51s | | `Go tests (control-plane)` | 1 | success | 2m33s | | `Go tests (storage)` | 1 | success | 53s | | `Repo policy lints (tenant + audit)` | 1 | success | 15s | | `Web console (type + unit + build)` | 1 | success | 2m36s | `GET /commits/6f628256/check-runs` returns exactly one check run per required context, all from run `30303363079` (`.github/workflows/ci.yml`). There is no second producer to rescue the failure, and re-running the workflow re-runs the same failing job. `mergeStateStatus` for #565 is **`BLOCKED`**. For contrast, #561 and #563 with no failing required check report `UNSTABLE`, so `BLOCKED` here is the merge gate refusing the pull request, not an artifact of draft status. ### Case 5 — this pull request This is itself a mixed pull request: `.github/workflows/**` and `.github/ci/**` alongside `.github/MERGE-POLICY.md` and `.wolf/buglog.json`. Two of those four paths are on the docs allow-list, and the verdict is still `run=true` because of the other two, which is the whole point of the polarity. | Head | Run | Detector | Required contexts | |---|---|---|---| | `07f08280` | [30303332435](https://github.com/sakibsadmanshajib/hive/actions/runs/30303332435) | success, 6s, `run=true` | all 6 pass: lints 12s, storage 47s, agent-engine 53s, edge-api 1m47s, control-plane 2m16s, web console 2m23s | | `ac6c37fa` (head) | [30303827155](https://github.com/sakibsadmanshajib/hive/actions/runs/30303827155) | success, 5s, `run=true` | all 6 pass: lints 17s, storage 52s, agent-engine 1m3s, edge-api 1m47s, control-plane 1m54s, web console 2m38s | The `Repo policy lints` leg includes the new merge-gate guard, so the guard is green against the tree it guards. ### Guard, both directions, locally - Current tree: `Merge-gate integrity OK: 15 check names across 5 pull-request workflows, 6 required contexts each with exactly one producer.` - With `ci-noop.yml` restored as a second workflow: fails, naming all four duplicated check names and both producing jobs. - With `web-unit` reverted to a plain job-level `if:`: fails with `A required job must use if: always() and gate its steps`. ## Branch protection **No change is required.** All six required check names are unchanged, and `.github/branch-protection-main.json` is untouched: ``` Go tests (agent-engine) Go tests (control-plane) Go tests (edge-api) Go tests (storage) Repo policy lints (tenant + audit) Web console (type + unit + build) ``` Confirmed against the live gate (`gh api repos/sakibsadmanshajib/hive/branches/main/protection`) before and after: same six contexts, same `app_id` 15368, `strict: false`, `enforce_admins: true`, `required_conversation_resolution: true`. Nothing to apply. One thing worth knowing for later: `Web E2E (full stack)` is **not** in the live required list, which is why deleting `web-e2e-shim` cannot deadlock anything. If it is ever promoted to required, `web-e2e` needs the same `if: always()` plus step-gating treatment the other required jobs now have, and the guard will say so. ## Cleanup All throwaway probe branches and pull requests (#562, #563, #564, #565, #567, #568, `probe/ci553-*`) are closed and deleted. `docs/verification/ci-553-*.md`, `apps/edge-api/docs/ci553_probe_test.go` and the harness workflow existed only on those branches and never touched this one. ## Files - `.github/workflows/ci.yml` — triggers unfiltered, `changes` job added, required jobs `if: always()` with gated steps, non-required jobs job-level gated, `web-e2e-shim` removed. - `.github/workflows/ci-noop.yml` — deleted. - `.github/ci/lint-workflow-check-names.mjs` — new merge-gate integrity guard. - `.github/MERGE-POLICY.md` — documents the in-workflow filter, why a trigger filter cannot be used here, and the one-producer-per-check-name rule. - `.wolf/buglog.json` — appended entry `ci-noop-echo-satisfies-required-checks`. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **CI & Reliability** - Improved pull request checks so required validations report clear results even when changes affect documentation-only or non-executable files. - Updated test and integration workflows to run selectively based on the files changed. - Removed legacy no-op and check-shim workflows. - **Documentation** - Clarified branch protection, workflow filtering, required checks, and how to apply configuration updates. - **Validation** - Added automated safeguards to detect missing, duplicate, or incorrectly configured required checks. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
1 participant
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.
Summary
Lands uncommitted cross-session work that was blocking the shared checkout from fast-forwarding to main:
.wolf/memory merge..wolf/buglog.jsonand.wolf/decisions.mdhad both diverged from main (five PRs, fix: resolve console redirect origins from the canonical host, not the 0.0.0.0 bind address #481 to fix: enforce per-tenant model entitlement on the inference path #484 plus chore: log console-hive Caddy Host-mismatch bug in buglog #464/fix: slash every zero the console sets in a dense numeric column #471, appended buglog entries; PR chore: record D-012/D-013/D-014 sandbox and design decisions in wolf ledger #475 appended decisions D-010 to D-014). Union-mergedbuglog.jsonso every entry from both sides survives (135 total, zero lost, validated as JSON).decisions.mdhad a real numbering collision: this session's local D-010 to D-019 covered different decisions than main's already-shipped D-010 to D-014. Kept main's D-010 to D-014 verbatim and renumbered only this session's colliding half to D-015 to D-024 (content, source, and date untouched, only the ID prefix changed). Grepped the repo first to confirm nothing else references the old numbers.apps/web-console/e2e/phase-19/owui/deployed-login.spec.tsplus its Playwright project inplaywright.owui.config.ts. Logs in against a live deployed OWUI origin over the real cross-origin OIDC consent hop, without installing storageState or thehive_jwt_forwardFunction (both of those mutate whatever they point at, which is not acceptable against a live deployment). Self-skips whenOWUI_URLis loopback or credentials are unset.error-context.md, attached on any failing web-first assertion) serializes live input values, including a password still sitting in a filled field. Verified against the current Playwright internals (v1.58.2) that there is no config flag to disable this capture, and that the snapshot is taken synchronously at the moment the assertion's retry loop times out, before test code regains control, so the only effective mitigation is to make sure the password is not in the DOM by the time a later assertion could fail. Added a scrub (passwordBox.fill("")) immediately after the login submit and before the next assertion in this spec. This closes the concrete leak vector for this one spec; the broader fix from Playwright ARIA snapshots serialize input values; CI uploads the report where a test password can land #554 (scoping what the CI workflow uploads as an artifact) is a.github/workflows/ci.ymlchange and stays out of scope here, tracked on that issue..claude/settings.json. Kept. Adds SessionStart andPreToolUse(Agent)hook nudges enforcing the wenyan-ultra subagent-dispatch convention, mirroring the equivalent hook already shipped in fundmoreai. Confirmed valid JSON and that the referencedemit-nudge.jshook script exists..pngfiles plus a stale localplaywright-report-owui/HTML report (both leftover manual-verification artifacts from earlier sessions, not gitignored, not referenced by any tracked doc) were moved out of the repo to the session scratchpad rather than committed or deleted, since deleting screenshots is not reversible and neither belongs at the repo root.Test plan
.wolf/buglog.jsonvalidated as parseable JSON (135 bugs, union of origin's 130 plus this session's 5 new entries).wolf/decisions.mdrenumbering verified againstgit logon both branches; grepped the repo for any code/doc references to the old D-010 to D-019 numbers (none found)apps/web-console/e2e/phase-19/owui/deployed-login.spec.tsreviewed against its config; the loopback-skip comment matches the config's own skip conditionowui-deployed-loginPlaywright project against a real deployedOWUI_URLwithOWUI_E2E_EMAIL/OWUI_E2E_PASSWORDset, confirm it reaches the OWUI new-chat screen and that no password value appears in the resultingerror-context.mdon an induced failureci-noop.yml, per issue ci-noop.yml publishes identically-named required checks and can fire alongside the real ci.yml #553) before mergeNo UI-touching change lands here (memory/config/test-tooling only), so no screenshot proof is attached per the visual-proof gate.