A module whose comment says it is the one place that knows this request, and the three other places that also know

Notifio is a desktop app that watches rental listing searches. It never sends email itself. It posts to the server, and the server owns templates, sending and the licence check, for reasons I went into in Our README tells you to make a Gmail app password, and nothing in the app has read one for months.

There is a module for that. It opens like this:

/**
 * Outbound notifications to the Notifio server, which owns the email sending.
 *
 * The app never talks to an SMTP provider itself. It posts to /api/notify with
 * the licence, and the server decides what to send. This module is the one place
 * that knows the shape of that request.
 *
 * Kept separate from monitor.ts so the reply engine can send its own email
 * without importing the monitor (which imports the reply engine).
 */

Two claims in there. The second is true and is a genuinely good reason for a file to exist. The first is not true.

$ grep -rn 'api/notify`' src/*.ts
src/monitor.ts:281:  const res = await fetch(`${serverUrl}/api/notify`, {
src/monitor.ts:327:    await fetch(`${serverUrl}/api/notify`, {
src/monitor.ts:386:    await fetch(`${serverUrl}/api/notify`, {
src/notify.ts:47:    const res = await fetch(`${serverUrl()}/api/notify`, {

Four callers. One in the module that says it is the only one, three in the file it was extracted out of.

The extraction worked, which is why nobody noticed

The cycle in the second paragraph of that comment is real. The import graph is:

monitor.ts  ->  reply-engine.ts  ->  notify.ts

monitor.ts runs the poll cycle and calls into the reply engine. The reply engine needs to send one email of its own, to tell the user a reply went out. If that send lived in monitor.ts, the arrow would point back and you would have reply-engine -> monitor -> reply-engine.

Worth being precise about what that would actually have cost, because "circular import" is often said as though it were automatically fatal. In CommonJS, which this app compiles to, a cycle does not throw. The second require of a partially-evaluated module returns whatever has been assigned to module.exports so far, and since every one of these is a function called much later at runtime rather than at module load, it would most likely have worked. The reason to avoid it is not that it breaks, it is that it works until somebody moves a call to module scope, and then it fails in a way whose error message points at the wrong file.

So extracting notify.ts was correct. The author moved the one function the reply engine needed, wrote down why, and stopped. The three sends already in monitor.ts had no cycle problem, so there was no pressure to move them, and the docstring went in describing the goal rather than the state.

That is the most ordinary way for a comment to become false. Not drift, not rot. It was aspirational on the day it was written.

What the four callers disagree about

The endpoint takes four message types, and the app has exactly one caller per type, which is tidier than it sounds and also how the duplication stayed invisible. Here is where they agree and where they do not.

The timeout agrees. All four pass signal: AbortSignal.timeout(15_000). That is the one line that is identical in four places, and I will take it.

The server URL does not. The extracted module goes through a helper:

// entitlement.ts
export function serverUrl(): string {
  return (process.env.NOTIFIO_SERVER_URL || 'https://notifio.app').replace(/\/$/, '');
}

The three in monitor.ts do this instead, three times:

const serverUrl = process.env.NOTIFIO_SERVER_URL || 'https://notifio.app';

Same fallback, same env var, no trailing-slash strip. So with NOTIFIO_SERVER_URL=https://notifio.app/ set, one caller requests /api/notify and the other three request //api/notify.

I expected that to be the bug in this post. It is not, and the reason is worth knowing:

$ curl -s -o /dev/null -w "%{http_code}\n" -X POST -d '{}' https://notifio.app/api/notify
400
$ curl -s -o /dev/null -w "%{http_code} -> %{redirect_url}\n" -X POST -d '{}' https://notifio.app//api/notify
308 -> https://notifio.app/api/notify

(The 400 is the endpoint correctly refusing a request with no licence in it.)

Next.js normalises the double slash with a 308, which preserves the method and the body, and fetch follows redirects by default. So the three callers without the strip work. They work by spending an extra network round trip on every alert, in an app whose entire proposition is that it tells you about a room before the forty other people hear about it. The duplication did not produce an outage. It produced a latency tax that no error log will ever mention.

Where the licence comes from does not agree either. Three callers take it from the config object the poll cycle is already holding:

const license = config.license;
if (!license?.email || !license?.token) return;

The extracted one reads the file again, every time, on purpose:

/** Licence from disk, read fresh so an activation mid-session is picked up. */
function license(): { email: string; token: string } | null {
  try {
    const config = JSON.parse(fs.readFileSync(CONFIG_PATH, 'utf8')) as Config;
    ...

Both of those are defensible and they are answers to different questions. The poll cycle loaded its config at the top of the cycle, so it is at most one cycle stale, which is about thirty seconds. A reply can be queued, run and finish much later, and in between the user may have typed their token in for the first time, so re-reading is the only way that send succeeds. The cost is a synchronous readFileSync on a path the monitor also reads every poll.

The thing I do not like is that a reader cannot tell which policy is deliberate without reading both. One of the two has a comment explaining itself. The other three just use whatever was in scope.

And the error policy deliberately does not agree. This is the one difference I would keep exactly as it is. The listings call throws:

// Email is the only remote channel. If it fails we throw, so the caller keeps
// the listings "unseen" and retries on the next poll.
if (!res.ok) {
  const body = await res.text();
  throw new Error(`Server returned ${res.status}: ${body}`);
}

The other three swallow and log. That asymmetry is the whole alerting design in four lines. A listing alert is the product: if the email did not go out, the listings must stay unseen so the next cycle tries again, because the alternative is silently losing the one thing the user paid for. The other three messages are about the app's own condition, and a failed attempt to say "we have stopped working" must never be the thing that stops the app working. The policy behind those is Two of our four emails exist only to say we stopped working, and three rules keep them from becoming noise.

The same thought appears in the reply notice, with the clearest version of the reason:

/**
 * Never throws: the reply itself has already happened by the time this runs, so
 * a failed email must not turn a successful send into a logged failure.
 */

What I am changing

Not the error policy, which is right and only looks like inconsistency.

The rest collapses into one function in notify.ts:

type NotifyPayload =
  | { type: 'listings'; alerts: AlertEntry[] }
  | { type: 'auth_failure'; authFailureSite: string }
  | { type: 'blocked'; blockedSite: string }
  | { type: 'reply'; reply: ReplyNotice };

async function postNotify(payload: NotifyPayload, options: { throwOnError: boolean }) { ... }

Three things fall out of that. The server URL is resolved once, by the helper that already strips the slash. The licence policy becomes one decision with one comment on it. And the discriminated union means the payload shape is checked against the server's four cases by the compiler, instead of by four JSON.stringify calls that happen to be written correctly.

Then the docstring becomes true, which is the actual point. A comment that claims a property is a promise, and the cheapest way to keep it is to make the thing it describes structurally the only option.

What transfers

1. An aspirational docstring is worse than none. "This module is the one place that knows X" is a fact that was never checked and reads like one that is maintained.
2. Extracting a function to break a cycle does not deduplicate anything. The pressure that caused the extraction stops exactly at the one call site that had the cycle.
3. A duplicated constant can cost latency rather than correctness. Three of these callers are wrong in a way that only shows up as an extra round trip, and nothing in any log says so.
4. Inconsistency that is deliberate needs a comment at every site, not one. The error-handling difference here is correct and looks exactly like the parts that are not.
5. One caller per enum case is how this hides. If two callers had sent the same type, the drift would have been obvious the first time one was changed.

If you want to see what these four messages actually produce, the help page covers what the app sends and when, and the per-site pages describe what a listing alert is for on each portal.