-
Notifications
You must be signed in to change notification settings - Fork 13
docs: Update AGENTS.md #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Completed Working on "Auto PR Description"✅ Workflow completed successfully. |
Completed Working on "Code Review"✅ Workflow completed successfully. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review summary
- Findings: 0 BLOCKER, 0 CRITICAL, 2 MAJOR, 1 MINOR
- Themes: (1) Repository guide overstates existing instrumentation test coverage; (2) Release workflows lack explicit documentation of required secrets for semantic-release and prerelease automation.
- Next steps: Update AGENTS.md to accurately describe which instrumentation specs exist (or add the missing tests) and clearly document the required
GITHUB_TOKEN/NPM_TOKENsecrets for both release and releaseBeta workflows so future agents configure CI correctly.
| - `src/`: Library entry points, module wiring, SDK helper exports, constants, and public API surface (re-exported via `src/index.ts`). | ||
| - `src/trace/`: Core tracing utilities: | ||
| - `decorators/` (e.g., `span.ts`, `traceable.ts`) implement `@Span`/`@Traceable`. | ||
| - `instrumentation/` houses all auto-instrumentation classes such as `controller.instrumentation.ts`, `guard.instrumentation.ts`, and shared logic in `base-trace.instrumentation.ts`. Each `.ts` file has a sibling `.spec.ts`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[MAJOR]: The Repository Structure section states that every file under src/trace/instrumentation/ has a matching .spec.ts, but the current tree disproves it (e.g., console-logger.instrumentation.ts and Instrumentation.ts have no specs). This misrepresents actual coverage expectations and will mislead contributors trying to mirror the documented layout. Suggested fix: either document the known exceptions / future intent or add the missing spec files so the guidance matches reality.
| ## CI/CD Expectations | ||
| - **`ci.yml`**: Executes on PRs and non-main pushes. Steps: `npm ci` → `npm run lint` → `npm run test:cov` → `npm run build`. Agents must replicate this order locally before opening PRs. | ||
| - **`release.yml`**: Runs on pushes to `main`. After lint/test/build, it invokes `npx semantic-release` with required tokens to publish to npm and create GitHub releases. Any breaking-change commits must follow conventional commit semantics for semantic-release to version correctly. | ||
| - **`releaseBeta.yml`**: Manual (`workflow_dispatch`) prerelease flow. Accepts a semver input (`1.0.0-beta.1`), sets that version, and publishes with the `beta` tag. Coordinate with maintainers before triggering. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[MAJOR]: The prerelease workflow section says releaseBeta.yml publishes beta builds but never states it relies on ${{ secrets.NPM_TOKEN }} for authentication. Without naming the secret, agents can trigger the workflow only to have it fail or rotate the wrong credential. Suggested fix: explicitly call out the NPM_TOKEN requirement in this section so workflow prerequisites stay in sync with the file.
|
|
||
| ## CI/CD Expectations | ||
| - **`ci.yml`**: Executes on PRs and non-main pushes. Steps: `npm ci` → `npm run lint` → `npm run test:cov` → `npm run build`. Agents must replicate this order locally before opening PRs. | ||
| - **`release.yml`**: Runs on pushes to `main`. After lint/test/build, it invokes `npx semantic-release` with required tokens to publish to npm and create GitHub releases. Any breaking-change commits must follow conventional commit semantics for semantic-release to version correctly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[MINOR]: The release.yml description says semantic-release runs with “required tokens” but never names them. The workflow injects ${{ secrets.GITHUB_TOKEN }} and ${{ secrets.NPM_TOKEN }}, and leaving the exact names out makes it harder to provision or rotate the correct secrets. Suggested fix: mention both secret names in this paragraph so future automation stays correctly configured.
Summary
This PR introduces a comprehensive AGENTS guide that documents the repository’s purpose, structure, tooling expectations, and workflows for automation contributors. It aligns future agent work by outlining coding patterns, CI/CD contracts, and documentation responsibilities.
Changes
AGENTS.mddetailing repository scope, directory structure, development tooling, coding/testing patterns, CI/CD workflows, common agent tasks, reference examples, and critical guardrails for contributors.Commits
Testing
AGENTS.mdrenders correctly on GitHub with proper headings, lists, and code blocks.