Skip to content

v2.0.1: auto-detected buglog entries name no file, and a substring match files false bugs #73

Description

@spignataro

v2.0.1 — auto-detected buglog entries name no file, and a substring match files false bugs

Observed on v2.0.1 (latest published), in dist/src/hooks/post-write.js. All four
points below are platform-independent and verified against the published tarball.

Companion to #68, which covers four different defects in the same file. No overlap.

This matters more than cosmetics because the OpenWolf protocol tells the agent to
read buglog.json before fixing anything. Entries that name no file, or that
describe a bug that was never there, are read as prior art by the next session.
In one project the auto-detected entries reached roughly 69% of the file.


1. The filename is never passed to detectFixPattern, so every template that wants it is broken

detectFixPattern(oldStr, newStr, ext) is defined at line 311 and called at line 272.
The relative path is already in scope — it is computed at line 270, two lines
above the call:

const relFile = normalizePath(path.relative(projectRoot, absolutePath));  // 270
// Detect what kind of fix this is
const detection = detectFixPattern(oldStr, newStr, ext);                  // 272

Every summary template that wants to say which file has to work around this, and
both workarounds are broken (points 2 and 3).

Fix: detectFixPattern(oldStr, newStr, ext, path.basename(relFile)), one new
argument at the single call site.

2. The null-safety summary always names an empty file

Line 331, verbatim:

summary: `Null/undefined access in ${path.basename(path.basename(""))}`,

path.basename("") returns "", and nesting the call does not change that:

$ node -e "const p=require('path');console.log(JSON.stringify(p.basename(p.basename(''))))"
""

So every null-safety entry ever written by this hook reads:

Null/undefined access in

Fix: with point 1 applied, Null/undefined access in ${basename}.

3. The error-handling detector is a substring match, so English prose triggers it

Line 315:

if (newStr.includes("catch") && !oldStr.includes("catch")) {

Adding the comment // we cannot catch that here files a bug report:

$ node -e "
const o='// note', n='// we cannot catch that here';
console.log('substring:', n.includes('catch') && !o.includes('catch'));
console.log('token:    ', /\bcatch\s*\(/.test(n) && !/\bcatch\s*\(/.test(o));"
substring: true
token:     false

Line 168 has the same shape (newStr.includes("try") && newStr.includes("catch")).

Fix: require a real token — \bcatch\s*\( for Java/JS/C#, \bexcept\b.*: for
Python.

Related: line 316 falls back to the literal string "unknown" when no function name
is found, producing summaries like "Missing error handling in unknown". With the
filename threaded through, the basename is a better fallback than "unknown".

4. Test files should be skipped by the error-handling and guard-clause rules

A test that adds catch, except, or assertThatThrownBy is asserting on errors,
not fixing missing error handling. One observed false entry came from an *IT.java
integration test.

Fix: skip these two rules when the basename matches /(Test|IT|Spec)\.\w+$/
(and the usual _test.py / .test.ts / _spec.rb conventions).


Suggested regression tests

  1. detectFixPattern for the null-safety category returns a summary containing the
    basename it was given — this fails today for any input.
  2. A diff whose only change is a comment containing the word "catch" produces no
    detection.
  3. No summary produced by any rule contains the literal "unknown" or ends with a
    dangling preposition.

Notes

Happy to send a PR for points 1-3 if that is useful — they are small and the call
site is single. Point 4 is a judgement call about scope, so I would rather agree the
rule before writing it.

Verified against npm pack openwolf@2.0.1. Line numbers are from
dist/src/hooks/post-write.js in that tarball.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions