feat(package-quality): Wave 2a observability host package quality#10
Conversation
Reviewer-seeded Research+Plan staging for the Wave 2 Integration adapters wave (logger, telemetry, aspire, kv, database, prisma-adapter-mysql, queue, cron). Structural re-baseline against feat/package-quality @ 4c57867; dynamic gates marked MEASURE-FIRST; Wave 1 review lessons folded in.
- Re-baseline all 8 A2 units with real deno publish --dry-run and deno doc --lint counts. Log deltas in drift.md. - Resolve OQ-1..OQ-7: split Wave 2 into 2a/2b/2c sub-waves; interfaces/->ports/ renames; drop aspire ./helpers; add ./testing entrypoints; remove skipLibCheck; publish hygiene. - plan.md: locked decisions, full A2 gate set, risk register, debt implications, 10+23+17 ordered commit slices. - worklog.md: per-unit port shape, adapters, composition roots, permissions, consumer-import impact, contributor path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…+ PLAN-EVAL PASS docs)
Commit Summary:
|
Commit Summary:
|
Commit Summary:
|
Commit Summary:
|
Commit Summary:
|
Commit Summary:
|
|
Use harness, run evaluator pass of this implementation. If surface of corrections is not too consequent proceed to fix them first then give final verdict. |
Commit df5be37 — fix(telemetry): use type-only imports for lintScope
Changes
Gate Evidence
Notes
|
Ran the IMPL-EVAL (final evaluator pass) for Sub-wave 2a (logger · telemetry · aspire). Installed deno 2.8.2 and independently re-ran every static + fitness gate. Verdict: Gates re-verified independently:
Small corrections were within reach, so I fixed them before the verdict (per your instruction):
(telemetry's two Note: this verdict covers 2a only — 2b (data) and 2c (messaging) are separate branches/PRs with their own Plan-Gate and IMPL-EVAL passes. |
Commit 32d8894 — fix(aspire): pin @std/assert in tests for lint gateScope
Changes
Gate Evidence
Notes
|
Commit 665f49e — docs(harness): 2a IMPL-EVAL PASS verdict + refresh run artifactsScope
Changes
Gate Evidence
Notes
|
🤖 Augment PR SummarySummary: This PR completes Wave 2a “package quality” work for the observability/host packages (logger, telemetry, aspire), plus updates harness evidence and adds scoped tooling for targeted validation. Changes:
Technical Notes: Telemetry/aspire updates focus on doc-lint/publish-readiness without widening runtime behavior, and the new scoped tools emit JSON summaries to support harnessed validation. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| } | ||
|
|
||
| async function collectRoot(root: string, options: Options, output: Set<string>): Promise<void> { | ||
| const absolute = await Deno.realPath(root).catch(() => root); |
There was a problem hiding this comment.
At .llm/tools/run-deno-fmt.ts:194, --cwd is only applied to the deno fmt subprocess, but file discovery (Deno.realPath/Deno.stat/Deno.readDir) still resolves paths relative to the process CWD. This makes --cwd potentially select the wrong files (or fail to find them) when the caller expects roots/files to be interpreted from the provided working directory.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| } | ||
|
|
||
| async function collectRoot(root: string, options: Options, output: Set<string>): Promise<void> { | ||
| const absolute = await Deno.realPath(root).catch(() => root); |
There was a problem hiding this comment.
At .llm/tools/run-deno-lint.ts:214, --cwd is passed to the deno lint command but the file selection logic (Deno.realPath/Deno.stat/Deno.readDir) still resolves relative to the process CWD. If --cwd is used, this can lint an unintended file set or error out because roots/files are interpreted from a different directory than the lint subprocess.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| "triggers-api", | ||
| "streams" | ||
| ], | ||
| "WatchDirs": ["//YOGA_BOOK_9I/netscript-incoming"], |
There was a problem hiding this comment.
At packages/aspire/tests/_fixtures/appsettings.json:197, the fixture includes a machine-specific WatchDirs value ("//YOGA_BOOK_9I/netscript-incoming"). Even if tests only parse this file, committing a host-specific path can leak local environment details and makes the fixture less portable for other contributors/CI.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| | string | ||
| | number | ||
| | boolean | ||
| | Array<string | null | undefined> |
There was a problem hiding this comment.
At packages/telemetry/src/core/types.ts:12-14, AttributeValue allows arrays containing null/undefined, but OpenTelemetry attribute values are typically limited to primitive arrays (string/number/boolean). This public contract may encourage callers to send invalid attribute payloads that upstream OTEL drops or refuses to record, leading to silent telemetry loss.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| */ | ||
| export function contextWithSpan(span: Span, parentContext?: Context): Context { | ||
| return trace.setSpan(parentContext ?? context.active(), span); | ||
| return trace.setSpan(parentContext ?? context.active(), span as OtelSpan); |
There was a problem hiding this comment.
At packages/telemetry/src/context/helpers.ts:22, contextWithSpan casts the exported Span interface to an @opentelemetry/api span (span as OtelSpan) before calling trace.setSpan. Since Span is a public structural type, a consumer could pass a non-OTEL implementation that type-checks but yields an invalid OTEL context at runtime.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
@copilot fix all issues that are legitimate raised by augment then reply to the comment to summarize your fixes
|
| Tools: z.record(z.string(), ToolEntryZod).default({}), | ||
| }).meta({ title: 'NetScriptConfig', description: 'Root NetScript application configuration' }); | ||
| /** Root NetScript configuration schema. */ | ||
| export const NetScriptConfigSchema: AspireSchema<NetScriptConfig> = NetScriptConfigZod; |
There was a problem hiding this comment.
At packages/aspire/config.ts:487, exporting NetScriptConfigSchema (and the other *Schema constants) as the narrowed AspireSchema<...> type removes Zod object APIs like .extend() from the public type surface, so the README’s schema-composition example won’t type-check for consumers. Consider whether the published contract should preserve Zod’s schema-composition methods (or the docs should be updated to reflect the narrowed contract).
Severity: medium
Other Locations
packages/aspire/config.ts:511packages/aspire/README.md:169
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| ToolEntry as ToolEntryConfig, | ||
| } from './config.ts'; | ||
|
|
||
| // --- Base Types (z.infer<> derived) --- |
There was a problem hiding this comment.
At packages/aspire/types.ts:38, the section header and module docs still describe these as z.infer<>-derived types, but the file now aliases exported interfaces from config.ts instead. This mismatch can confuse consumers and makes it unclear whether the package intends schema-derived typing or explicit public contracts.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| } | ||
|
|
||
| function relativePath(cwd: string, path: string): string { | ||
| return normalizePath(path.startsWith(cwd) ? path.slice(cwd.length).replace(/^[/\\]/, '') : path); |
There was a problem hiding this comment.
At .llm/tools/run-deno-fmt.ts:175, relativePath() uses path.startsWith(cwd) without a path-separator boundary check, so a path like ${cwd}X/... would be incorrectly treated as inside cwd and sliced into an invalid relative path. That can lead to selecting/linting the wrong files in edge cases where directory names share prefixes.
Severity: low
Other Locations
.llm/tools/run-deno-lint.ts:195
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
Addressed the legitimate Augment findings in
Validation passed for targeted checks/tests, telemetry doc-lint + publish dry-run, Aspire lint/tests + publish dry-run, and scoped runner |
#8 #9 #10 #15) - reference/ai: explicit note that AI provider keys have no typed config surface; raw Deno.env.get is the supported path (#8). - reference/queue + reference/kv: See-it-live link blocks to the queue-kv-cron how-to and data-persistence concept page (#9). - reference/contracts + reference/sdk: showcase sentence per lede — contracts obviate manual req.json() validation; defineServices() wires the whole typed stack in one call (#10). - durable-workflows/streams: one sentence reframing no in-process subscribe() as an intentional single HTTP/SSE surface (#15). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VifzAytcxBk5odEMaCXihM
…-tos, auth example (#686) * docs: clarify PORT/typed-config relationship across 8 pages (#660 #7) Aspire injects PORT at runtime; netscript.config.ts services.<name>.port (apps.<name>.port for Fresh apps) is the typed source of truth the scaffold wires as the fallback default. One shared sentence at each first parseInt occurrence so the raw-env read no longer reads as the canonical pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VifzAytcxBk5odEMaCXihM * docs: reference showcase ledes, see-it-live links, streams framing (#660 #8 #9 #10 #15) - reference/ai: explicit note that AI provider keys have no typed config surface; raw Deno.env.get is the supported path (#8). - reference/queue + reference/kv: See-it-live link blocks to the queue-kv-cron how-to and data-persistence concept page (#9). - reference/contracts + reference/sdk: showcase sentence per lede — contracts obviate manual req.json() validation; defineServices() wires the whole typed stack in one call (#10). - durable-workflows/streams: one sentence reframing no in-process subscribe() as an intentional single HTTP/SSE surface (#15). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VifzAytcxBk5odEMaCXihM * docs: add runnable end-to-end blocks to three decision how-tos (#660 #11) discover-services, choose-a-queue-provider, and add-a-task-runtime-adapter each gain one copy-pasteable end-to-end block (resulting file + full command sequence) at deploy-local-aspire density, demonstrating the pluggability the pages previously only asserted. All commands/APIs grounded in package source. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VifzAytcxBk5odEMaCXihM * docs: add complete route-protection example to better-auth-plugins (#660 #12) A real Fresh routes/*.tsx handler gated on an active session and an admin role, at sibling auth.md's depth: resolves the session via backend.sessions.getSession, maps to a Principal with principalMapper, and fails closed on both checks. Grounded in packages/auth-better-auth + @netscript/service/auth exports. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VifzAytcxBk5odEMaCXihM * chore(run): docs-660b sweep worklog (#660) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VifzAytcxBk5odEMaCXihM --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Status
Sub-wave 2a (observability/host) is complete and has passed its IMPL-EVAL (final evaluator) pass with verdict
PASS. Scope coverspackages/logger,packages/telemetry, andpackages/aspire.Slices
/docs, package task hygiene, runnable docs doctests, and README ≥150 lines../helpersexport removal (no shim), and publish dry-run verification../helpersremoval has zero consumers acrosspackages/andplugins/.Evidence
Independently re-run with deno 2.8.2 during IMPL-EVAL:
deno publish --dry-run --allow-dirtyendedSuccess Dry run completewith 0 slow types for all three packages.deno checkclean on all entrypoints; tests pass (logger 11, telemetry 12, aspire 18/49 steps).deno lintanddeno fmt --checkclean for all three packages./docspresent for each package.IMPL-EVAL Corrections
Small corrections surfaced by the evaluator pass were fixed in place:
@std/assertimport specifier in two runtime tests to satisfydeno lint(no-unversioned-import).aspire-helpers-subpath-shimarch-debt entry with evidence now that./helpersis removed.worklog.md,commits.md,drift.md) to record slices 4-9 and the resolved telemetry escalation.verbatim-module-syntaxlint errors were resolved upstream indf5be37.The verdict is recorded in
.llm/tmp/run/feat-package-quality-wave2-adapters--adapters/evaluate.md. This verdict scopes to 2a only — sub-waves 2b (data) and 2c (messaging) are separate branches/PRs with their own Plan-Gate and IMPL-EVAL passes.