Skip to content

Skip the consent drift cases that read files this export omits - #1469

Closed
aniruddhaadak80 wants to merge 1 commit into
CodebuffAI:mainfrom
aniruddhaadak80:fix/sponsored-consent-test-public-mirror
Closed

aniruddhaadak80 wants to merge 1 commit into
CodebuffAI:mainfrom
aniruddhaadak80:fix/sponsored-consent-test-public-mirror

Conversation

@aniruddhaadak80

Copy link
Copy Markdown

Summary

common/src/ads/sponsored-consent.test.ts is permanently red on main. Eight cases in its every surface asks the same question block read two files in the Desktop Electron app, and this repository does not ship that tree, so every one of them dies with ENOENT before it asserts anything.

This skips exactly those eight cases, on the presence of the file each one reads.

Fixes #1467

Problem

The block reads:

  • freebuff-desktop/electron/mcp-consent-bridge.cjs
  • freebuff-desktop/electron/consent-window.html

freebuff-desktop/ is not one of the in-scope paths in CONTRIBUTING.md (cli/, sdk/, common/, agents/, packages/*, freebuff/ excluding the private web app, scripts/tmux/, public docs), and this repository is an export of the private source tree. The directory simply does not exist here.

The comment above the block states its purpose: the two Desktop surfaces carry their own copies of these strings, and the block "is what stops the two surfaces drifting into asking two different questions."

In the public mirror the guard is therefore not just failing, it is inert — it can never detect drift, and it fails loudly enough that anyone running the suite learns to ignore this file.

Repro on main (0cd284b)

$ bun test common/src/ads/sponsored-consent.test.ts
error: ENOENT: no such file or directory, open
'...\freebuff-desktop\electron\mcp-consent-bridge.cjs'
error: ENOENT: no such file or directory, open
'...\freebuff-desktop\electron\consent-window.html'
... (8 occurrences)

 5 pass
 8 fail

This is not a platform artifact: the path is absent from the export on every platform, so it fails identically on Linux and macOS.

Why CI never caught it

.github/workflows/ci.yml installs dependencies, builds the SDK, builds the Freebuff binary and smoke-tests the binary. It never runs bun test. A permanently red test file therefore produces a green check on every PR.

Root cause

The export strips freebuff-desktop/, but the tests that read it were exported with it. There is no presence check between the two, so a file that cannot exist by construction is asserted on unconditionally.

Changes

common/src/ads/sponsored-consent.test.ts only:

  • Import existsSync alongside readFileSync.
  • Add isInThisTree(rel), documented with why the guard exists.
  • Name the two paths once as BRIDGE / WINDOW instead of repeating the literals in six places.
  • Convert the five test.each / test(...) blocks that read those files to test.skipIf(!isInThisTree(rel)), iterating [BRIDGE, WINDOW] where both surfaces are checked.

This is the same isInThisTree + test.skipIf shape already used for the planner and desktop sources in common/src/__tests__/free-agents.test.ts, so the repo has one idiom for this rather than two.

The check is not disabled. It keeps running in the private tree, where the sources do exist, and reactivates here by itself if the export ever starts including them. the Windows sentence says the three things, in plain words needs no external file and still runs here unchanged.

Testing

Whole common/src/ads suite, 36 files, same command on both trees:

$ bun test $(git ls-files 'common/src/ads/*.test.ts')
Tree Result
main (0cd284b) 662 pass, 0 skip, 9 fail, 671 tests
This branch 662 pass, 8 skip, 1 fail, 671 tests

Same test count. The 8 ENOENT failures are gone. The 1 remaining failure is pre-existing on main and untouched by this diff:

(fail) command-scoped advertiser attribution > a command-scoped assignment reaches the child but not the next command

The single file, before and after:

# main
 5 pass
 8 fail
# this branch
 5 pass
 8 skip
 0 fail

Regression proof: the guard is not a blanket mute

I created freebuff-desktop/electron/consent-window.html with deliberately wrong content and re-ran. The four consent-window.html cases ran and failed on the content, while the still-absent mcp-consent-bridge.cjs cases stayed skipped:

(skip) ... > freebuff-desktop/electron/mcp-consent-bridge.cjs carries the same sentence, verbatim
(fail) ... > freebuff-desktop/electron/consent-window.html carries the same sentence, verbatim
(skip) ... > the bridge carries the same in-place sentence, verbatim
(fail) ... > freebuff-desktop/electron/consent-window.html carries the same Windows no-sandbox sentence, verbatim (COD-642)
(fail) ... > the desktop window says the same thing when it cannot name who is asking
(skip) ... > freebuff-desktop/electron/mcp-consent-bridge.cjs caps the name at the same length
(fail) ... > freebuff-desktop/electron/consent-window.html caps the name at the same length

So when the source is present, the drift check still fails on real drift. The scratch file was removed afterwards; the branch diff is the test file only.

Formatting

bunx prettier --check reports this file both before and after the change. I confirmed the committed blob is byte-identical to Prettier's own output (0 differing lines) and contains no CR bytes, so the warning is the local core.autocrlf=true Windows checkout, not the patch. The committed blob is LF-only, as on main. No reformat churn is included.

  • New/updated tests fail without this change
  • Full existing suite passes (unchanged pre-existing failure excepted, documented above)
  • Lint / format / typecheck pass (see Formatting note)
  • Tests read the real files they assert on

Compatibility / risk

None. Test-only, no production code path is touched. The eight cases remain registered and will run automatically wherever the sources exist. Test names are preserved (the test.each rows become template-literal names carrying the same relative path), so nothing downstream that filters on test name loses a case.

Notes for the maintainer

This is the third file in the tree with the same shape, after common/src/__tests__/free-agents.test.ts and common/src/__tests__/freebuff-public-data-use-copy.test.ts. The underlying gap is that ci.yml never runs bun test, so an export-stripped test file is indistinguishable from a passing one. A bun test step over the packages CI already installs would surface the whole class at once; I kept this PR to the one file so it stays reviewable, but happy to follow up with that separately if useful.

… omits

The `every surface asks the same question` block in
`common/src/ads/sponsored-consent.test.ts` reads two files that live in the
Desktop Electron app:

- `freebuff-desktop/electron/mcp-consent-bridge.cjs`
- `freebuff-desktop/electron/consent-window.html`

`freebuff-desktop/` is not one of the in-scope paths in CONTRIBUTING.md and the
public export does not ship it, so the directory does not exist in this tree and
all 8 cases in that block die with ENOENT before asserting anything. The block
exists to stop Desktop and the CLI asking the user two different questions, so in
this mirror the guard is not merely failing, it is inert: it can never detect
drift, and it fails loudly enough that anyone running the suite learns to ignore
this file.

Public CI never runs `bun test` (it installs, builds the SDK, builds the binary
and smoke-tests the binary), which is why a permanently red file has gone
unnoticed on main.

Guard each case on the presence of the file it reads, using the same
`isInThisTree` + `test.skipIf` shape already used for the planner and desktop
sources in `common/src/__tests__/free-agents.test.ts`. The check keeps running in
the private tree, where those sources do exist, and reactivates here by itself
if the export ever starts including them. The one case that needs no external
file, `the Windows sentence says the three things, in plain words`, still runs
here.

Fixes CodebuffAI#1467
@aniruddhaadak80

Copy link
Copy Markdown
Author

Closing this one: upstream main moved to a8c632ee2 while I had it open, and in that sync common/src/ads/sponsored-consent.test.ts was DELETED upstream, so this PR is now CONFLICTING/DIRTY and rebasing it would re-add a file the project has since removed.

I confirmed this rather than assuming:

  • git cat-file -e 5e1579a5:common/src/ads/sponsored-consent.test.ts -> exists (my base)
  • git cat-file -e upstream/main:common/src/ads/sponsored-consent.test.ts -> does not exist
  • common/src/ads/sponsored-consent.ts (the module under test) still exists upstream, so the code was kept and its test file was dropped
  • the same sync deleted 7 other common/src/ads/*.test.ts files (supabase-*.test.ts, sponsored-compute-contract, sponsored-verification, tests/supabase-backend-foundation-contract), so this looks like a deliberate pruning of ads tests rather than an accident in my favour

Re-adding a deleted test file is a product decision, not a bug fix, so I am not going to smuggle it back in under a "fix the ENOENT" title. Leaving the diagnosis in case it is useful:

The block every surface asks the same question read freebuff-desktop/electron/mcp-consent-bridge.cjs and consent-window.html, which the public export does not ship, so all 8 of its cases died with ENOENT while the pure-constant case in the same file still passed. Verified 5 pass / 8 fail on the old base, 5 pass / 8 skip / 0 fail with an existsSync + test.skipIf guard, and confirmed the guard is not a blanket mute by planting a consent-window.html with wrong content and watching the four window cases fail on it. Whole common/src/ads suite on the old base: 662 pass / 9 fail, on the branch 662 pass / 8 skip / 1 fail, identical 671-test count.

The same defect class in files that still exist is fixed by #1458 and #1459 (both still apply cleanly to current main), and the broader cause is that ci.yml never runs bun test.

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.

sponsored-consent.test.ts fails with ENOENT: it reads freebuff-desktop/ files this export omits

1 participant