Skip to content

Remove accidental file commit - #2378

Merged
prklm10 merged 5 commits into
masterfrom
intellistory_beta
Aug 10, 2026
Merged

Remove accidental file commit#2378
prklm10 merged 5 commits into
masterfrom
intellistory_beta

Conversation

@RaghavsBrowserStack

@RaghavsBrowserStack RaghavsBrowserStack commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Accidentally added some local files as part of bump version PR which is now failing binary generation. This PR fixes that

RaghavsBrowserStack and others added 3 commits August 10, 2026 18:34
packages/core/.test-archive-mixed and .test-archive-symlink were leftovers
from a local unit-test run that got picked up by the version-bump commit.
.test-archive-symlink/linked.json is an absolute symlink into a developer's
home directory, so it dangles on a fresh CI checkout and makes
`babel packages -d build` (build_cjs) fail with ENOENT, breaking the
Build Executables job.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RaghavsBrowserStack and others added 2 commits August 10, 2026 19:39
d302b09 deleted these on this branch, but master picked up the same
accidental add in d663543 (bump version, #2377), so merging master back in
(2405903) restored them and left PR #2378 with an empty diff.

packages/core/.test-archive-symlink/linked.json is an absolute symlink into a
developer's home directory. It dangles on a fresh CI checkout, so
`babel packages -d build` (build_cjs) fails with ENOENT and takes down the
Build Executables job.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@RaghavsBrowserStack

Copy link
Copy Markdown
Contributor Author

Claude Code PR Review

PR: #2378Head: ef71bd5Reviewers: orchestrator inline (delegation to stack:code-reviewer was declined by the author; security lens intentionally disabled in this harness)

Summary

Removes 3 test byproduct files that were accidentally committed in the version-bump commit (e0df394b), one of which was a symlink pointing at a developer-local absolute path and was breaking the Build Executables workflow on master.

Root cause verified

Run 31393378096 failed on d6635434 (master's own tip, not a PR branch) at ./scripts/executable.sh:

Error: ENOENT: no such file or directory, stat 'packages/core/.test-archive-symlink/linked.json'
npm ERR! Failed at the @ build_cjs script.

The committed symlink targeted /Users/raghavkumar/.../packages/core/.test-archive-symlink/target.json — an absolute path that does not exist on CI runners, so stat threw during build_cjs. Deleting it is the correct fix.

Scope is complete — the 2 remaining fixtures are pre-existing and intentional

master tracks 5 .test-archive* files; this PR removes 3. I verified the other two are not part of this accident:

File Introduced by Verdict
.test-archive-invalid/bad.json 543eb805 — "Add archive snapshots option and separate replay command (#2216)", 2026-05-08 Pre-existing, leave alone
.test-archive/My Snapshot-1e082840.json 543eb805 (#2216), 2026-05-08 Pre-existing, leave alone
.test-archive-mixed/notes.txt e0df394b "bump version", 2026-08-10 Correctly removed
.test-archive-symlink/target.json e0df394b, 2026-08-10 Correctly removed
.test-archive-symlink/linked.json e0df394b, 2026-08-10 Correctly removed — this is the build breaker

Both pre-existing files are regular files with no dangling target, so they do not break the executable build. Removing them is out of scope here.

No test regression

Both consuming tests create their fixtures at runtime rather than reading committed ones, so the deletions are safe:

  • packages/core/test/unit/archive.test.js:191-200mkdirSync + writeFileSync('notes.txt')
  • packages/core/test/unit/archive.test.js:202-220mkdirSync + writeFileSync('target.json') + symlinkSync(...'linked.json')

CI on ef71bd5: 46/47 checks pass, 1 pending. Critically, Build & verify executable (the job that failed on master) passes, and Test @percy/core passes in 20m39s.

Review Table

Priority Category Check Status Notes
High Security No hardcoded secrets or credentials Pass No secrets. The deleted symlink embedded a developer-local absolute path; removal eliminates that minor path disclosure.
High Security Authentication/authorization checks present N/A Deletion-only change, no auth surface.
High Security Input validation and sanitization N/A No input-handling code touched.
High Security No IDOR — resource ownership validated N/A No resource access code.
High Security No SQL injection (parameterized queries) N/A No queries.
High Correctness Logic is correct, handles edge cases Pass No source logic changed; removes the artifact causing stat ENOENT on CI.
High Correctness Error handling is explicit, no swallowed exceptions N/A No error-handling code changed.
High Correctness No race conditions or concurrency issues N/A No concurrent code.
Medium Testing New code has corresponding tests N/A No new code; the producing tests already exist.
Medium Testing Error paths and edge cases tested N/A Unchanged.
Medium Testing Existing tests still pass (no regressions) Pass Fixtures are created at runtime, not read from git. Test @percy/core passes.
Medium Performance No N+1 queries or unbounded data fetching N/A Not applicable.
Medium Performance Long-running tasks use background jobs N/A Not applicable.
Medium Quality Follows existing codebase patterns Pass Test artifacts do not belong in version control.
Medium Quality Changes are focused (single concern) Pass Exactly 3 deletions, nothing incidental.
Low Quality Meaningful names, no dead code Pass Removes dead committed artifacts.
Low Quality Comments explain why, not what N/A No comments changed.
Low Quality No unnecessary dependencies added Pass None added.

Findings

No blocking findings. Two non-blocking follow-ups, both about preventing recurrence rather than anything wrong with this PR:

  • File: .gitignore (absent entry)

  • Severity: Low

  • Reviewer: orchestrator inline

  • Issue: There is no .gitignore entry for .test-archive*. Running the @percy/core unit tests regenerates these directories in the repo root of packages/core, so they remain stageable. This has now leaked into git across four commits (543eb805, e0df394b, and the three cleanup commits fighting it: d302b09f, 14d891e1, ef71bd57). Without an ignore rule, the next git add -A after a test run re-breaks the executable build the same way.

  • Suggestion: Add to packages/core/.gitignore:

    .test-archive*/
    
  • File: packages/core/test/unit/archive.test.js:191, :202

  • Severity: Low

  • Reviewer: orchestrator inline

  • Issue: Neither test cleans up after itself — there is no afterEach removing the created directories. :207 does a defensive rmSync before creating the symlink dir (added for Windows CI EEXIST), which confirms the leftover-state problem is already known, but nothing removes the artifacts once the test finishes. Note also that :210-213 deliberately uses path.resolve, producing an absolute symlink target — harmless for a runtime-created file, but that is exactly what made the committed copy fatal on CI.

  • Suggestion: Add cleanup to the describe block so the artifacts never outlive the run:

    afterEach(() => {
      for (let dir of ['.test-archive', '.test-archive-invalid', '.test-archive-mixed', '.test-archive-symlink']) {
        fs.rmSync(dir, { recursive: true, force: true });
      }
    });

Verdict: PASS

@prklm10
prklm10 merged commit c87cb81 into master Aug 10, 2026
48 checks passed
@prklm10
prklm10 deleted the intellistory_beta branch August 10, 2026 15:05
RaghavsBrowserStack added a commit that referenced this pull request Aug 11, 2026
* bump version

* remove stray archive test fixtures committed by mistake

packages/core/.test-archive-mixed and .test-archive-symlink were leftovers
from a local unit-test run that got picked up by the version-bump commit.
.test-archive-symlink/linked.json is an absolute symlink into a developer's
home directory, so it dangles on a fresh CI checkout and makes
`babel packages -d build` (build_cjs) fail with ENOENT, breaking the
Build Executables job.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix unwanted commits

* remove stray archive test fixtures reintroduced by the master merge

d302b09 deleted these on this branch, but master picked up the same
accidental add in d663543 (bump version, #2377), so merging master back in
(2405903) restored them and left PR #2378 with an empty diff.

packages/core/.test-archive-symlink/linked.json is an absolute symlink into a
developer's home directory. It dangles on a fresh CI checkout, so
`babel packages -d build` (build_cjs) fails with ENOENT and takes down the
Build Executables job.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* regenerate yarn.lock so `lerna publish` sees a clean tree

The lockfile block added in 8311242 (IntelliStory, #2339) was edited by hand
rather than regenerated, leaving it inconsistent with the dependency graph:

  - @types/node@* had no entry, though @types/cacheable-request, @types/keyv,
    @types/responselike and @types/yauzl all require it
  - get-stream@^5.1.0 had no entry, though cacheable-request@^7.0.2 requires it
  - @types/yauzl@^2.9.1 was orphaned — nothing referenced it

So `yarn` on a clean CI checkout resolved the two missing descriptors, pruned
the orphan and rewrote the file. That left the working tree dirty, and
`lerna publish from-package` aborted with EUNCOMMIT (M yarn.lock), failing the
Release job.

Regenerated with a plain `yarn install`. `yarn install --frozen-lockfile` is
now a no-op, so the release checkout stays clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

2 participants