Skip to content

nightly: fix tracker stripping, export spawn errors, release version gap - #312

Merged
conorbronsdon merged 3 commits into
mainfrom
nightly/fix-20260914
Sep 14, 2026
Merged

conorbronsdon merged 3 commits into
mainfrom
nightly/fix-20260914

Conversation

@conorbronsdon

Copy link
Copy Markdown
Owner

Nightly review of commits 6663722..aa4da8b (Sep 12–14, 83 non-merge commits). npm test was 20/20 green on origin/main before these fixes and is still 20/20 after them.

Fixes (each reproduced on origin/main first)

  1. detector/validate.js: a tracker before bold markers, a dash or an ellipsis is reported as an altered URL (regression from fix: preserve query params when stripping trackers #305). The bare-URL extractor keeps trailing **, —it, and . fix: preserve query params when stripping trackers #305 only peeled off [.,;:!?], so See **https://ex.com/p?utm_source=chatgpt.com** nowSee **https://ex.com/p** now now fails with url-missing. Before fix: preserve query params when stripping trackers #305 it passed. The fix also treats emphasis markers as punctuation, and treats a dash or ellipsis plus the text after it as suffix, but only when what remains of the last field is an exact tracker. Controls: ?ref=home—it → removed still errors, and ?utm_source=chatgpt.com.au—x still errors. Three cases added to the fix: preserve query params when stripping trackers #305 test. Generated copies re-synced.
  2. scripts/rewrite-eval-opencode.js:577: a failed export launch hides the real error (Add an explicit audited OpenCode evaluation runner #297). When opencode export fails to start, stderr is undefined, so the assert message itself throws Cannot read properties of undefined (reading 'trim'). failure.json then records that message with exit_status: 0. The call now goes through requireCommand, like the other commands.
  3. detector-test.yml: add a release version check to PR CI (gap from ci: harden release and review boundaries #306). Since ci: harden release and review boundaries #306 the release workflow triggers only on package.json. No PR check compared package.json with CHANGELOG, so a release PR that bumps SKILL.md, manifests and CHANGELOG but forgets package.json merges green and never releases. The fix adds node scripts/verify-release-versions.js to the detector job, which runs on every PR. It passes on current main (3.35.0).

Needs a maintainer decision (not changed here)

These findings change corpus segmentation, and so the measurement provenance. All three reproduced:

Also noted:

  • rewrite-eval-opencode.js can't launch an npm opencode.cmd shim on Windows (spawnSync EINVAL). It needs the native .exe path, and neither the code nor the README says so.
  • TITLE_CASE_HEADER no longer matches a mid-title I (## What I Learned And Why It Matters).

Automated nightly review of 2026-09-14's commits. Draft for Conor's review.

🤖 Generated with Claude Code

- validate: #305 stopped stripping an AI tracker that sits directly before
  bold markers, an em/en dash, or an ellipsis, reporting url-missing
  (e.g. **https://x.com/p?utm_source=chatgpt.com** -> **https://x.com/p**).
- rewrite-eval-opencode: an `opencode export` spawn failure threw
  "Cannot read properties of undefined (reading 'trim')" and hid the real
  error; route it through requireCommand like the other calls.
- detector CI: since #306 releases trigger only on package.json changes, so
  a release PR that forgets package.json merged green and never released.
  Run verify-release-versions.js on every PR.

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

@conorbronsdon conorbronsdon left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

ChatGPT/Codex review of ff15a57804d373f04ef1d9e7b75a6ef99c3c5931: ready to land.

I checked the full diff and the surrounding URL normalization, command-error handling, and release guard. All three focused suites pass locally (validator, OpenCode runner with fake clients, release versions), and the release guard confirms package/changelog agreement. All seven CI checks pass on this head. No provider calls were needed.

The tracker change still requires an exact recognized final tracker after separating punctuation; non-tracking query fields remain protected. Export launch failures now use the existing error-preserving command helper. The release-version check runs in the existing PR test job. Generated validator copies agree.

Thanks for the reproduced fixes and for separating the corpus/provenance findings from this patch. Those deferred findings should receive their own measured follow-up; they are not blockers for these fixes.

@conorbronsdon
conorbronsdon marked this pull request as ready for review September 14, 2026 16:41
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Fix tracker suffix handling, export errors, and release version CI

🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Preserve adjacent formatting and prose when stripping terminal AI-referrer parameters.
• Surface OpenCode export spawn failures without masking their original errors.
• Validate package and changelog version agreement on every pull request.
Diagram

graph TD
  CI["PR CI"] -->|"runs"| VERIFY["Version check"] -->|"compares"| META["Package + changelog"]
  TESTS["Validator tests"] -->|"covers"| VALIDATOR["URL validator"] -->|"syncs"| COPIES["Distributed copies"]
  EVAL["Eval runner"] -->|"exports session"| CLI["OpenCode CLI"]
Loading
High-Level Assessment

The targeted approach fits the existing architecture: extend suffix normalization without replacing the established URL extractor, reuse requireCommand instead of duplicating subprocess checks, and add version verification to the existing branch-protected job. A broader URL-parser refactor or separate CI job would add risk or require branch-protection changes without improving these focused fixes.

Files changed (8) +25 / -6

Bug fix (5) +17 / -5
validate.jsPreserve formatting and prose after terminal trackers +4/-1

Preserve formatting and prose after terminal trackers

• Extends terminal suffix recognition to emphasis markers, pipes, dashes, and ellipses with attached prose. A suffix is detached only when the remaining final query field exactly matches an AI tracker.

detector/validate.js

validate.jsSynchronize plugin validator suffix handling +4/-1

Synchronize plugin validator suffix handling

• Propagates the corrected tracker suffix normalization into the avoid-ai-writing plugin’s packaged validator copy.

plugins/avoid-ai-writing/skills/avoid-ai-writing/detector/validate.js

rewrite-eval-opencode.jsPreserve OpenCode export spawn errors +1/-1

Preserve OpenCode export spawn errors

• Routes session export results through requireCommand so launch failures report the underlying spawn error instead of dereferencing undefined stderr.

scripts/rewrite-eval-opencode.js

validate.jsSynchronize skill validator suffix handling +4/-1

Synchronize skill validator suffix handling

• Propagates the corrected tracker suffix normalization into the standalone avoid-ai-writing skill copy.

skills/avoid-ai-writing/detector/validate.js

validate.jsSynchronize preservation verifier URL normalization +4/-1

Synchronize preservation verifier URL normalization

• Propagates the corrected tracker suffix normalization into the preservation verifier’s validator copy.

skills/preservation-verifier/scripts/validate.js

Tests (1) +3 / -0
validate.test.jsCover tracker removal beside Markdown and punctuation +3/-0

Cover tracker removal beside Markdown and punctuation

• Adds regression cases for bold-wrapped URLs, em-dash-attached prose, and ellipses while retaining non-tracking query parameters.

detector/validate.test.js

Documentation (1) +1 / -1
CHANGELOG.mdDocument corrected tracker suffix handling +1/-1

Document corrected tracker suffix handling

• Expands the unreleased tracker-preservation entry to cover bold markers, dashes, and ellipses adjacent to removed tracker parameters.

CHANGELOG.md

Other (1) +4 / -0
detector-test.ymlEnforce release version agreement in pull-request CI +4/-0

Enforce release version agreement in pull-request CI

• Runs verify-release-versions.js in the existing detector job so mismatched package.json and CHANGELOG.md versions cannot merge unnoticed.

.github/workflows/detector-test.yml

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 14, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Users cannot find the export fix ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The requireCommand change corrects the error shown when opencode export cannot start, but the
Unreleased changelog only documents the tracker-stripping fix. Anyone troubleshooting a failed
rewrite evaluation therefore has no version-history record that upgrading restores the underlying
spawn error.
Code

scripts/rewrite-eval-opencode.js[577]

+    requireCommand(exported, `${task.id}: opencode export failed`);
Evidence
Compliance rule 1 requires every user-facing change to have an Unreleased changelog entry. The
changed export path now preserves the real launch failure, while the current Unreleased section
describes only tracker handling and unrelated earlier work.

CLAUDE.md: Maintain Changelog Entries for User-Facing Changes: CLAUDE.md: Maintain Changelog Entries for User-Facing Changes: CLAUDE.md: Maintain Changelog Entries for User-Facing Changes: CLAUDE.md: Maintain Changelog Entries for User-Facing Changes
scripts/rewrite-eval-opencode.js[577-577]
CHANGELOG.md[7-12]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The user-visible correction to failed `opencode export` error reporting is missing from the Unreleased changelog.
## Fix Focus Areas
- CHANGELOG.md[9-12]
## Recommended Fix
Add a concise bullet under the Unreleased `Fixed` section explaining that export launch failures now report the underlying spawn error instead of a secondary `stderr.trim()` exception.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Query parameter changes pass validation ✓ Resolved 🐞 Bug ≡ Correctness
Description
normalizeUrl treats any dash or ellipsis and all remaining query text as prose once the preceding
value resembles an exact tracker. When a URL contains data such as
?utm_source=chatgpt.com—foo&keep=1, rewriting it as —foo&keep=1 passes validation even though
the query structure and functional parameter were altered.
Code

detector/validate.js[93]

+      const punctuation = query.match(/(?:[–—…].*|[.,;:!?*_~|]+)$/)?.[0] || '';
Evidence
The URL extractors retain Unicode dashes, ellipses, ampersands, and equals signs, after which every
extracted URL is normalized before multiset comparison. The changed expression can therefore move
—foo&keep=1 into suffix, remove the apparent tracker, and produce the same normalized string as
an altered rewrite, contrary to the documented requirement to leave functional query parameters
unchanged.

detector/validate.js[42-43]
detector/validate.js[87-109]
detector/validate.js[261-272]
SKILL.full.md[451-453]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The expanded punctuation regex can classify valid query data after a dash or ellipsis as prose, allowing functional query changes to pass preservation validation.
## Fix Focus Areas
- detector/validate.js[87-100]
- detector/validate.test.js[190-205]
- skills/avoid-ai-writing/detector/validate.js[87-100]
- plugins/avoid-ai-writing/skills/avoid-ai-writing/detector/validate.js[87-100]
- skills/preservation-verifier/scripts/validate.js[87-100]
## Recommended Fix
Restrict dash and ellipsis suffix handling so candidates containing query syntax such as `&` or `=` remain part of the URL. Add regression tests proving that functional parameters after these characters cannot be removed or moved while retaining the intended prose-suffix cases, then synchronize the generated validator copies.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Plugin versions can remain stale ✓ Resolved 🐞 Bug ≡ Correctness
Description
The newly required verify-release-versions.js invocation compares only the package version with
the first released changelog heading. A release that updates those two files but omits SKILL.md or
either plugin manifest still passes PR CI despite violating the documented release procedure.
Code

.github/workflows/detector-test.yml[54]

+      - run: node scripts/verify-release-versions.js
Evidence
The added workflow step relies on a verifier that opens only CHANGELOG.md and package.json and
compares only those values. Repository guidance explicitly requires synchronized versions in
SKILL.md and both plugin manifests, each of which independently carries release metadata that the
new check does not inspect.

scripts/verify-release-versions.js[78-80]
scripts/verify-release-versions.js[103-124]
CONTRIBUTING.md[173-177]
SKILL.md[1-4]
plugins/avoid-ai-writing/.claude-plugin/plugin.json[1-4]
.codex-plugin/plugin.json[1-4]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new release-version CI step checks only package.json and CHANGELOG.md, allowing stale skill and plugin manifest versions through the release process.
## Fix Focus Areas
- .github/workflows/detector-test.yml[51-54]
- scripts/verify-release-versions.js[78-125]
- scripts/verify-release-versions.test.js[75-86]
- CONTRIBUTING.md[173-177]
## Recommended Fix
Extend the verifier to read the version from SKILL.md, the Claude plugin manifest, and the Codex plugin manifest and require every published version to equal package.json and the latest changelog release. Add fixtures covering each independently stale file so the PR check fails before release.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread scripts/rewrite-eval-opencode.js
Comment thread detector/validate.js Outdated
Comment thread .github/workflows/detector-test.yml
Address nightly review findings with query-separator regressions, independent skill/plugin version drift controls, and the export-error changelog entry.
@conorbronsdon
conorbronsdon merged commit 0e18435 into main Sep 14, 2026
7 checks passed
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