Skip to content

refactor(downloads): isolate observation ownership - #234

Open
lamemustafa wants to merge 7 commits into
masterfrom
tapish-codex/observation-ownership-20260826
Open

refactor(downloads): isolate observation ownership#234
lamemustafa wants to merge 7 commits into
masterfrom
tapish-codex/observation-ownership-20260826

Conversation

@lamemustafa

@lamemustafa lamemustafa commented Aug 26, 2026

Copy link
Copy Markdown
Owner

Summary

Addresses #172 by extracting the existing live download-ownership helpers into an import-free leaf. The observer and staged-ZIP module import the leaf directly; the durable reconciler re-exports the existing helper API and shares the same singleton sets.

Draft only. Authenticated live qualification is missing, and this PR is not merge-ready or release-ready.

Root Cause / Decision Record

  • Static runtime imports formed one eight-module strongly connected component. The observer and staged-ZIP module imported small ownership helpers from the durable reconciler, pulling recovery orchestration back into their dependency graph.
  • This is a source-structure hardening change, not a reproduced production crash. Moving the three existing helper declarations and four existing sets removes both back-edges without changing guards, persistence, or sequencing. The helper declarations were verified byte-identical to the base.
  • The new TypeScript-AST guard checks static runtime cycles reachable from background modules and excludes type-only edges. It also checks that the ownership leaf has no runtime imports. This addresses the source-level cycle in Runtime import cycle cluster across src/background #172; it does not provide live qualification.

Scope

  • Runtime: one new ownership leaf; import changes in the observer, staged ZIP, and durable reconciler; preserved helper re-exports.
  • Tests: AST cycle guard, repository-relative diagnostic regression, cross-import singleton/listener proof, and affected mock imports. Existing race and exact-ID assertions retained.
  • Docs/governance: this PR body only.
  • Explicitly out of scope: new fields, permissions, dependencies, recovery states, delivery semantics, portal behavior, or unrelated refactors.

Pack Workflow Preflight

  • pnpm workflow:preflight was run before editing/push, or the skip reason is documented.
  • This PR was opened from a Pack branch, not master.
  • I checked latest master Pack AGENTS guidance or recorded the stale-guidance warning.
  • PR body keeps the required Pack privacy/review/verification checklist visible.

Base: 214aabdf77dd3884a44db62878bab4345a2da970. Preflight passed before editing and was rerun after the initial commits.

Sanchika Adoption Gate

  • If this PR consumes @sanchika/* packages or copied Sanchika guidance, I
    read sanchika/docs/adoption-pack.md in the coordinated parent worktree.
  • If this PR consumes Sanchika, it links ComplyEaze and Axal completion evidence
    and records the Sanchika commit or copied guidance used.
  • This PR does not import ../sanchika, sanchika/packages/*/src, or parent
    source paths.

The first two items are not applicable: no Sanchika consumption or copied guidance.

Privacy And Data-Flow Impact

  • No new browser permissions.
  • No new host permissions.
  • No new network calls.
  • No analytics, telemetry, ads, or session replay.
  • No credential, OTP, CAPTCHA, cookie, token, GST file, or taxpayer-data capture.
  • Public copy and privacy declarations are updated if behaviour changed.

No new capture or storage is introduced. Existing transient ownership state remains transient, and persisted checkpoints remain the recovery authority. Existing public claims are unchanged. Cycle diagnostics now use repository-relative paths.

Sensitive Surface Review

  • Current tab / portal target binding is preserved or intentionally changed.
  • Download completion remains evidence-backed and fail-closed.
  • Ambiguous side-effect delivery cannot be reported as confirmed success.
  • Service-worker durability impact is understood and documented.
  • Real taxpayer data, local paths, raw URLs/referrers, and portal HTML are absent from the diff.

The extraction preserves the single shared ownership state used by the inline observer and durable listener. Focused tests cover claim-before-search, pending-ID races, and reconciliation after inline ownership ends. Mandatory local security/privacy review is being finalized on the exact head; authenticated live qualification is not performed.

Chrome Web Store Impact

  • This PR does not expand beyond the existing Chrome Web Store V0 listing unless every gate in docs/PUBLICATION_READINESS.md is checked.
  • Full fiscal year ships in the one binary but stays out of Store-advertised scope until its evidence gates are recorded.
  • Store copy, README status, Privacy QA, and reviewer instructions were reviewed if user-facing behavior changed.
  • CI ZIP creation, provenance, and protected publishing are treated as release evidence, not manual store-submission sign-off.
  • PR title uses Conventional Commits so Release Please can bump Pack after merge.

No Store or public-copy change. No publication-readiness claim.

Verification

  • pnpm install --frozen-lockfile
  • pnpm audit --audit-level high
  • pnpm exec wxt prepare
  • pnpm exec prettier --check .
  • pnpm exec eslint . --max-warnings 0
  • pnpm exec tsc --noEmit
  • pnpm exec vitest run
  • pnpm exec wxt build
  • node scripts/verify-extension-package.mjs .output/chrome-mv3
  • pnpm exec wxt zip
  • node scripts/verify-extension-zip.mjs
  • node scripts/write-release-provenance.mjs
  • node scripts/verify-github-release-assets.mjs --tag <tag> --zip <zip> --checksum <sha256> --provenance <json> when release assets exist
  • node scripts/publish-chrome-web-store.mjs --zip .output/<zip> --provenance .output/pack-release-provenance.v1.json --publisher-id <id> --dry-run true
  • git diff --check
  • pnpm review:gate -- --strict-head-review --wait-head-review-ms 180000 before merge/readiness claim; a missing Codex review blocks readiness:

Focused verification passed: four files, 88 tests. Type/lint/format checks were rerun after the test-only privacy correction. The final-head full suite passed: 126 files, 2123 tests. Build and package verification passed with the same runtime source. Final full-suite lines:

      Tests  2123 passed (2123)
   Start at  08:56:05
   Duration  165.14s (transform 1.54s, setup 0ms, import 12.59s, tests 136.79s, environment 7ms)

Mutation proof: each old import edge was restored separately and confirmed in source, then the entire five-test graph file was run. Each mutation produced exactly one intended cycle failure and four passing neighboring tests. Diagnostics contained repository-relative cycle paths. Both mutations were reverted before final verification.

Graphify refresh was unavailable because this checkout has neither graph data nor the referenced refresh script. The AST guard supplies the local structural proof; no graph artifact is claimed.

Release-only commands were not run locally. Hosted Pack CI passed its build, package, ZIP creation, exact-ZIP verification, and checksum steps. No authenticated live run was attempted. Exact-head external review and strict review-gate clearance remain missing; the ordinary hosted review gate passed without supplying that stricter evidence.

Artifact Evidence

  • CI run: Pack CI 32926695073, successful on 11ccc05785f69e61cf7cd734fe59d24b9de472fc; ordinary review gate 32926694942 also successful. CodeQL, Conventional PR Title, and GitGuardian checks passed.
  • ZIP artifact: generated and verified inside CI; no retained downloadable artifacts returned by the run's artifacts API at the final check. None produced locally.
  • ZIP SHA-256: not independently retrieved; no checksum claim.
  • Clean source/tag or head SHA: 11ccc05785f69e61cf7cd734fe59d24b9de472fc.

PR Review Follow-Up

  • GitHub Actions completed.
  • Autogenerated Codex/bot review comments inspected after checks completed for the latest head SHA.
  • Inline review threads are resolved, outdated, or answered with evidence.
  • No commits were pushed after the last required human/bot review without re-review.
  • Any follow-up PRs or issues are listed here instead of being left implicit.
Thread/comment Disposition Commit or evidence
Local privacy review: absolute cycle diagnostics could expose checkout paths fixed-with-evidence 11ccc05785f69e61cf7cd734fe59d24b9de472fc; relative-path regression and both edge mutation runs

External review has not yet occurred. The existing #172 remains the tracking issue. Missing authenticated live qualification and required exact-head review are explicit blockers to merge/readiness.

Screenshots

Not applicable: no UI change. Tests use synthetic fixtures only.

Post-update alias-guard follow-up

  • Base was updated with gh pr update-branch after PR fix(recovery): restore initial observing summary outcomes #233 merged; this produced merge commit 2a23683 (no rebase, amend, or force-push).
  • Current head: 7ae790a2c477b0636a5dc58bd9b5329677f867f3 (test(downloads): resolve configured background aliases).
  • The graph guard now reads the existing root TypeScript config and uses ts.resolveModuleName; resolved files outside src/background/** are excluded.

Review-thread disposition register (current head)

Finding Disposition Evidence
Empty-import source-only cycle Stale with evidence 7d35fc3 already requires both importClause and at least one named binding; original thread resolved.
Empty re-export source-only cycle Stale with evidence 7d35fc3 already requires at least one named export; original thread resolved.
Configured alias imports were omitted Fixed with evidence 7ae790a uses the root tsconfig plus ts.resolveModuleName, confines results to src/background/**, and the review thread is resolved.

Verification (current head)

  • Red proof before the resolver change: tests/background/background-module-graph.test.ts had exactly 1 failed | 7 passed (8); the configured-alias cycle resolved to null.
  • Mutation proof: after confirming the original resolved-file return was absent, replacing it with return null reproduced exactly 1 failed | 7 passed (8); restoration produced 1 passed (1) and 9 passed (9), including both empty import/re-export regressions.
  • Passed: pnpm workflow:preflight, pnpm exec tsc --noEmit, pnpm exec eslint . --max-warnings 0, pnpm exec prettier --check ., pnpm exec wxt build, and node scripts/verify-extension-package.mjs .output/chrome-mv3.
  • Full Vitest output:
     Test Files  127 passed (127)
          Tests  2132 passed (2132)
       Start at  20:32:20
       Duration  152.88s (transform 1.14s, setup 0ms, import 9.09s, tests 130.11s, environment 7ms)
    
  • Immediately before that full run, the host had one peer Vitest suite active (seven matching pgrep -f [v]itest processes); this is recorded as host state only and makes no timing or stability claim.

This update is ready for a fresh exact-head review; no merge-readiness claim is made while hosted checks and that review are pending.

@lamemustafa

Copy link
Copy Markdown
Owner Author

@codex review exact head 11ccc05. Please focus on preservation of shared singleton ownership, unchanged download guards and sequencing, and the AST cycle regression tests. This PR remains draft; authenticated live qualification is missing.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 11ccc05785

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@lamemustafa
lamemustafa marked this pull request as ready for review August 26, 2026 04:04

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 48cde4b6c7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tests/background/background-module-graph.test.ts

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2a99ec489

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tests/background/background-module-graph.test.ts
Comment thread tests/background/background-module-graph.test.ts Outdated
@lamemustafa
lamemustafa force-pushed the tapish-codex/observation-ownership-20260826 branch from f95ae3f to 7d35fc3 Compare August 26, 2026 13:42
@lamemustafa

Copy link
Copy Markdown
Owner Author

@codex review Please review the current exact head 7ae790a2c477b0636a5dc58bd9b5329677f867f3. The new commit fixes configured TypeScript aliases in the background cycle guard and includes red/mutation proof plus full local verification in the PR body.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 7ae790a2c4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@lamemustafa

Copy link
Copy Markdown
Owner Author

@codex review Please review the exact current head d1bec29 after the non-rebase base update. Confirm the existing ownership-extraction guarantees remain unchanged. The PR body still records authenticated live qualification as intentionally missing; no merge-readiness claim is made.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: d1bec290f6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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