Conversation
Preserve task-focused articles and localized troubleshooting, derive public counts, and bind localized disclosures to the actual advertising scope without account-state claims. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
10 issues found across 20 files
Confidence score: 2/5
src/ui/legal-content.jsoverwrites the security section at runtime, so the new security-reporting instructions never reach users; preserve the detailed localized security content and limit the response-message override to contact content.src/utils/ads.jscan still allow homepage advertising while the updated Terms prohibit it, creating a policy/behavior mismatch; aligngetAdPolicySnapshot(),pageAllowsAds(), andslotKeyForPath()and add coverage for the homepage case.scripts/i18n-content-locales.mjsdoes not detect stale generated locale output, allowing outdated localized guidance or legal text to ship; make CI or the build regenerate and verify the generated file.src/ui/faq-content.jsdrops an explicit FAQ locale across internal links and makes the existing non-English fallback arrays unreachable, causing navigation or fallback regressions; preservelangin links and either use or remove the obsolete arrays.
Not reviewed (too large): scripts/data/content-locales.json (~1,661 lines), src/ui/generated-content-locales.js (~1,661 lines), src/ui/blog.js (~953 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/i18n-content-locales.mjs">
<violation number="1" location="scripts/i18n-content-locales.mjs:4">
P2: When `scripts/data/content-locales.json` changes without manually regenerating the output, the build and CI never detect it and can deploy stale localized guidance or legal text from `src/ui/generated-content-locales.js`. Add this generator to the build/check pipeline, or add a CI `--check` step that fails on stale output.</violation>
</file>
<file name="src/ui/faq-content.js">
<violation number="1" location="src/ui/faq-content.js:2038">
P3: The new reviewed-locale branch makes every non-English `FAQ_CONTENT` array unreachable because generated data exists for all supported locales. Remove the obsolete localized arrays or make them a real fallback so stale duplicate content is not bundled and maintained.</violation>
<violation number="2" location="src/ui/faq-content.js:2038">
P2: When a localized FAQ is opened with an explicit `lang` query, its newly selected translated answers lose that locale on every internal link. Preserve the current language when rendering FAQ links, for example by applying `withLanguageQuery` to internal FAQ hrefs.</violation>
</file>
<file name="src/ui/legal-content.js">
<violation number="1" location="src/ui/legal-content.js:3885">
P2: The new security-reporting instructions are never rendered because this branch overwrites the security section at runtime. Apply the response-message override only to contact, or preserve and localize the detailed security response separately.</violation>
<violation number="2" location="src/ui/legal-content.js:3893">
P3: The About templates already include `data-public-tool-count`, but this fallback replacement inserts the same attribute again when the rendered count is 45. Preserve the captured opening tag without adding a second attribute, or remove this fallback now that the templates are registry-backed.</violation>
</file>
<file name="src/ui/legal-pages.js">
<violation number="1" location="src/ui/legal-pages.js:101">
P2: The new ad-policy disclosure (renderAdPolicyScope) has no test coverage: honest-copy.test.js's "machine-bound advertising disclosure" suite only asserts that the section binds getAdPolicySnapshot() and the label object — it never renders a legal page and never checks the emitted markup. So the machine-readable scope this PR introduces (data-ad-policy-scope="v1", data-ad-policy-path, data-auto-ads-status, data-cmp-status) can silently drift out of sync with getAdPolicySnapshot().add before approval.</violation>
</file>
<file name="src/ui/honest-copy.test.js">
<violation number="1" location="src/ui/honest-copy.test.js:336">
P2: This test will fail as written. The regex-guide content renders the token as a doubled backslash `\d` (the article source writes `<code>\\d</code>`, which a template literal evaluates to backslash-backslash-d), so `codeTokens.find((token) => token === "\d")` never matches, `digitToken` is `undefined`, and `expect(digitToken).toBe("\d")` throws while the page shows `\d`. The test's intent (asserting the example demonstrates single-backslash `\d` matching ASCII digit `5` but not Arabic-Indic `٥`) requires the content to render a single backslash; fix the article content to `<code>\d</code>` in the blog.js template literal (one escaped backslash under the template-literal convention) rather than `\\d`.</violation>
</file>
<file name="docs/adsense-integration.md">
<violation number="1" location="docs/adsense-integration.md:42">
P2: The local-testing instructions reference `.omo/evidence/lean-final/adsense/current-worker-http-harness.mjs`, but that harness is not present or tracked in this repository. Replace the reference with a committed harness or a reproducible command against a deployed production URL so operators can actually verify the worker.</violation>
</file>
<file name="src/routes/changelog.js">
<violation number="1" location="src/routes/changelog.js:35">
P3: Adding or removing a production tool later will silently change this already-published release note because `PRODUCTION_TOOL_COUNT` is computed from the current registry. Keep historical changelog entries stable by removing the interpolation or recording the release-time count.</violation>
</file>
<file name="src/utils/ads.js">
<violation number="1" location="src/utils/ads.js:44">
P2: The published manual-ad policy still includes `/`, but the updated Terms copy explicitly removes the homepage from the advertising allow-list. Align `getAdPolicySnapshot()`, `pageAllowsAds()`/`slotKeyForPath()`, and the legal copy so the homepage is either consistently eligible and disclosed or consistently excluded.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| @@ -0,0 +1,87 @@ | |||
| #!/usr/bin/env node | |||
There was a problem hiding this comment.
P2: When scripts/data/content-locales.json changes without manually regenerating the output, the build and CI never detect it and can deploy stale localized guidance or legal text from src/ui/generated-content-locales.js. Add this generator to the build/check pipeline, or add a CI --check step that fails on stale output.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/i18n-content-locales.mjs, line 4:
<comment>When `scripts/data/content-locales.json` changes without manually regenerating the output, the build and CI never detect it and can deploy stale localized guidance or legal text from `src/ui/generated-content-locales.js`. Add this generator to the build/check pipeline, or add a CI `--check` step that fails on stale output.</comment>
<file context>
@@ -0,0 +1,87 @@
+#!/usr/bin/env node
+import fs from "node:fs";
+
+const sourceUrl = new URL("./data/content-locales.json", import.meta.url);
+const outputUrl = new URL("../src/ui/generated-content-locales.js", import.meta.url);
+const localeSource = JSON.parse(fs.readFileSync(sourceUrl, "utf8"));
</file context>
| const currentLang = normalizeLanguage(lang); | ||
| return ( | ||
| FAQ_CONTENT[currentLang] || FAQ_CONTENT[DEFAULT_LANGUAGE] || FAQ_CONTENT.en | ||
| REVIEWED_CONTENT_LOCALES[currentLang]?.faq || |
There was a problem hiding this comment.
P2: When a localized FAQ is opened with an explicit lang query, its newly selected translated answers lose that locale on every internal link. Preserve the current language when rendering FAQ links, for example by applying withLanguageQuery to internal FAQ hrefs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ui/faq-content.js, line 2038:
<comment>When a localized FAQ is opened with an explicit `lang` query, its newly selected translated answers lose that locale on every internal link. Preserve the current language when rendering FAQ links, for example by applying `withLanguageQuery` to internal FAQ hrefs.</comment>
<file context>
@@ -2146,6 +2035,9 @@ const FAQ_CONTENT = {
const currentLang = normalizeLanguage(lang);
return (
- FAQ_CONTENT[currentLang] || FAQ_CONTENT[DEFAULT_LANGUAGE] || FAQ_CONTENT.en
+ REVIEWED_CONTENT_LOCALES[currentLang]?.faq ||
+ FAQ_CONTENT[currentLang] ||
+ FAQ_CONTENT[DEFAULT_LANGUAGE] ||
</file context>
| return sections.map((section, index) => { | ||
| const next = { ...section }; | ||
| if ( | ||
| (pageId === "contact" || pageId === "security") && |
There was a problem hiding this comment.
P2: The new security-reporting instructions are never rendered because this branch overwrites the security section at runtime. Apply the response-message override only to contact, or preserve and localize the detailed security response separately.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ui/legal-content.js, line 3885:
<comment>The new security-reporting instructions are never rendered because this branch overwrites the security section at runtime. Apply the response-message override only to contact, or preserve and localize the detailed security response separately.</comment>
<file context>
@@ -3906,5 +3874,29 @@ export function getLegalSections(pageId, lang = DEFAULT_LANGUAGE) {
+ return sections.map((section, index) => {
+ const next = { ...section };
+ if (
+ (pageId === "contact" || pageId === "security") &&
+ index === sections.length - 1
+ ) {
</file context>
| @@ -86,6 +86,28 @@ function wrapLegalPage({ | |||
| }); | |||
There was a problem hiding this comment.
P2: The new ad-policy disclosure (renderAdPolicyScope) has no test coverage: honest-copy.test.js's "machine-bound advertising disclosure" suite only asserts that the section binds getAdPolicySnapshot() and the label object — it never renders a legal page and never checks the emitted markup. So the machine-readable scope this PR introduces (data-ad-policy-scope="v1", data-ad-policy-path, data-auto-ads-status, data-cmp-status) can silently drift out of sync with getAdPolicySnapshot().add before approval.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ui/legal-pages.js, line 101:
<comment>The new ad-policy disclosure (renderAdPolicyScope) has no test coverage: honest-copy.test.js's "machine-bound advertising disclosure" suite only asserts that the section binds getAdPolicySnapshot() and the label object — it never renders a legal page and never checks the emitted markup. So the machine-readable scope this PR introduces (data-ad-policy-scope="v1", data-ad-policy-path, data-auto-ads-status, data-cmp-status) can silently drift out of sync with getAdPolicySnapshot().add before approval.</comment>
<file context>
@@ -86,6 +86,28 @@ function wrapLegalPage({
+ .map((prefix) => `<li><code data-ad-policy-prefix="${prefix}">${prefix}*</code></li>`)
+ .join("");
+
+ return `<div class="not-prose mt-4 rounded-lg border border-border bg-muted/40 p-4" data-ad-policy-scope="v1" data-auto-ads-status="${policy.autoAdsStatus}" data-cmp-status="${policy.cmpStatus}">
+ <p class="text-sm font-semibold text-foreground">${labels.title}</p>
+ <p class="mt-2 text-sm text-muted-foreground">${labels.manualUnits}</p>
</file context>
| const codeTokens = [...article.content.matchAll(/<code>([^<]+)<\/code>/g)].map( | ||
| (match) => match[1], | ||
| ); | ||
| const digitToken = codeTokens.find((token) => token === "\\d"); |
There was a problem hiding this comment.
P2: This test will fail as written. The regex-guide content renders the token as a doubled backslash \d (the article source writes <code>\\d</code>, which a template literal evaluates to backslash-backslash-d), so codeTokens.find((token) => token === "\d") never matches, digitToken is undefined, and expect(digitToken).toBe("\d") throws while the page shows \d. The test's intent (asserting the example demonstrates single-backslash \d matching ASCII digit 5 but not Arabic-Indic ٥) requires the content to render a single backslash; fix the article content to <code>\d</code> in the blog.js template literal (one escaped backslash under the template-literal convention) rather than \\d.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ui/honest-copy.test.js, line 336:
<comment>This test will fail as written. The regex-guide content renders the token as a doubled backslash `\d` (the article source writes `<code>\\d</code>`, which a template literal evaluates to backslash-backslash-d), so `codeTokens.find((token) => token === "\d")` never matches, `digitToken` is `undefined`, and `expect(digitToken).toBe("\d")` throws while the page shows `\d`. The test's intent (asserting the example demonstrates single-backslash `\d` matching ASCII digit `5` but not Arabic-Indic `٥`) requires the content to render a single backslash; fix the article content to `<code>\d</code>` in the blog.js template literal (one escaped backslash under the template-literal convention) rather than `\\d`.</comment>
<file context>
@@ -303,6 +320,174 @@ describe("retired blog twins", () => {
+ const codeTokens = [...article.content.matchAll(/<code>([^<]+)<\/code>/g)].map(
+ (match) => match[1],
+ );
+ const digitToken = codeTokens.find((token) => token === "\\d");
+ expect(digitToken).toBe("\\d");
+ const digitPattern = new RegExp(`^${digitToken}$`);
</file context>
| - Blog and FAQ load the client script for site-connect but are not unit inventory. | ||
| - Dev / local environments disable ads, ads.txt, and the account meta. | ||
| - Blog and FAQ load the client script for site-connect but are not unit inventory. Privacy disclosures include these script-load routes even though they have no manual unit. | ||
| - Dev / local environments disable ads, ads.txt, and the account meta. To test the current worker without deployment, invoke the worker with a production URL as `.omo/evidence/lean-final/adsense/current-worker-http-harness.mjs` does; changing local environment variables while requesting localhost cannot enable ads. |
There was a problem hiding this comment.
P2: The local-testing instructions reference .omo/evidence/lean-final/adsense/current-worker-http-harness.mjs, but that harness is not present or tracked in this repository. Replace the reference with a committed harness or a reproducible command against a deployed production URL so operators can actually verify the worker.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/adsense-integration.md, line 42:
<comment>The local-testing instructions reference `.omo/evidence/lean-final/adsense/current-worker-http-harness.mjs`, but that harness is not present or tracked in this repository. Replace the reference with a committed harness or a reproducible command against a deployed production URL so operators can actually verify the worker.</comment>
<file context>
@@ -29,14 +29,17 @@ slot IDs. Secret tools never load the script.
-- Blog and FAQ load the client script for site-connect but are not unit inventory.
-- Dev / local environments disable ads, ads.txt, and the account meta.
+- Blog and FAQ load the client script for site-connect but are not unit inventory. Privacy disclosures include these script-load routes even though they have no manual unit.
+- Dev / local environments disable ads, ads.txt, and the account meta. To test the current worker without deployment, invoke the worker with a production URL as `.omo/evidence/lean-final/adsense/current-worker-http-harness.mjs` does; changing local environment variables while requesting localhost cannot enable ads.
Example:
</file context>
Suggested change
| - Dev / local environments disable ads, ads.txt, and the account meta. To test the current worker without deployment, invoke the worker with a production URL as `.omo/evidence/lean-final/adsense/current-worker-http-harness.mjs` does; changing local environment variables while requesting localhost cannot enable ads. | |
| - Dev / local environments disable ads, ads.txt, and the account meta. To test the production configuration, request a deployed production URL; changing local environment variables while requesting localhost cannot enable ads. |
|
|
||
| export function getAdPolicySnapshot() { | ||
| return Object.freeze({ | ||
| manualUnitPaths: Object.freeze(["/", "/json-formatter", ...LEGAL_AD_PATHS]), |
There was a problem hiding this comment.
P2: The published manual-ad policy still includes /, but the updated Terms copy explicitly removes the homepage from the advertising allow-list. Align getAdPolicySnapshot(), pageAllowsAds()/slotKeyForPath(), and the legal copy so the homepage is either consistently eligible and disclosed or consistently excluded.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/utils/ads.js, line 44:
<comment>The published manual-ad policy still includes `/`, but the updated Terms copy explicitly removes the homepage from the advertising allow-list. Align `getAdPolicySnapshot()`, `pageAllowsAds()`/`slotKeyForPath()`, and the legal copy so the homepage is either consistently eligible and disclosed or consistently excluded.</comment>
<file context>
@@ -25,6 +25,31 @@ export const LEGAL_AD_PATHS = Object.freeze([
+
+export function getAdPolicySnapshot() {
+ return Object.freeze({
+ manualUnitPaths: Object.freeze(["/", "/json-formatter", ...LEGAL_AD_PATHS]),
+ scriptPaths: AD_SCRIPT_PATHS,
+ scriptPrefixes: AD_SCRIPT_PREFIXES,
</file context>
| const currentLang = normalizeLanguage(lang); | ||
| return ( | ||
| FAQ_CONTENT[currentLang] || FAQ_CONTENT[DEFAULT_LANGUAGE] || FAQ_CONTENT.en | ||
| REVIEWED_CONTENT_LOCALES[currentLang]?.faq || |
There was a problem hiding this comment.
P3: The new reviewed-locale branch makes every non-English FAQ_CONTENT array unreachable because generated data exists for all supported locales. Remove the obsolete localized arrays or make them a real fallback so stale duplicate content is not bundled and maintained.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ui/faq-content.js, line 2038:
<comment>The new reviewed-locale branch makes every non-English `FAQ_CONTENT` array unreachable because generated data exists for all supported locales. Remove the obsolete localized arrays or make them a real fallback so stale duplicate content is not bundled and maintained.</comment>
<file context>
@@ -2146,6 +2035,9 @@ const FAQ_CONTENT = {
const currentLang = normalizeLanguage(lang);
return (
- FAQ_CONTENT[currentLang] || FAQ_CONTENT[DEFAULT_LANGUAGE] || FAQ_CONTENT.en
+ REVIEWED_CONTENT_LOCALES[currentLang]?.faq ||
+ FAQ_CONTENT[currentLang] ||
+ FAQ_CONTENT[DEFAULT_LANGUAGE] ||
</file context>
| if (pageId === "about" && next.html) { | ||
| next.html = next.html.replace( | ||
| /(<div class="text-3xl font-bold[^>]*)(>)(?:45|47)(<\/div>)/, | ||
| `$1 data-public-tool-count$2${PRODUCTION_TOOL_COUNT}$3`, |
There was a problem hiding this comment.
P3: The About templates already include data-public-tool-count, but this fallback replacement inserts the same attribute again when the rendered count is 45. Preserve the captured opening tag without adding a second attribute, or remove this fallback now that the templates are registry-backed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ui/legal-content.js, line 3893:
<comment>The About templates already include `data-public-tool-count`, but this fallback replacement inserts the same attribute again when the rendered count is 45. Preserve the captured opening tag without adding a second attribute, or remove this fallback now that the templates are registry-backed.</comment>
<file context>
@@ -3906,5 +3874,29 @@ export function getLegalSections(pageId, lang = DEFAULT_LANGUAGE) {
+ if (pageId === "about" && next.html) {
+ next.html = next.html.replace(
+ /(<div class="text-3xl font-bold[^>]*)(>)(?:45|47)(<\/div>)/,
+ `$1 data-public-tool-count$2${PRODUCTION_TOOL_COUNT}$3`,
+ );
+ }
</file context>
Suggested change
| `$1 data-public-tool-count$2${PRODUCTION_TOOL_COUNT}$3`, | |
| `$1$2${PRODUCTION_TOOL_COUNT}$3`, |
| { | ||
| type: "fix", | ||
| text: "About lists 47 production tools; homepage and JSON Formatter ship original visible copy for the thin-content review", | ||
| text: `About derives its ${PRODUCTION_TOOL_COUNT} public-tool count from the production-visible registry`, |
There was a problem hiding this comment.
P3: Adding or removing a production tool later will silently change this already-published release note because PRODUCTION_TOOL_COUNT is computed from the current registry. Keep historical changelog entries stable by removing the interpolation or recording the release-time count.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/routes/changelog.js, line 35:
<comment>Adding or removing a production tool later will silently change this already-published release note because `PRODUCTION_TOOL_COUNT` is computed from the current registry. Keep historical changelog entries stable by removing the interpolation or recording the release-time count.</comment>
<file context>
@@ -31,7 +32,7 @@ const RELEASES = [
{
type: "fix",
- text: "About lists 47 production tools; homepage and JSON Formatter ship original visible copy for the thin-content review",
+ text: `About derives its ${PRODUCTION_TOOL_COUNT} public-tool count from the production-visible registry`,
},
{
</file context>
Suggested change
| text: `About derives its ${PRODUCTION_TOOL_COUNT} public-tool count from the production-visible registry`, | |
| text: "About derives its public-tool count from the production-visible registry", |
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fix(content): make guidance and policy factual. This is logical change 9/11 in the dependency-ordered product-audit release stack.
Refs #969
What changed
fe81d8b964327feac887cb0a1a6578cb9fe4c3f5; validated tree:52777f0a38508bf23e8b3717289719eaccf3bf57.Why
Replace unsupported and generic guidance with task-specific localized content and consistent advertising disclosures.
Verification
bun run buildpassed on this exact candidate tree.npm test -- --maxWorkers=2passed on this exact candidate tree.20 content routes returned 200 with one h1; localized legal, FAQ and article rendering checked; 90 localized broad articles and 126 FAQ entries preserve code/links/keys.
Final combined tree additionally passed 801 unit tests and all 293 Playwright tests with retries disabled, plus all 48 primary tool workflows at desktop and mobile. The exploratory Color Converter exact-HEX boundary remains a documented pre-existing defect; its runtime is unchanged by this stack.
Risk & rollback
Summary by cubic
Makes guidance and policy factual by replacing generic copy with task-specific localized content and aligning ad disclosures with the actual placement rules. Refs #969.
Changes
PRODUCTION_TOOL_COUNTinstead of hard-coded numbers.homeis reserved but unplaced until a call site exists.Rollout
Written for commit fe81d8b. Summary will update on new commits.