Initial core package commit - #2
Conversation
There was a problem hiding this comment.
Hello, I'm 🤖 Stormie-Bot! The Storm team sincerely appreciates your effort/interest in contributing to this project. A Storm developer will review this change and get back to you ASAP. Please feel free to reach out to the Storm team (contact@stormsoftware.com) if you have any questions/comments.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adds the initial @sourcebook/core package with pipeline execution, document models, checkpointing, and Markdown rendering.
Changes:
- Adds core orchestration, schemas, retries, checkpoints, and rendering.
- Integrates Nx, TypeScript, Vitest, and workspace dependencies.
- Adds tests, documentation, exports, and agent skill metadata.
Reviewed changes
Copilot reviewed 20 out of 23 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Reviewed change | Final findings |
|---|---|---|
tsconfig.json |
References the core package. | — |
skills-lock.json |
Records the added skill. | — |
pnpm-workspace.yaml |
Adds workspace catalog dependencies. | — |
pnpm-lock.yaml |
Locks dependency updates. | — |
packages/core/vitest.config.mts |
Configures Vitest. | — |
packages/core/tsconfig.spec.json |
Configures test compilation. | — |
packages/core/tsconfig.lib.json |
Configures library compilation. | — |
packages/core/tsconfig.json |
Defines project references. | — |
packages/core/src/lib/pipeline.ts |
Implements pipeline execution and retries. | Critical (3 votes): path for observer exceptions can trigger retries and queue closure can be left pending. Moderate (2 votes): reject non-finite delays; provide fallback error messages. Moderate (2 votes): publish pipeline-failed through the callback. |
packages/core/src/lib/pipeline.spec.ts |
Tests pipeline behavior. | — |
packages/core/src/lib/model.ts |
Defines document schemas and types. | Moderate (3 votes): reject citations with endLine but no startLine. |
packages/core/src/lib/markdown.ts |
Renders Markdown artifacts. | Critical (3 votes): validate both path components against traversal, including Windows separators. Moderate (2 votes): safely serialize frontmatter keys. Moderate (3 votes): select a fence longer than embedded backtick runs. |
packages/core/src/lib/fake-agent.ts |
Provides a test agent stage. | — |
packages/core/src/lib/checkpoint.ts |
Provides checkpoint storage. | — |
packages/core/src/index.ts |
Exports the core API. | — |
packages/core/README.md |
Documents core usage. | — |
packages/core/project.json |
Defines the Nx project. | — |
packages/core/package.json |
Defines package metadata and dependencies. | — |
package.json |
Adds workspace development dependencies. | — |
nx.json |
Configures Nx plugins. | — |
.gitignore |
Ignores Vitest timestamp files. | — |
.agents/skills/grill-me/SKILL.md |
Adds skill metadata. | — |
.agents/skills/grill-me/agents/openai.yaml |
Configures skill presentation. | — |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (4)
nx.json:28
- These
include/excludeglobs are one level too deep for this project: Nx matches them against project roots, and this root ispackages/core, not a child under it. The first TypeScript plugin therefore still processes core while this build-enabled plugin does not select it, sonx run-many --target=buildcan omit the package. Use the project root in both filters.
{
"plugin": "@nx/js/typescript",
"include": ["packages/core/*"],
packages/core/src/lib/markdown.ts:60
- A page can legally contain
frontmatter.title, and the spread after the dedicatedtitlefield overwritespage.titlein the canonical output. This makes the required page title silently non-authoritative; merge custom frontmatter first and emit the page's canonical fields afterward.
const values = {
title: page.title,
...(page.description ? { description: page.description } : {}),
...page.frontmatter
};
packages/core/src/lib/markdown.ts:95
- Link fields accept arbitrary strings and are interpolated directly into Markdown. A
javascript:destination, or a label/destination containing Markdown delimiters, is emitted unchanged and can become an unsafe link or malformed output in a downstream renderer. Validate allowed protocols and escape/link-encode both values before rendering.
content = block.links
.map(link => `- [${link.label}](${link.url})`)
packages/core/src/lib/pipeline.ts:165
stableValueis meant to make checkpoint input hashes deterministic, butlocaleCompareuses the runtime's default locale/collation. A persisted checkpoint can therefore hash differently on hosts with different ICU/locale settings and be needlessly skipped. Use a locale-independent code-unit comparator for the key sort.
.sort(([left], [right]) => left.localeCompare(right))
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const lines = Object.entries(values).map(([key, value]) => { | ||
| if (Array.isArray(value)) { | ||
| return `${key}: [${value.map(serializeScalar).join(", ")}]`; | ||
| } | ||
| return `${key}: ${serializeScalar(value)}`; |
| const normalizedSlug = slug.replace(/^\/+|\/+$/g, ""); | ||
| if (!normalizedSlug || normalizedSlug.split("/").includes("..")) { | ||
| throw new TypeError(`Invalid document slug: ${slug}`); | ||
| } | ||
| const normalizedDirectory = directory.replace(/^\/+|\/+$/g, ""); |
| case "code": | ||
| content = `${block.title ? `**${block.title}**\n\n` : ""}\`\`\`${block.language ?? ""}\n${block.code}\n\`\`\``; | ||
| break; |
| .refine( | ||
| citation => | ||
| citation.startLine === undefined || | ||
| citation.endLine === undefined || | ||
| citation.endLine >= citation.startLine, | ||
| "Citation endLine must be greater than or equal to startLine" | ||
| ); |
| if (policy?.delayMs !== undefined && policy.delayMs < 0) { | ||
| throw new TypeError(`${location} delayMs cannot be negative`); | ||
| } |
| const publish = (event: PipelineEvent) => { | ||
| queue.push(event); | ||
| options.onEvent?.(event); |
| severity: "error", | ||
| code: "STAGE_EXECUTION_FAILED", | ||
| message: lastError.message, | ||
| stageId: stage.id, |
| : new PipelineExecutionError("Pipeline execution failed", job, { | ||
| cause: asError(error) | ||
| }); | ||
| queue.push({ |
No description provided.