Repository navigation
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
aa1c0ce to
f68180a
Compare
[Low impact] The PR changes installation guidance without changing installation behavior. The PR should wait until the existing release-example test failure is fixed. Findings
Fix with agent prompt### Issue 1
docs/installation.md:undefined-1945
The new installer block makes `releaseConfiguration.test.ts` fail. That test expects every version-marked block to contain `rea-agents@${packageJson.version}`, but this block contains only `rea_version=6.8.0`, so the check finds no match. Extend the test to check both example forms against the package version rather than removing the release markers.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryThis PR clarifies how to choose a known release and review the installer before running it.
No new actionable findings or security issues were identified. Whether another PR supersedes this work could not be checked because GitHub access required authentication. Reviews (2) · Last reviewed commit: "docs: prefer pinned install commands and..." · Reviewed by Greptile |
| To review a version-pinned installer before running it, select the release | ||
| version, download its tagged script, and inspect the downloaded file: | ||
|
|
||
| <!-- x-release-please-start-version --> |
There was a problem hiding this comment.
The new installer block makes releaseConfiguration.test.ts fail. That test expects every version-marked block to contain rea-agents@${packageJson.version}, but this block contains only rea_version=6.8.0, so the check finds no match. Extend the test to check both example forms against the package version rather than removing the release markers.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/installation.md
Line: 1945
Comment:
**Release example test fails**
The new installer block makes `releaseConfiguration.test.ts` fail. That test expects every version-marked block to contain `rea-agents@${packageJson.version}`, but this block contains only `rea_version=6.8.0`, so the check finds no match. Extend the test to check both example forms against the package version rather than removing the release markers.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Show a version-pinned setup command before the @latest opt-in, keep the tagged install.sh review path under release-please version management, and note the same guidance in the English README. Website FAQ/get-started copy is left unchanged so complete translation catalogs are not forced into English fallback. Fixes morluto#3423
f68180a to
f27fefe
Compare
|
Hey @BeqaKs, thanks for putting this together, the docs update looks useful. What's left: there is 1 unresolved review thread from Greptile, about "Release example test fails". Greptile is at 4/5 on the current head, mainly over "Release example test fails", so addressing that thread should bump it. Once that's done I'm happy to merge it. |
Summary
Fixes #3423
Onboarding still presented
npx rea-agents@latest setupand themain-branchcurl … | bashinstaller without an adjacent pinned or reviewable alternative. This PR addresses the remaining item 1 of the issue; items 2–5 were verified as already aligned on currentmainand are not touched.Problem and expected behavior
docs/installation.mdintroduced@latestas the way to "request the latest release" without first showing a version-pinned command, and its tagged-installer example was hard-coded to6.7.0(current release is6.8.0) outside release-please management.website/public/get-started/index.htmlshowed only themaininstall.shpipe, and its primary setup command only@latest.website/public/faq/index.htmlandREADME.mddescribed@latestwith no pinned alternative.Expected: a pinned command is shown first,
@latestis described as an explicit opt-in that follows every publish, and the existing taggedrea-agents-<version>download-review-run path is linked where onboarding presents themainpipe.Change and scope
Documentation only.
docs/installation.md: package-runner section now showsnpx rea-agents@6.8.0 setup(insidex-release-pleasemarkers) before the@latestopt-in; the start-setup pointer explains unversioned/@latestresolution and@<version>; the curl wrapper section states themainpipe runs the branch at fetch time and points at the tagged download; the tagged example'srea_versionmoves to6.8.0under release-please markers so it tracks releases (docs/installation.mdis already a genericextra-filesentry).website/public/get-started/index.html: one sentence after the primary@latestsetup command, and one paragraph after themaincurl command linking to the tagged installer path.website/public/faq/index.html: one paragraph after the refresh/one-off guidance.README.md: one prose sentence (no new links or bash blocks, so translated README editions stay aligned).website/locales/*: regenerated withtranslations extract(three new source messages; the rest is reference line-number churn).Not included: rewriting every
@latestexample across guides/translations, or adding the website HTML to release-pleaseextra-files.Items 2–5 verified on
main(1ff5a65), unchangeddocs/javascript-runtime-observation.md:105anddocs/browser-observation.md:164already state REA does not sendDebugger.setSkipAllPausesor resume pauses.docs/browser-scenario-contract.md:39-55now describes duration/action/navigation timeouts versuscommand_timeout_ms; the "fixed provider-owned liveness limits" wording is gone.capture.limitsretained event budgets; no "no event caps" claim remains.docs/browser-observation.md(docs(browser): correct cache fingerprint completeness limits #3546).Contract and boundary impact
install.shand npm package resolution; docs only.docs/installation.md.Expected effects:
Evidence and regression coverage
Source-derived:
install.shaccepts--versionand--dry-run(install.sh:82-88); tagrea-agents-6.8.0exists upstream (git ls-remote);package.jsonversion is6.8.0;release-please-config.jsonlistsdocs/installation.mdas a generic extra file.website.py checkcould not render the sharing image locally (native Cairo missing); its HTML sync check reported 0 pages changed.Validation performed
npm run docs:check— passes (includingreadmeConsistency.test.ts, 9 tests)python3 scripts/website.py translations extract— run;website.py check— shared HTML clean, sharing image skipped (no Cairo)npx prettier --checkon the four edited source files — passesgit diff --check— cleanCompatibility, safety, and release
docs/installation.mdWebsite scope
FAQ and get-started HTML are intentionally unchanged. Adding English-only onboarding paragraphs forced complete locale catalogs into
data-translation-fallbackand failed website CI. Pinned/@latestguidance lives indocs/installation.mdand a short English README note instead.Author readiness
Human owner: @BeqaKs
Before merge
type(scope): outcome.