Skip to content

done#185

Merged
Gaurav1998-dev merged 1 commit intomainfrom
pnpm-quickstart-improvement
Sep 13, 2025
Merged

done#185
Gaurav1998-dev merged 1 commit intomainfrom
pnpm-quickstart-improvement

Conversation

@Gaurav1998-dev
Copy link
Copy Markdown

No description provided.

@vercel
Copy link
Copy Markdown

vercel Bot commented Sep 13, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
agents-docs Building Building Preview Comment Sep 13, 2025 9:02pm
agents-manage-api Building Building Preview Comment Sep 13, 2025 9:02pm
agents-manage-ui Building Building Preview Comment Sep 13, 2025 9:02pm
agents-run-api Building Building Preview Comment Sep 13, 2025 9:02pm

@Gaurav1998-dev Gaurav1998-dev merged commit d9f809c into main Sep 13, 2025
2 of 7 checks passed
@Gaurav1998-dev Gaurav1998-dev deleted the pnpm-quickstart-improvement branch September 13, 2025 21:02
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Sep 13, 2025

Claude finished @Gaurav1998-dev's task —— View job


PR Review Analysis

Analyzing the merged PR for PNPM quickstart improvements...

Todo List:

  • Read and analyze the changed documentation file
  • Evaluate documentation quality and accuracy
  • Review customer experience impact
  • Provide architectural feedback on documentation patterns
  • Check alignment with existing documentation standards

Review Analysis

I've analyzed PR #185 which creates a new quickstart documentation file. Here's my comprehensive review:

🎯 PR Understanding & Context

This PR adds a new quickstart guide (agents-docs/content/docs/quick-start.mdx) to help users get started with the Inkeep multi-agent framework locally. The documentation references the @inkeep/create-agents CLI tool and includes setup instructions.

⚠️ Critical Issues Found

1. Broken Documentation Reference

  • Severity: High
  • Issue: The documentation references weather.graph.ts in Step 4, but this file doesn't exist in the repository structure created by @inkeep/create-agents
  • Evidence: After searching the entire codebase, weather.graph.ts only exists in the create-agents utility template at /packages/create-agents/src/utils.ts:785
  • Impact: Users following the quickstart will encounter a file-not-found error, breaking their onboarding experience
  • Recommendation: Update the documentation to reflect the actual project structure created by the CLI tool

2. Inconsistent Step Sequencing

  • Severity: Medium
  • Issue: The workflow assumes users install the CLI globally (Step 3) after creating a project (Step 1), but this creates dependency issues
  • Better Flow: Install CLI first, then create project, or clarify that the project template includes the CLI
  • Impact: May cause confusion during setup

📋 Documentation Quality Issues

3. Missing Context and Prerequisites

  • Severity: Medium
  • No Node.js version requirements mentioned (the create-agents package requires Node ≥22.0.0)
  • No explanation of what "agents project" means for new users
  • Missing troubleshooting section for common setup issues

4. Incomplete Instructions

  • Severity: Medium
  • Step 2 says "Start the development server" but doesn't explain what this accomplishes
  • Step 6 is vague about "the project you created" - lacks specific navigation guidance
  • No verification steps to confirm successful setup

🔧 Technical Accuracy Concerns

5. Command Validation Issues

  • Severity: Medium
  • The cd src/my-project path in Step 4 assumes a specific project structure that may not match what @inkeep/create-agents actually creates
  • Should verify against the actual CLI behavior and generated directory structure

6. Port and Service Assumptions

  • Severity: Low
  • Documentation doesn't mention which ports the services run on or how to access them
  • Step 5 runs inkeep dev but doesn't explain what this starts or where to access it

🎨 User Experience & Design

7. Missing Progressive Disclosure

  • Severity: Low
  • All steps are presented at once without explanation of the overall workflow
  • Would benefit from a brief "What you'll build" section
  • No estimated time to completion

8. Inconsistent with Repository Standards

  • Severity: Low
  • The CLAUDE.md file shows comprehensive development commands, but the quickstart doesn't leverage this existing documentation
  • Missing integration with the existing pnpm dev, pnpm build, etc. commands shown in CLAUDE.md

🚀 Recommendations

Immediate Fixes (High Priority):

  1. Verify and fix the file paths - Ensure weather.graph.ts exists where documented or update the path
  2. Test the entire workflow end-to-end - Run through each step to identify broken commands
  3. Add prerequisites section with Node.js version requirements

Enhancements (Medium Priority):

  1. Reorganize step sequence for logical dependency flow
  2. Add verification steps after each major milestone
  3. Include expected outputs so users know they're on track
  4. Add port information and service access details

Future Improvements (Low Priority):

  1. Add troubleshooting section for common issues
  2. Create "What you'll build" overview
  3. Link to deeper documentation for advanced users

🏗️ Architectural Considerations

Documentation Architecture:

  • This quickstart should integrate better with the existing documentation ecosystem
  • Consider creating reusable components for common setup patterns
  • The documentation structure should match the actual CLI-generated project structure

Customer Journey:

  • The documentation assumes too much prior knowledge
  • Would benefit from progressive disclosure of complexity
  • Should include success indicators and next steps

Overall Assessment

While this PR adds valuable quickstart documentation, it contains critical accuracy issues that will break the user experience. The primary concern is the mismatch between documented paths/files and the actual CLI tool behavior. This needs immediate attention before users encounter failed onboarding experiences.

The documentation would benefit from end-to-end testing and better alignment with the existing development workflow established in CLAUDE.md.

inkeep-oss-sync Bot pushed a commit that referenced this pull request Apr 22, 2026
…ded (#194)

* fix(ci): always reset copybara/sync on every mirror run

Closes #188

Drop the "leave branch in place if open PR is < STALE_PR_HOURS" branch in
the mirror sync reconcile step. Letting Copybara "append" to an existing
copybara/sync was never safe: the Copybara config uses fetch=main, so
every run baselines off inkeep/agents main's last GitOrigin-RevId. When
a new push lands on agents-private main while a prior sync PR is still
open, Copybara rebuilds the older origin change from main's HEAD (new
SHA due to timestamps) and the non-force push to copybara/sync is
rejected as non-fast-forward. This is the failure mode that blew up
the release cascade in #188 (Version Packages #185 merged while #3166
was still open 9 minutes after being created).

Every mirror run now closes any open sync PR and deletes copybara/sync
before Copybara runs, so each run pushes a fresh history. The concurrency
group already serializes runs and every new run includes all accumulated
changes since the last imported revision, so no information is lost.
PR churn (one inkeep/agents sync PR per agents-private main push) is the
cost, and it is much cheaper than a stuck release cascade.

CI_RUNBOOK gets a new entry for this specific failure string so future
red runs route to the fix without a re-investigation.

* fix(ci): harden release cascade against silent strandings

Bundled on top of the copybara/sync reset in this PR so the whole
release path (mirror sync -> npm publish -> GH Release -> Vercel
prod deploy -> scheduler restart) can run end-to-end with no human
intervention. Each fix closes a distinct silent-stranding mode.

1. public-mirror-sync.yml Create-PR guard
   - Reconcile now always deletes copybara/sync before Copybara runs,
     which introduced a regression: when Copybara exits 4 (no changes
     to sync, eg. workflow_dispatch with an idle main), the branch is
     gone and the next `gh pr create --head copybara/sync` would fail.
     Add an explicit branch-existence check; short-circuit cleanly.
   - Add explicit --state open to the gh pr list call. Defaults to open
     but being explicit prevents a future refactor from reintroducing
     the PR #184 bug class.
   - Replace the PR number extraction `grep -o '[0-9]*$'` on the PR URL
     with gh pr view --json number. gh's stdout format is not a contract.

2. private-agents-ui-version-packages.yml publish detection
   - Was parsing `Publishing "X" at "Y"` via grep/sed on the changesets
     log, which is the exact fragility PR #174 removed from public
     release.yml. If changesets v2 changes format, published=false is
     written despite a successful publish, the widget-release dispatch
     is skipped, and agents-docs changelog silently desyncs.
   - Use the stable "packages published successfully" presence marker
     and read the version from package.json (authoritative for a fixed
     release group).

3. public/agents/.github/workflows/release.yml catch-all + dispatch retry
   - `Notify agents-private (failure)` was gated on
     `steps.detect.outputs.has_changesets == 'false'`. If the workflow
     failed before the detect step ran (install, build, token gen),
     has_changesets is unset and the condition evaluated false -> no
     dispatch, no tracking issue on agents-private, red run sitting
     invisibly in the Actions tab. Drop the has_changesets gate.
   - Replace peter-evans/repository-dispatch with a bash retry loop
     (3 attempts, 5/10s backoff). The action has no built-in retry, so
     a transient 5xx or rate-limit during the post-publish dispatch
     loses the signal permanently: npm publishes, but no GH Release is
     created and no Vercel prod deploy fires. Retry + explicit error
     on exhausted attempts so the stranding is loud, not silent.

4. public-agents-vercel-production.yml concurrency + failure tracker
   - Add concurrency: vercel-production-deploy. DB migrations are not
     idempotent; two parallel deploys (eg. a release published while a
     manual re-dispatch is in flight) would race on migrate-databases
     and leave schema in a half-applied state.
   - Add notify-on-failure job (mirrors the tracking-issue pattern
     from public-mirror-sync.yml). At this point npm has published,
     the GH Release exists, but prod runtime is stale. Needs to be
     loud: auto-open a "Vercel production deploy failing" issue so
     the half-shipped state is visible instead of buried in the
     Actions tab.

CI_RUNBOOK.md: reword the release/publish failure entries to match
the new retry/tracking behavior, and add a new entry covering the
post-publish deploy failure case.

Intentionally out of scope: the auto-format.yml + Dependabot
`pnpm install --frozen-lockfile` race. Not a release-cascade issue,
will go in a separate PR.

* docs(runbook): bold Historical marker for consistency

GitOrigin-RevId: 04ff8b544833e109b57f75ded3236730d7fb10eb
github-merge-queue Bot pushed a commit that referenced this pull request Apr 22, 2026
* Version Packages (agents) (#185)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
GitOrigin-RevId: 7263142a67ac9ce9c9873a68a5673bfb436dbc1c

* chore(copilot-app): remove redundant lockfile, install from monorepo root (#186)

* chore(copilot-app): remove redundant lockfile, install from monorepo root

copilot-app is a workspace member (pnpm-workspace.yaml line 18), so the root
lockfile already resolves its dependencies. The second lockfile only existed
because vercel.json used pnpm install --ignore-workspace --frozen-lockfile,
which severs workspace context and therefore needed a local lockfile.

Two install boundaries for the same app meant root pnpm.overrides did not
apply to the Vercel install, so CI and Vercel could silently resolve to
different dependency trees. PR #167's description originally said "Vercel
to install + build from the monorepo root via pnpm --filter copilot-app...",
but the committed vercel.json drifted to --ignore-workspace. This aligns the
implementation with the stated plan.

- Delete private/copilot-app/pnpm-lock.yaml
- Change private/copilot-app/vercel.json installCommand to install from the
  monorepo root with a workspace filter
- Drop the copilot-app entry from scripts/check-monorepo-traps.mjs and simplify
  the DUAL_LOCKFILE_ROOTS comment (every remaining entry is a true workspace
  boundary, so the ignoreWorkspace workaround is no longer needed for any of
  them)

* docs(private): update lockfile section after copilot-app cleanup

* chore: add install:all convenience script for dual-lockfile installs

* chore: include create-agents-template in install:all

* fix(copilot-app): drop redundant cd ../.. from vercel installCommand

* docs: point dual-lockfile guidance at pnpm install:all

This PR introduces the install:all script; update every doc that teaches
the old cd-and-install-twice pattern to reference the shorthand instead.

- AGENTS.md (root) Dual lockfiles section: replaces the two-step pnpm
  install invocation with a single install:all, and lists all three
  lockfile scopes (root, public/agents, public/agents/create-agents-template)
  so readers understand what the shorthand covers.
- CI_RUNBOOK ERR_PNPM_OUTDATED_LOCKFILE: same substitution plus the third
  lockfile in the git add line.
- public/agents/AGENTS.md pnpm-lock.yaml Resolution Strategy: adds a
  When changing dependencies callout pointing at install:all, so readers
  inside the public/agents subtree know they have a root shortcut for
  the whole-monorepo regeneration.

* chore(check-monorepo-traps): drop dead ignoreWorkspace flag

Every DUAL_LOCKFILE_ROOTS entry is now a true workspace boundary that
installs without --ignore-workspace. The flag had exactly one live
consumer (private/copilot-app) which this PR removes. Simplify the data
structure to an array of path strings and drop the now-unused flag
branches in the install command and regen hint.

Also: the regen hint gains a pointer at the install:all shorthand, since
that's the recommended path for a whole-monorepo resync.

* docs: comprehensive command cheatsheet + check:structural aggregate

The problem: every time a new shorthand is added (install:all, check:*)
it lands in code but stays invisible in docs. People default to the raw
cd-and-install form, which is how we drift. The cheatsheet is the fix
for the drift-by-ignorance path.

Changes:
- Adds check:structural to root package.json - one command for the full
  structural guard set (boundaries + monorepo-traps + release-groups
  validate). Complements the existing pre-push hook which only runs
  check:monorepo-traps.
- Rewrites AGENTS.md 'Command routing' section as 'Command cheatsheet'
  with a scenario-driven quick-lookup table at top, then grouped by
  intent: install/lockfiles, build+dev+lint+typecheck+test, structural
  guards, changesets+releases, mirror/Copybara, parity, database.
- Documents the suffix convention (:agents, :agents-ui, :chat-to-edit,
  :inkeep-cloud-mcp, :copilot, :ext; no suffix = fan-out) so people
  can guess commands instead of memorizing.
- Every command gets a one-line description of what it does and when
  to reach for it.

* fix(check-monorepo-traps): guard the create-agents-template lockfile too

Docs introduced in this PR call out three lockfiles (root, public/agents,
public/agents/create-agents-template) and point at install:all as the
shorthand that regenerates them. The check only validated two — the
starter-kit lockfile could drift silently and slip past the pre-push
hook, surfacing for end users later when they cloned the starter.

Add public/agents/create-agents-template to DUAL_LOCKFILE_ROOTS and
update the comment to reflect the actual install-boundary taxonomy
(monorepo / Copybara+Vercel / standalone starter). install:all and the
check now cover the same set.

* ci: gate publish on check:structural (defense-in-depth)

Required checks on the source PR already run check:structural, and both
version-packages workflows check out origin/main before doing anything.
In practice, publish always runs against a validated main state.

But 'in practice' isn't the same as 'structurally'. A workflow_dispatch
run against main, an admin bypass of branch protection, or a future
change that loosens merge requirements could let a misconfigured main
reach the publish step without re-validation. Today's agents-ui release
already surfaced one post-publish pipefail bug that shouldn't have been
possible if we trusted the pipeline - this gate is the same intuition
applied upstream.

Adds 'Validate structural invariants' step between Install and the
release machinery in both private-agents-ui-version-packages.yml and
public-agents-version-packages.yml. Runs pnpm check:structural, which
aggregates check:boundaries + check:monorepo-traps + release-groups:validate
(including the workspace-isolation guard introduced in #191). Fails
hard on any structural misconfig, refusing to publish.

Cost: ~30-60s per publish run. Cheaper than a bad release.
GitOrigin-RevId: 684d52e5ab7734f592479b61e972cdfe5fc3ae23

* fix(ci): harden release cascade so copybara + npm publish run unattended (#194)

* fix(ci): always reset copybara/sync on every mirror run

Closes #188

Drop the "leave branch in place if open PR is < STALE_PR_HOURS" branch in
the mirror sync reconcile step. Letting Copybara "append" to an existing
copybara/sync was never safe: the Copybara config uses fetch=main, so
every run baselines off inkeep/agents main's last GitOrigin-RevId. When
a new push lands on agents-private main while a prior sync PR is still
open, Copybara rebuilds the older origin change from main's HEAD (new
SHA due to timestamps) and the non-force push to copybara/sync is
rejected as non-fast-forward. This is the failure mode that blew up
the release cascade in #188 (Version Packages #185 merged while #3166
was still open 9 minutes after being created).

Every mirror run now closes any open sync PR and deletes copybara/sync
before Copybara runs, so each run pushes a fresh history. The concurrency
group already serializes runs and every new run includes all accumulated
changes since the last imported revision, so no information is lost.
PR churn (one inkeep/agents sync PR per agents-private main push) is the
cost, and it is much cheaper than a stuck release cascade.

CI_RUNBOOK gets a new entry for this specific failure string so future
red runs route to the fix without a re-investigation.

* fix(ci): harden release cascade against silent strandings

Bundled on top of the copybara/sync reset in this PR so the whole
release path (mirror sync -> npm publish -> GH Release -> Vercel
prod deploy -> scheduler restart) can run end-to-end with no human
intervention. Each fix closes a distinct silent-stranding mode.

1. public-mirror-sync.yml Create-PR guard
   - Reconcile now always deletes copybara/sync before Copybara runs,
     which introduced a regression: when Copybara exits 4 (no changes
     to sync, eg. workflow_dispatch with an idle main), the branch is
     gone and the next `gh pr create --head copybara/sync` would fail.
     Add an explicit branch-existence check; short-circuit cleanly.
   - Add explicit --state open to the gh pr list call. Defaults to open
     but being explicit prevents a future refactor from reintroducing
     the PR #184 bug class.
   - Replace the PR number extraction `grep -o '[0-9]*$'` on the PR URL
     with gh pr view --json number. gh's stdout format is not a contract.

2. private-agents-ui-version-packages.yml publish detection
   - Was parsing `Publishing "X" at "Y"` via grep/sed on the changesets
     log, which is the exact fragility PR #174 removed from public
     release.yml. If changesets v2 changes format, published=false is
     written despite a successful publish, the widget-release dispatch
     is skipped, and agents-docs changelog silently desyncs.
   - Use the stable "packages published successfully" presence marker
     and read the version from package.json (authoritative for a fixed
     release group).

3. public/agents/.github/workflows/release.yml catch-all + dispatch retry
   - `Notify agents-private (failure)` was gated on
     `steps.detect.outputs.has_changesets == 'false'`. If the workflow
     failed before the detect step ran (install, build, token gen),
     has_changesets is unset and the condition evaluated false -> no
     dispatch, no tracking issue on agents-private, red run sitting
     invisibly in the Actions tab. Drop the has_changesets gate.
   - Replace peter-evans/repository-dispatch with a bash retry loop
     (3 attempts, 5/10s backoff). The action has no built-in retry, so
     a transient 5xx or rate-limit during the post-publish dispatch
     loses the signal permanently: npm publishes, but no GH Release is
     created and no Vercel prod deploy fires. Retry + explicit error
     on exhausted attempts so the stranding is loud, not silent.

4. public-agents-vercel-production.yml concurrency + failure tracker
   - Add concurrency: vercel-production-deploy. DB migrations are not
     idempotent; two parallel deploys (eg. a release published while a
     manual re-dispatch is in flight) would race on migrate-databases
     and leave schema in a half-applied state.
   - Add notify-on-failure job (mirrors the tracking-issue pattern
     from public-mirror-sync.yml). At this point npm has published,
     the GH Release exists, but prod runtime is stale. Needs to be
     loud: auto-open a "Vercel production deploy failing" issue so
     the half-shipped state is visible instead of buried in the
     Actions tab.

CI_RUNBOOK.md: reword the release/publish failure entries to match
the new retry/tracking behavior, and add a new entry covering the
post-publish deploy failure case.

Intentionally out of scope: the auto-format.yml + Dependabot
`pnpm install --frozen-lockfile` race. Not a release-cascade issue,
will go in a separate PR.

* docs(runbook): bold Historical marker for consistency

GitOrigin-RevId: 04ff8b544833e109b57f75ded3236730d7fb10eb

---------

Co-authored-by: Varun Varahabhotla <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants