Repository navigation
Conversation
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
🚀 Deploying Preview to Cloudflare 🚀Preview Deployments by commit
|
There was a problem hiding this comment.
4 issues found across 23 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/adapters/webhook.ts">
<violation number="1" location="src/adapters/webhook.ts:48">
P2: This swallows response-read failures and records an unreadable 2xx as published with a fallback ID. Read the body through `deliver()` so the publish is recorded as uncertain when the response stream fails.</violation>
</file>
<file name="src/contrast.ts">
<violation number="1" location="src/contrast.ts:39">
P2: Brand colours in `hsl()`, `oklch()`, named, `#rrggbbaa`, or alpha `rgba()` notation are silently skipped by the contrast check. `doctor` never reports them, so an all-unsupported palette gets no brand check at all. Report skipped pairs in `doctor` as unverified, or support these formats.</violation>
</file>
<file name="src/ledger.ts">
<violation number="1" location="src/ledger.ts:56">
P2: `firstAttemptAt` and `retryBefore` are only type-checked as strings, not as dates like `updatedAt`. A malformed value makes the Mastodon retry-window check fail open, or makes `toISOString()` throw and abort the publish run. Validate both as parseable dates here.</violation>
</file>
<file name="src/lint.ts">
<violation number="1" location="src/lint.ts:101">
P3: The lint message says `set alt:` instead of `set alt text`, producing a malformed instruction for users. Change it to `set alt text for the image...`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| // Any 2xx counts. A reply is optional, but a JSON reply that doesn't parse is worth a note. | ||
| let reply: { id?: unknown; url?: unknown } = {}; | ||
| let warning: string | undefined; | ||
| const raw = await res.text().catch(() => ""); |
There was a problem hiding this comment.
P2: This swallows response-read failures and records an unreadable 2xx as published with a fallback ID. Read the body through deliver() so the publish is recorded as uncertain when the response stream fails.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/adapters/webhook.ts, line 48:
<comment>This swallows response-read failures and records an unreadable 2xx as published with a fallback ID. Read the body through `deliver()` so the publish is recorded as uncertain when the response stream fails.</comment>
<file context>
@@ -42,11 +42,20 @@ export function webhook(name: string, cfg: WebhookChannelConfig, deps: AdapterDe
+ // Any 2xx counts. A reply is optional, but a JSON reply that doesn't parse is worth a note.
+ let reply: { id?: unknown; url?: unknown } = {};
+ let warning: string | undefined;
+ const raw = await res.text().catch(() => "");
+ if (raw.trim()) {
+ try {
</file context>
| const raw = await res.text().catch(() => ""); | |
| const raw = await deliver(`${name}: webhook response`, () => res.text()); |
| export function contrastRatio(foreground: string, background: string): number | undefined { | ||
| const fg = parseColor(foreground); | ||
| const bg = parseColor(background); | ||
| if (!fg || !bg) return undefined; |
There was a problem hiding this comment.
P2: Brand colours in hsl(), oklch(), named, #rrggbbaa, or alpha rgba() notation are silently skipped by the contrast check. doctor never reports them, so an all-unsupported palette gets no brand check at all. Report skipped pairs in doctor as unverified, or support these formats.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/contrast.ts, line 39:
<comment>Brand colours in `hsl()`, `oklch()`, named, `#rrggbbaa`, or alpha `rgba()` notation are silently skipped by the contrast check. `doctor` never reports them, so an all-unsupported palette gets no brand check at all. Report skipped pairs in `doctor` as unverified, or support these formats.</comment>
<file context>
@@ -0,0 +1,42 @@
+export function contrastRatio(foreground: string, background: string): number | undefined {
+ const fg = parseColor(foreground);
+ const bg = parseColor(background);
+ if (!fg || !bg) return undefined;
+ const [hi, lo] = [luminance(fg), luminance(bg)].sort((a, b) => b - a) as [number, number];
+ return (hi + 0.05) / (lo + 0.05);
</file context>
| if (ext !== ".png" && ext !== ".jpg" && ext !== ".jpeg") add(post, "image-format", "error", "image must be PNG or JPEG"); | ||
| if (!post.alt?.trim()) add(post, "image-alt", "warn", "set alt text for the image"); | ||
| if (!post.alt?.trim()) { | ||
| add(post, "image-alt", cfg.rules.requireAlt ? "error" : "warn", "set alt: for the image, so screen reader users know what it shows"); |
There was a problem hiding this comment.
P3: The lint message says set alt: instead of set alt text, producing a malformed instruction for users. Change it to set alt text for the image....
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/lint.ts, line 101:
<comment>The lint message says `set alt:` instead of `set alt text`, producing a malformed instruction for users. Change it to `set alt text for the image...`.</comment>
<file context>
@@ -97,7 +97,9 @@ export function lint(cfg: Config, posts: Post[]): Finding[] {
if (ext !== ".png" && ext !== ".jpg" && ext !== ".jpeg") add(post, "image-format", "error", "image must be PNG or JPEG");
- if (!post.alt?.trim()) add(post, "image-alt", "warn", "set alt text for the image");
+ if (!post.alt?.trim()) {
+ add(post, "image-alt", cfg.rules.requireAlt ? "error" : "warn", "set alt: for the image, so screen reader users know what it shows");
+ }
}
</file context>
| add(post, "image-alt", cfg.rules.requireAlt ? "error" : "warn", "set alt: for the image, so screen reader users know what it shows"); | |
| add(post, "image-alt", cfg.rules.requireAlt ? "error" : "warn", "set alt text for the image, so screen reader users know what it shows"); |
…, check colours as drawn
There was a problem hiding this comment.
1 issue found across 16 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/doctor.ts">
<violation number="1" location="src/doctor.ts:168">
P2: The new check ignores custom layouts and the user stylesheet, which can override these colors or draw different backgrounds. Scope the result to built-in layouts or inspect the effective custom styles before reporting it as a pass.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| const panel = read(colors.panel); | ||
| const accent = read(colors.accent); | ||
| type Pair = { name: string; fg: Rgb | undefined; bg: Rgb | undefined; min: number }; | ||
| const pairs: Pair[] = [ |
There was a problem hiding this comment.
P2: The new check ignores custom layouts and the user stylesheet, which can override these colors or draw different backgrounds. Scope the result to built-in layouts or inspect the effective custom styles before reporting it as a pass.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/doctor.ts, line 168:
<comment>The new check ignores custom layouts and the user stylesheet, which can override these colors or draw different backgrounds. Scope the result to built-in layouts or inspect the effective custom styles before reporting it as a pass.</comment>
<file context>
@@ -157,26 +157,44 @@ export async function runDoctor(opts: DoctorOptions): Promise<Check[]> {
+ const panel = read(colors.panel);
+ const accent = read(colors.accent);
+ type Pair = { name: string; fg: Rgb | undefined; bg: Rgb | undefined; min: number };
+ const pairs: Pair[] = [
+ { name: "text on background", fg: read(colors.text), bg, min: 4.5 },
+ { name: "muted text on background", fg: read(colors.muted), bg, min: 4.5 },
</file context>
…pelling, fit warnings, silent blocks
There was a problem hiding this comment.
5 issues found across 11 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/render/card.ts">
<violation number="1" location="src/render/card.ts:178">
P2: This `margin-top:2.4rem` on `footer` changes the layout of every built-in card, not only the overflow case. The flex `.main` area shrinks by 24px, so centred content shifts and overflowing content pushes the footer further into the clipped area. If the spacing is intended, say so; otherwise drop the margin. Either way, re-render the examples to confirm they still match the 0.3.2 claim that they render unchanged.</violation>
</file>
<file name="src/lint.ts">
<violation number="1" location="src/lint.ts:148">
P3: Merging depends on comparing message text, and `too-long` puts the channel name inside that text when no variant is set. The same over-length text on two channels is still listed twice. Pass the channel as a structured field on `Finding` instead of parsing the message.</violation>
</file>
<file name="src/cli.ts">
<violation number="1" location="src/cli.ts:473">
P2: The new default skips the scaffolded draft, but the documented first-render workflow still runs `postline render --html` and expects `welcome.png`; it now exits successfully without creating that file. Update the onboarding commands and the `init` next-step message to name the sample or pass `--drafts`.</violation>
</file>
<file name="src/render/png.ts">
<violation number="1" location="src/render/png.ts:122">
P2: The overflow check compares against the padding box, so text that runs into the card's padding margin is not flagged. `scrollWidth`/`scrollHeight` only exceed `clientWidth`/`clientHeight` once content passes the outer edge. Compare the descendants' bounding rects against the `.card` content box instead, so margin spill triggers the warning the CHANGELOG describes.</violation>
<violation number="2" location="src/render/png.ts:128">
P2: This warning is dropped in `preview` and `pr-report`, because those commands construct `Renderer` without a `warn` callback. Pass the same `warn: (m) => console.error(yellow(m))` option there, or route the message through a default logger, so authors see overflow warnings when they iterate on cards in preview.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| .panel{background:var(--panel);border:1px solid var(--border);border-radius:1.6rem;padding:${wide ? "2.4rem 2.6rem" : "3.4rem"};display:flex;flex-direction:column;justify-content:flex-start} | ||
| .panel .chip{align-self:flex-start;margin-top:1.6rem} | ||
| footer{display:flex;justify-content:space-between;align-items:flex-end;gap:4rem;color:var(--muted);font-size:2.4rem;line-height:1.35} | ||
| footer{display:flex;justify-content:space-between;align-items:flex-end;gap:4rem;margin-top:2.4rem;color:var(--muted);font-size:2.4rem;line-height:1.35} |
There was a problem hiding this comment.
P2: This margin-top:2.4rem on footer changes the layout of every built-in card, not only the overflow case. The flex .main area shrinks by 24px, so centred content shifts and overflowing content pushes the footer further into the clipped area. If the spacing is intended, say so; otherwise drop the margin. Either way, re-render the examples to confirm they still match the 0.3.2 claim that they render unchanged.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/render/card.ts, line 178:
<comment>This `margin-top:2.4rem` on `footer` changes the layout of every built-in card, not only the overflow case. The flex `.main` area shrinks by 24px, so centred content shifts and overflowing content pushes the footer further into the clipped area. If the spacing is intended, say so; otherwise drop the margin. Either way, re-render the examples to confirm they still match the 0.3.2 claim that they render unchanged.</comment>
<file context>
@@ -175,7 +175,7 @@ ${designHead(design)}
.panel{background:var(--panel);border:1px solid var(--border);border-radius:1.6rem;padding:${wide ? "2.4rem 2.6rem" : "3.4rem"};display:flex;flex-direction:column;justify-content:flex-start}
.panel .chip{align-self:flex-start;margin-top:1.6rem}
- footer{display:flex;justify-content:space-between;align-items:flex-end;gap:4rem;color:var(--muted);font-size:2.4rem;line-height:1.35}
+ footer{display:flex;justify-content:space-between;align-items:flex-end;gap:4rem;margin-top:2.4rem;color:var(--muted);font-size:2.4rem;line-height:1.35}
footer .url{color:var(--accent);font-weight:700;white-space:nowrap}
</style>
</file context>
| footer{display:flex;justify-content:space-between;align-items:flex-end;gap:4rem;margin-top:2.4rem;color:var(--muted);font-size:2.4rem;line-height:1.35} | |
| footer{display:flex;justify-content:space-between;align-items:flex-end;gap:4rem;color:var(--muted);font-size:2.4rem;line-height:1.35} |
| const targets = posts.filter((p) => p.card && (!id || p.id === id)); | ||
| // A bulk render skips drafts (their cards are placeholders, and the library | ||
| // keeps every version); name a draft or pass --drafts to render it anyway. | ||
| const targets = posts.filter((p) => p.card && (id ? p.id === id : p.status !== "draft" || Boolean(args.flags.drafts))); |
There was a problem hiding this comment.
P2: The new default skips the scaffolded draft, but the documented first-render workflow still runs postline render --html and expects welcome.png; it now exits successfully without creating that file. Update the onboarding commands and the init next-step message to name the sample or pass --drafts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/cli.ts, line 473:
<comment>The new default skips the scaffolded draft, but the documented first-render workflow still runs `postline render --html` and expects `welcome.png`; it now exits successfully without creating that file. Update the onboarding commands and the `init` next-step message to name the sample or pass `--drafts`.</comment>
<file context>
@@ -467,7 +468,9 @@ async function main(argv: string[]): Promise<number> {
- const targets = posts.filter((p) => p.card && (!id || p.id === id));
+ // A bulk render skips drafts (their cards are placeholders, and the library
+ // keeps every version); name a draft or pass --drafts to render it anyway.
+ const targets = posts.filter((p) => p.card && (id ? p.id === id : p.status !== "draft" || Boolean(args.flags.drafts)));
if (id && targets.length === 0) {
console.error(`no post "${id}" with a card`);
</file context>
| ), | ||
| ); | ||
| if (overflow) { | ||
| this.options.warn?.(`${label}: the card's content doesn't fit the ${size} card; shorten the text or use a larger card.size`); |
There was a problem hiding this comment.
P2: This warning is dropped in preview and pr-report, because those commands construct Renderer without a warn callback. Pass the same warn: (m) => console.error(yellow(m)) option there, or route the message through a default logger, so authors see overflow warnings when they iterate on cards in preview.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/render/png.ts, line 128:
<comment>This warning is dropped in `preview` and `pr-report`, because those commands construct `Renderer` without a `warn` callback. Pass the same `warn: (m) => console.error(yellow(m))` option there, or route the message through a default logger, so authors see overflow warnings when they iterate on cards in preview.</comment>
<file context>
@@ -109,6 +116,18 @@ export class Renderer {
+ ),
+ );
+ if (overflow) {
+ this.options.warn?.(`${label}: the card's content doesn't fit the ${size} card; shorten the text or use a larger card.size`);
+ }
+ }
</file context>
| if (label) { | ||
| // Text that doesn't fit is pushed into the margins or off the card, and | ||
| // nothing else would say so. | ||
| const overflow = await page.evaluate(() => |
There was a problem hiding this comment.
P2: The overflow check compares against the padding box, so text that runs into the card's padding margin is not flagged. scrollWidth/scrollHeight only exceed clientWidth/clientHeight once content passes the outer edge. Compare the descendants' bounding rects against the .card content box instead, so margin spill triggers the warning the CHANGELOG describes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/render/png.ts, line 122:
<comment>The overflow check compares against the padding box, so text that runs into the card's padding margin is not flagged. `scrollWidth`/`scrollHeight` only exceed `clientWidth`/`clientHeight` once content passes the outer edge. Compare the descendants' bounding rects against the `.card` content box instead, so margin spill triggers the warning the CHANGELOG describes.</comment>
<file context>
@@ -109,6 +116,18 @@ export class Renderer {
+ if (label) {
+ // Text that doesn't fit is pushed into the margins or off the card, and
+ // nothing else would say so.
+ const overflow = await page.evaluate(() =>
+ [document.body, document.querySelector(".card")].some(
+ (el) => el instanceof HTMLElement && (el.scrollHeight > el.clientHeight + 1 || el.scrollWidth > el.clientWidth + 1),
</file context>
| merged.push(f); | ||
| continue; | ||
| } | ||
| const rest = match[2]!; |
There was a problem hiding this comment.
P3: Merging depends on comparing message text, and too-long puts the channel name inside that text when no variant is set. The same over-length text on two channels is still listed twice. Pass the channel as a structured field on Finding instead of parsing the message.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/lint.ts, line 148:
<comment>Merging depends on comparing message text, and `too-long` puts the channel name inside that text when no variant is set. The same over-length text on two channels is still listed twice. Pass the channel as a structured field on `Finding` instead of parsing the message.</comment>
<file context>
@@ -125,5 +128,34 @@ export function lint(cfg: Config, posts: Post[]): Finding[] {
+ merged.push(f);
+ continue;
+ }
+ const rest = match[2]!;
+ const key = `${f.post.id}\u0000${f.rule}\u0000${f.severity}\u0000${rest}`;
+ const group = groups.get(key);
</file context>
…skip note, token name clashes
There was a problem hiding this comment.
1 existing issue remains and 1 new issue found across 10 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="CHANGELOG.md">
<violation number="1" location="CHANGELOG.md:21">
P3: The generated PR report does not show content-fit warnings; `pr-report` only sends them to stderr while its Markdown contains lint findings and render errors. Remove the PR-report claim or include the warning in `buildReport` before documenting it as part of the report.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
View guided diff | Turn on auto-fix | Re-trigger cubic
| - Token CSS variables keep the token's own spelling as well as lowercase, so `var(--fgColor-default)` works for design systems that use capitals, as Primer does | ||
| - `postline render` skips drafts unless you name one or pass `--drafts` (and says how many it skipped), so placeholder cards don't fill the graphics library | ||
| - Two design tokens that would become the same CSS variable are now a config error instead of one silently winning | ||
| - The renderer warns when a card's content doesn't fit: long text that spills off the card or eats more than half of its margin. `render`, `preview` and the PR report all show it |
There was a problem hiding this comment.
P3: The generated PR report does not show content-fit warnings; pr-report only sends them to stderr while its Markdown contains lint findings and render errors. Remove the PR-report claim or include the warning in buildReport before documenting it as part of the report.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At CHANGELOG.md, line 21:
<comment>The generated PR report does not show content-fit warnings; `pr-report` only sends them to stderr while its Markdown contains lint findings and render errors. Remove the PR-report claim or include the warning in `buildReport` before documenting it as part of the report.</comment>
<file context>
@@ -16,8 +16,9 @@ Found by running postline end to end for a real brand (GitHub's changelog, with
-- The renderer warns when a card's content doesn't fit (long text pushing into the margins), and the footer always keeps its spacing
+- `postline render` skips drafts unless you name one or pass `--drafts` (and says how many it skipped), so placeholder cards don't fill the graphics library
+- Two design tokens that would become the same CSS variable are now a config error instead of one silently winning
+- The renderer warns when a card's content doesn't fit: long text that spills off the card or eats more than half of its margin. `render`, `preview` and the PR report all show it
- A post that has used up `maxAttempts` now says so and how to fix it; before, the run exited with an error and no message
- The same problem on several channels is reported once, and error messages no longer repeat the channel name
</file context>
| - The renderer warns when a card's content doesn't fit: long text that spills off the card or eats more than half of its margin. `render`, `preview` and the PR report all show it | |
| - The renderer warns when a card's content doesn't fit: long text that spills off the card or eats more than half of its margin. `render` and `preview` show it |
Summary
Safer rendering. Cards render with JavaScript off, and the page can only load
data:URLs, the brand font host (plusfonts.gstatic.comfor Google Fonts) and hosts listed in the newcard.allowedHosts(HTTPS only). Post text inserted with{{{raw}}}can't run scripts or send data anywhere while a card renders. Checked by rendering a hostile template: 0.3.0 ran the injected script, this branch doesn't. The example cards render the same as before.Reliability
maxAttemptsand stops retrying, instead of failing every run.uncertain. Done with a generic "retry before" deadline on the ledger entry, checked against the publish run's own clock.Lint and doctor
rules.requireAltmakes missing alt text on image posts an error (it stays a warning by default).postline doctorchecks brand colours against WCAG AA contrast (text, muted and chip text at 4.5:1, accent at 3:1). Both example brands pass.Docs, changelog and version 0.3.2.
Test plan
npm test: 110 pass (new: render allow-list, contrast, requireAlt, render-failure cap, Mastodon window inside and after, lock released while waiting, webhook bad JSON)Summary by cubic
Sandboxes card rendering so injected post text can't run scripts or send data, and fixes several reliability and publishing gaps.
data:URLs, and hosts incard.allowedHosts(HTTPS only); a non-HTTPSbrand.font.urlis now a config error.maxAttemptsand stops retrying instead of failing every publish run.uncertain.rules.requireAltmakes missing alt text on image posts an error;postline doctorchecks brand colours against WCAG AA contrast as the base card draws them, and lists colours it can't read.An end-to-end run against a real brand fixed the gaps it found:
postline planlint with warnings instead of errors (a mistyped status stays an error), andrenderskips drafts unless one is named or--draftsis passed.var(--fgColor-default)works; two tokens that map to the same CSS variable are now a config error; the renderer warns when card content doesn't fit.maxAttemptssays what happened and how to retry; the same finding on several channels is reported once, without repeating channel names.Docs and changelog updated; version bumped to 0.3.2.
Written for commit c664d78. Summary will update on new commits.
Fixes from an end-to-end trial
postline was run end to end for a real brand: GitHub's changelog, with Primer's published design tokens (unmodified), Mona Sans, a custom layout, four channels (files and a signed webhook for real; LinkedIn and Bluesky without credentials), alerts, and the PR report. It found:
postline plandrafts failedlintwith errors, which would fail the PR check. Declared drafts now get warnings; a mistyped status stays an error.var(--fgColor-default)silently matched nothing in a custom layout. Both spellings are now emitted.renderrendered placeholder drafts into the graphics library. Drafts are now skipped unless named or--drafts.maxAttemptsmadepublishexit 1 with no output. It now says what happened and how to fix it.