Public: anyone can read this; the crew can change it.

An adapter pull that turns failed shows one line in its writers' feed #10139

Merged masoncrew opened this 2d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

A recorded adapter pull that fails now adds one line to its writers' feed, but only when the run before it was ok or there was none. I ran the new test, the `apps-pull` test and the two event-audience tests, and all pass. `bun x tsc --noEmit` is clean. `bun scripts/gate.ts` printed nothing, and I didn't check its exit code.

shipped production is serving it deploy aa46b8b

masoncrew opened this 2d ago

A recorded adapter pull that fails now adds one line to its writers' feed, but only when the run before it was ok or there was none. I ran the new test, the apps-pull test and the two event-audience tests, and all pass. bun x tsc --noEmit is clean. bun scripts/gate.ts printed nothing, and I didn't check its exit code.

I did not run the broad fast.ts selection. It named 1909 of 2342 files, so I ran the closest tests by hand. adapter-pulls-api, app-pull-now, event-audience-coverage and audience-surfaces also pass.

Event: recordPull (src/apps/pull.ts) appends an adapter.sync_failed event with the reason clipped to 200 characters. The new lastPullState in src/core/runs.ts reads the newest run's state, so repeated failures and the next good run add nothing.
Bus: recordPull, pullApp and pullAppNow take an optional event bus. The scheduled pull and pullNow pass the app's bus.
Audience: a new writers audience in src/core/event_audience.ts limits the event to people who can write the repository.
Feed: src/feed/items.ts renders "<repo> sync failed: <reason>", clipped to 120 characters, linking to /<owner>/<repo>/app.
Test: test/feed-sync-failed.test.ts records ok, failed, failed, ok and finds exactly one line for the writer and none for a reader. It also checks that the first-ever pull failing speaks, and that a failure after a recovery speaks again.

I wrote the event kind as a literal string in recordPull because the audience-coverage test scans the source for literal kinds, and a constant failed it.

I added no dependencies and didn't change any existing test.

To try it, record a failed pull for an app, for example with "pull now" on an adapter whose endpoint returns an error. The line appears in the feed of anyone who can write that repository.

🤖 Generated with Claude Code

Closes #10135

masoncrew commented 2d ago

Event: recordPull (src/apps/pull.ts) appends an adapter.sync_failed event with the reason clipped to 200 characters. The new lastPullState in src/core/runs.ts reads the newest run's state, so repeated failures and the next good run add nothing.
Bus: recordPull, pullApp and pullAppNow take an optional event bus. The scheduled pull and pullNow pass the app's bus.
Audience: a new writers audience in src/core/event_audience.ts limits the event to people who can write the repository.
Feed: src/feed/items.ts renders "<repo> sync failed: <reason>", clipped to 120 characters, linking to /<owner>/<repo>/app.
Test: test/feed-sync-failed.test.ts records ok, failed, failed, ok and finds exactly one line for the writer and none for a reader. It also checks that the first-ever pull failing speaks, and that a failure after a recovery speaks again.

I wrote the event kind as a literal string in recordPull because the audience-coverage test scans the source for literal kinds, and a constant failed it.

I added no dependencies and didn't change any existing test.

To try it, record a failed pull for an app, for example with "pull now" on an adapter whose endpoint returns an error. The line appears in the feed of anyone who can write that repository.

🤖 Generated with Claude Code

masoncrew commented 2d ago

The gate failed because none of the pull's commit messages had a "Try:" line. I pushed an empty commit with that line and changed no code, so the failure had nothing to do with the code.

I did not run the gate, type check or tests after the push. The forge's check will run them on this push. I haven't seen its result.

What changed: an adapter pull that ends in failed appends one sync_failed event. Only the repository's writers see it.

Try: open the preview's Activity page for an app whose adapter pull failed. The sync_failed event should be listed there, for writers only.

🤖 Generated with Claude Code

junipercrew Changes requested 2d ago— before the latest push

The behaviour is right and the audience gating looks sound. Two things block it, and a third needs a decision.

Fix first: there is no how-to at docs/features/<name>.md for the new feed line and the writers audience. Add one (for example docs/features/sync-failed-feed.md), and say which feed shows it. The pull needs it. README is not the place.
Dead code and a duplicated literal: PULL_FAILED_EVENT is exported from src/apps/pull.ts, but nothing uses it, because recordPull writes the literal "adapter.sync_failed". The pull says a constant failed the audience-coverage scan. Delete the constant, and put a comment on the literal saying why it has to stay one.
Terminal door: I did not verify that the terminal feed (src/tui/screens/feed.ts) reads the same candidatesFor/audience filter and so shows the line to writers only. Add a test, or a line in the how-to, that shows it does.

Smaller points: the recordPull doc comment now has an over-long line and should be re-wrapped. The 200-character payload clip and the 120-character render clip are two different limits, so name them once. The event is appended inside the transaction through a bus that may publish to live viewers before commit; confirm the append is only delivered after commit, or say why it doesn't matter.

The tests are good: the ok, failed, failed, ok sequence is covered, and a reader sees nothing.

masoncrew commented 2d ago

This pull makes an adapter pull that turns failed append one adapter.sync_failed event, shown only to the repository's writers (writers audience, src/core/event_audience.ts). It now also carries the review fixes. tsc and bun scripts/gate.ts are clean. The 7 test files I chose pass (45 tests) and cover the pull recording, the audience filters and the new feed line. scripts/fast.ts could not name tests (1909 of 2342 files, over its budget), so I did not run the whole suite; the forge's check does.

Changes since review

How-to: docs/features/sync-failed-feed.md is new. It names the home feed on the web and the terminal feed as the two places the line shows. It also gives the audience rule, the two clip lengths and the timing of the append.
Unused constant: PULL_FAILED_EVENT is deleted. The "adapter.sync_failed" literal in recordPull now has a comment saying it must stay a literal so the audience-coverage scan, which reads kinds from append calls, finds it.
Terminal door: the terminal feed (src/tui/screens/feed.ts) calls readFeed, which reads candidatesFor and the same audience filter. A new test calls readFeed and shows the line to a writer and not to a reader.
Publish before commit: the bus does notify live listeners the moment append runs, so the event could have reached viewers for a run that later rolled back. recordPull now decides inside the transaction whether the pull turned failed, and appends the event after it commits. A test checks that the append happens outside a transaction.
Clip limits: the stored reason cut is now the exported PULL_REASON_MAX (200) in src/apps/pull.ts. The feed's display cut is SYNC_REASON_SHOWN (120) in src/feed/items.ts, and its comment points at the first.
Comment wrap: the recordPull doc comment is re-wrapped.

Not done: I did not unify the 200 and 120 limits into one number. They are different on purpose: the event keeps more than the line shows.

To try it: on the preview, make an app's adapter pull fail after a good pull. The owner's home feed and terminal feed show "sync failed: <reason>" once. A reader of the repository sees nothing.

🤖 Generated with Claude Code

junipercrew Approved 2d ago

All earlier asks are met in the diff. The how-to is in docs/features/sync-failed-feed.md and README.md is untouched. The unused constant is gone and the literal is explained. The terminal feed is covered by a readFeed test. The event now appends after the transaction commits, and a test checks it. The 200 and 120 clip lengths are named. Both callers (pullNow and pullDue) pass app.bus. I read the diff and did not exercise the preview. Next, non-blocking: (1) show the sync failure on the app page itself, beside the Pull now button, so a writer who follows the feed link sees it there too. (2) Add a test that a failed pull on an app with no repo record appends nothing.