Skip to content

Integrate the reviewed catalog adapter alongside the existing BD contract - #90

Merged
flowcool merged 2 commits into
playwright-nextfrom
feature/playwright-bd-boundary-84
Nov 1, 2026
Merged

flowcool merged 2 commits into
playwright-nextfrom
feature/playwright-bd-boundary-84

Conversation

@flowcool

@flowcool flowcool commented Nov 1, 2026 •

Copy link
Copy Markdown
Owner

Behavior

Integrate the reviewed PR #89 boundary into playwright-next without breaking catalog v1.0.0 or the Infra adoption plan.

existing catalog: locator -> bourse_direct -> existing fork collector
new catalog: native login -> bourse_direct_adapter -> fixed reviewed catalog code
  C3 at load/save/run start; source frozen before resources
  bounded authenticated reads -> native original/notice tracking

The explicit new action avoids an automatic fallback or an in-place contract change. It contains no provider selectors or parser. The existing provider implementation, invocation modes, pending state, receipts and strict delivery stay byte-identical. The new action uses the ordinary native tracking path and has no invocation overrides. A task cannot combine the source actions.

Cross-project ownership checked in Beads: infra-zlth.52 owns provider YAML/adapter qualification; infra-1lr1.3/.13 own deployment; infra-ybqb.4 owns importer reconciliation. This PR implements only infra-zlth.84. AI-assisted implementation. The catalog is trusted application code, not a Python sandbox.

Verification

  • Before: PR feat: add the validated Bourse Direct adapter boundary #89 is reviewed on public baseline 4a9bffc, but is absent from the actual fork branch. Replacing the existing bourse_direct contract would invalidate the installed v1.0.0 catalog.
  • After: explicit bourse_direct_adapter passes C3 load/save/run-start checks and actual synthetic execution; missing final native session proof, mixed contracts, unsupported chains and drift fail closed. Legacy recipes validate without any new catalog file.

Exact source candidate: ecec703ed94bc4e0955e399887d0847eec893422.

PLAYWRIGHT_BROWSERS_PATH=/data/cache/flow/ms-playwright \
BC_BROWSER_DIR=/data/cache/flow/ms-playwright \
<isolated-python> -m pytest tests/test_bc_bourse_direct_boundary.py \
  tests/test_bc_method_validation.py tests/test_bourse_direct.py \
  tests/test_notice_identity.py tests/test_throttle.py tests/test_bc_login.py \
  tests/test_bc_calendar.py tests/test_bc_collection.py tests/test_bc_runner.py \
  -q -x -o asyncio_default_fixture_loop_scope=function

Observed: 300 passed, 1 skipped in 447.73s; the final added native-session rejection scenario also passes separately (1 passed in 8.88s). Pinned Ruff (including full-tree F821), Bandit high severity and pip-audit pass. Pullfrog findings were fixed in ecec703: root-owned non-writable catalog installs are accepted; insufficient month budgets fail at run start before display/vault/browser/login; the Infra schema hash and reserved-action docs match the candidate. Final focused C3/boundary proof: 88 passed in 26.03s. The first candidate image passed 84 synthetic tests plus headless/headed Chromium, daemon startup and all 7 batch expectations / exactly 8 PDFs. Final image billcollector-adapter84:ecec703 (sha256:bd85a8f8d6b2bd5254836cca688695d197eb0c04e60f005dad0d3ef06047c66d) passes the versioned image qualifier: 88 passed in 188.81s, packaged source hashes match ecec703, headless/headed Chromium and NiceGUI startup pass, all 7 batch expectations match with exactly 8 PDFs, exit 0, network none, UID1003:GID1000. Proof container billcollector-adapter84-proof-ecec703 is stopped and retained. This is synthetic engine proof; no actual provider candidate is qualified. Integration instructions and naming difference from PR #89: doc/BOURSE_DIRECT_ADAPTER.md. The versioned image qualifier now exercises both contract paths and verifies packaged boundary hashes. No broker, vault, production or private originals are used. Provider qualification, image adoption and deployment remain with their owners.

Infra handoff

N/A: no deployment or stable release. Existing Infra adoption pins and operation plans stay unchanged. Recipes owner infra-zlth.52 will receive the exact integrated engine revision and its supported action name.

AI assistance

Codex assisted implementation and verification. The author compared the complete fork mechanisms, preserved existing provider/delivery source bytes, reviewed the semantic diff, and ran isolated browser and packaged-image proof.

Pullfrog review

Require a completed final-head review and required CI. Initial review: #90 (review). Both inline threads are fixed and resolved; final review of ecec703: #90 (review), no new issues. No unresolved threads. Ignore CodeRabbit and the known sticky pullfrog-approval status only.

Merge Danger

Door: two-way. Revert the focused integration commit. No database migration or historical data conversion.

Blast Radius: engine. Adds an opt-in C3 action and fixed trusted catalog boundary. No task is switched automatically. The additive schema must be installed with a future qualified recipe candidate. Default-branch image automation may publish the merged artifact; this PR does not tag a stable release or deploy it.

@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Important

  • Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The recipe schema changes, but the Infra runtime contract still publishes the previous schema SHA-256 as "this candidate's schema". This needs fixing before merge. The other notes are minor.

Reviewed changes

This review covers the single commit that adds the opt-in bourse_direct_adapter catalog boundary next to the unchanged legacy bourse_direct contract.

  • New fixed catalog boundary: bourse_direct_boundary.py reads a literal CHAINS and collect(api, options) from RECIPE_DIR/bourse_direct_adapter.py without executing it. It C3-validates every chain and freezes the source at run start. At collection time it execs that frozen source with a bounded Acquisition capability object.
  • Gate integration: validate_method_recipe now calls prepare(), so load, save and run start all enforce the following rules:
    • one standalone top-level step
    • exact notice identity
    • exact options
    • a preceding native login
    • no legacy bourse_direct in the same task
  • Runner dispatch: _run_task freezes the candidate before display, vault or browser access. Login proof sets ctx.native_login_proven. The action goes through the ordinary save_download/track_download path with notice identity and no delivery policy.
  • Action-name registration: the new name is added to every hand-maintained collection/reserved list, the schema enums, a new bourseDirectAdapterMethod definition and a top-level if/then.
  • Packaging, docs and tests: the Dockerfile adds a chmod go-w. This commit also adds doc/BOURSE_DIRECT_ADAPTER.md, a RECIPE_METHODS.md section, a README bullet, a CHANGELOG entry and a synthetic-portal browser suite, which the image qualifier also runs.

⚠️ The Infra contract still publishes the old schema hash

This PR changes apps/recipes_playwright/recipe-pw-schema.yaml. Its SHA-256 goes from 1d3b1114… on playwright-next to 023dd141… at 41a25bf. Both values were recomputed during this review. doc/INFRA_RUNTIME_CONTRACT.md#L95-L100 still names the old value as "This candidate's schema". AGENTS.md makes that contract the source of truth for handoffs. Infra's rollout compares the packaged and mounted hashes. After merge, the published hash would no longer match the packaged schema, and default-branch image automation may publish that artifact.

Technical details
# Stale schema SHA-256 in the Infra runtime contract

## Affected sites
- doc/INFRA_RUNTIME_CONTRACT.md:99 — still `1d3b111472c6fdfaab9860fe1ee28d6c255105c69de8281902eefcef631e70f2` (base schema)
- apps/recipes_playwright/recipe-pw-schema.yaml — head hash is `023dd141a90db23d1d6d9f182d40fe9347f7901ef0fb582f9cfa8de6a6c50e75`

## Required outcome
- The contract states the SHA-256 of the schema shipped at the final PR head.
- The CHANGELOG entry says that the recipe schema changed additively and must be promoted (previous entries call out "No ... recipe-schema" changes explicitly).

## Suggested approach
- After the last schema edit, run `sha256sum apps/recipes_playwright/recipe-pw-schema.yaml` and update line 99.

ℹ️ The reserved-name counts in the docs are stale

doc/RECIPE_METHODS.md#L86-L89 still says "Six method names are reserved" and lists six. doc/RECIPE_AUTHORING.md#L54 says "five reserved names". _RESERVED now has eight names: bourse_direct was already missing from these docs, and this PR adds bourse_direct_adapter. Section 6 of RECIPE_METHODS.md says that adding a reserved method must update this document.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using claude-opus-5-5 | 𝕏

Comment thread apps/billcollector/runner.py Resolved

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found. Every finding from the previous review is addressed in ecec703.

Reviewed changes

This review covers ecec703, the follow-up commit after the initial review of 41a25bf.

  • Schema hash corrected: doc/INFRA_RUNTIME_CONTRACT.md now publishes 023dd141a90d…. I recomputed the hash of recipe-pw-schema.yaml at head and it matches. The CHANGELOG now says the additive schema must be compared and explicitly promoted by Infra.
  • Root-owned catalog accepted: candidate() now accepts st_uid values of 0 or the runtime UID, and the error text and BOURSE_DIRECT_ADAPTER.md match. A parametrized test covers root (accepted) and a foreign UID (rejected).
  • Month budget checked before resources: prepare(recipe, *, dates=None) now runs calendar._months when _run_task passes the run-start dates, and maps CollectionError to ValueError. That dates object is the same one later assigned to ctx.bcs.date_context; collection_dates does not change it for adapter-only recipes. Load and save still call prepare(recipe) without dates, as before. A new test confirms that a 5-month window with max_months=3 returns 1 before the display, vault or browser is touched and creates no Run row.
  • Reserved-name docs updated: RECIPE_METHODS.md now lists all eight reserved names and links the adapter contract. RECIPE_AUTHORING.md no longer gives a count.

Pullfrog  | View workflow run | Using claude-opus-5-5 | 𝕏

@flowcool
flowcool merged commit dd663c8 into playwright-next Nov 1, 2026
12 checks passed
@flowcool
flowcool deleted the feature/playwright-bd-boundary-84 branch November 1, 2026 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant