feat(workspace-primitives): add digest() and layoutManifest() exports across first-party adapters#93
Merged
Merged
Conversation
… across first-party adapters
Implements work items 2 and 3 from
AgentWorkforce/relayfile docs/workspace-primitives-spec.md (PR
relayfile#147), pairing with the relayfile implementation PR that
consumes these contracts.
Per-provider digest() handler:
packages/{linear,notion,github,slack,jira,confluence}/src/digest.ts
+ digest.test.ts
The handler signature is the spec's contract:
(ctx: DigestContext) => Promise<DigestSection | null>
where DigestContext/DigestSection/DigestHandler are defined locally in
each provider for M1. (Hoisting these into
@relayfile/adapter-core/digest-contract.ts is a follow-up; doing it now
would block this PR on a core-side export change that isn't required
for the relayfile digest generator to consume the per-provider results.)
Implementations:
- linear, notion, github: real handlers — sort by event time, render
past-tense bullets with canonical mount-relative paths, return null
on empty windows.
- slack, jira, confluence: M1 no-op returning null, consistent with
the spec's "others can no-op to null for M1" guidance.
Per-provider layoutManifest() / layout.ts:
packages/{linear,notion,github,slack,jira,confluence}/src/layout.ts
+ layout.test.ts
Each provider exports a layoutManifest() that the relayfile mount
daemon will call to render <mount>/<provider>/.layout.md as a virtual
file. Manifests describe the provider's top-level resource
directories, the canonical filename convention, the by-* alias
subtrees populated, and writeback subdirectories.
Barrel re-exports:
packages/{linear,notion,github,slack,jira,confluence}/src/index.ts
Each provider's public barrel re-exports digest and layoutManifest so
relayfile can import them through the standard adapter entry point.
Pairs with AgentWorkforce/relayfile PR for work items 1, 4, 5, 6
(digest generator + .schema.json virtual files + writeback list CLI +
dead-letter sidecars). Determinism tests in digest.test.ts assert that
two runs over the same fixture produce byte-identical DigestSection
outputs, satisfying WI 2 acceptance criterion 4.
Generated by the workspace-primitives implementation workflow run
382654597c96e23b0b1dfa39 (run from the relayfile worktree). Per-slice
fix-loop reports confirm typecheck + provider tests green
(linear 101/0, github 237/0, notion 106/0, slack 85/0, jira 42/0,
confluence 63/0).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds digest event processing and layout manifest discovery features across six provider adapters (Confluence, GitHub, Jira, Linear, Notion, Slack). GitHub, Linear, and Notion receive full digest implementations with event sorting and identifier formatting; other providers receive null stubs. All providers define layout manifests with materialization modes and writeback schema configurations. ChangesMulti-provider digest and layout infrastructure
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+90
to
+91
| if (/(open|opened|create|created|add|added|write|written)/u.test(action)) { | ||
| return 'was opened'; |
There was a problem hiding this comment.
🔴 Substring regex in GitHub pastTense misclassifies 'reopened' as 'was opened'
The pastTense regex /(open|opened|create|created|add|added|write|written)/u performs substring matching without word boundaries. When a GitHub event has action: "reopened" (a standard GitHub webhook action for issues and PRs), the regex matches "open" inside "reopened" and returns "was opened" instead of falling through to "was updated". This produces incorrect digest text — a reopened issue is reported as newly opened, which is semantically misleading.
Suggested change
| if (/(open|opened|create|created|add|added|write|written)/u.test(action)) { | |
| return 'was opened'; | |
| if (/\b(open|opened|create|created|add|added|write|written)\b/u.test(action)) { | |
| return 'was opened'; |
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+93
to
+94
| if (/(delete|deleted|remove|removed|archive|archived)/u.test(action)) { | ||
| return 'was archived'; |
There was a problem hiding this comment.
🔴 Substring regex in Notion pastTense inverts 'unarchived' to 'was archived'
The pastTense regex /(delete|deleted|remove|removed|archive|archived)/u performs substring matching without word boundaries. When a Notion event has action: "unarchived" (restoring an archived page), the regex matches "archive" inside "unarchived" and returns "was archived" — the semantic opposite of the actual action. This is a particularly severe inversion: the digest would report a restored page as archived.
Suggested change
| if (/(delete|deleted|remove|removed|archive|archived)/u.test(action)) { | |
| return 'was archived'; | |
| if (/\b(delete|deleted|remove|removed|archive|archived)\b/u.test(action)) { | |
| return 'was archived'; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| export const layoutManifest: LayoutManifestProvider = () => ({ | ||
| provider: 'github', | ||
| filenameConvention: '<number>__<slug>/meta.json', | ||
| aliasSegments: ['by-id', 'by-title'], |
There was a problem hiding this comment.
🟡 GitHub layout top-level aliasSegments missing 'by-name' used by repos resource
The GitHub layoutManifest declares top-level aliasSegments: ['by-id', 'by-title'] at packages/github/src/layout.ts:28, but the github/repos resource at line 34 uses aliasSegments: ['by-name']. The top-level array should be the union of all resource alias segments. Consumers that inspect only manifest.aliasSegments to discover available alias types will miss by-name, failing to discover that repositories can be looked up by name.
Suggested change
| aliasSegments: ['by-id', 'by-title'], | |
| aliasSegments: ['by-id', 'by-name', 'by-title'], |
Was this helpful? React with 👍 or 👎 to provide feedback.
| export const layoutManifest: LayoutManifestProvider = () => ({ | ||
| provider: 'linear', | ||
| filenameConvention: '<identifier>__<uuid>.json', | ||
| aliasSegments: ['by-id', 'by-title', 'by-state'], |
There was a problem hiding this comment.
🟡 Linear layout top-level aliasSegments missing 'by-name' used by teams resource
The Linear layoutManifest declares top-level aliasSegments: ['by-id', 'by-title', 'by-state'] at packages/linear/src/layout.ts:28, but the linear/teams resource at line 51 uses aliasSegments: ['by-id', 'by-name']. The top-level array should be the union of all resource alias segments. Consumers that inspect only manifest.aliasSegments to discover available alias types will miss by-name, failing to discover that teams can be looked up by name.
Suggested change
| aliasSegments: ['by-id', 'by-title', 'by-state'], | |
| aliasSegments: ['by-id', 'by-name', 'by-title', 'by-state'], |
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
packages/notion/src/digest.test.ts (1)
6-58: ⚡ Quick winAdd regression cases for timestamp-offset ordering and
/notionroot canonical paths.Current tests validate the happy path well, but they don’t guard the edge cases that can regress sorting/path eligibility behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/notion/src/digest.test.ts` around lines 6 - 58, Add regression tests to digest.test.ts to cover timestamp-offset ordering and canonicalPath roots that start with a leading slash: add a case where DigestContext.changeEvents returns events with equivalent timestamps expressed with different timezone offsets and/or identical timestamps but different ids to ensure digest() still deterministically sorts by timestamp then id (use the same pattern as the existing test that calls digest(ctx) twice and asserts equality), and add a case where events contain canonicalPath values beginning with '/notion/...' to ensure digest() treats those paths as eligible and normalizes them (output canonicalPath should match the existing expected format without the leading slash); reference the digest function and the DigestContext.changeEvents implementation to locate where to add these tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/confluence/src/digest.test.ts`:
- Around line 6-22: The test currently calls digest(ctx) once; to assert
determinism call digest(ctx) a second time with the same DigestContext and
assert the two results are equal (e.g. store await digest(ctx) in result1 and
result2 and use assert.deepEqual(result1, result2)); keep the existing assertion
(or replace it with an equality check that both runs return the expected null)
and reference the test name, DigestContext and digest(ctx) when making the
change.
In `@packages/confluence/src/layout.test.ts`:
- Around line 8-26: Add collision tests for each alias subtree declared by
layoutManifest: call layoutManifest(), iterate manifest.aliasSegments and for
each segment add a unit test (e.g., "alias collision for <segment>") that uses
the same normalization function used by the implementation (import the
normalize/alias-key helper the code uses, e.g., normalizeAlias or aliasKeyFrom)
to produce two different inputs that normalize to the same alias key, then
assert the manifest/resource behavior on collision (for
manifest.resources[*].aliasSegments and the corresponding alias subtree — e.g.,
check the produced alias key is identical and that lookups/registry expose the
expected single resolved entry or the defined collision resolution). Ensure
tests reference layoutManifest, manifest.resources, manifest.aliasSegments and
CANONICAL_ALIAS_SEGMENTS so each declared subtree has one collision test.
In `@packages/github/src/layout.test.ts`:
- Around line 8-27: Add a new test in the same file that uses layoutManifest()
to exercise alias-subtree collisions: iterate manifest.resources and for each
resource build the per-alias subtree key (using manifest.aliasSegments and
resource.aliasSegments) and assert there are no ambiguous/duplicate exposed
targets without a deterministic tie-breaker; specifically verify colliding alias
names (e.g., the documented case like by-title vs by-name) are resolved
consistently by checking that when two aliases map to the same subtree the
manifest always prefers the same alias according to manifest.aliasSegments
order. Use the existing symbols layoutManifest, manifest.aliasSegments,
resource.aliasSegments and CANONICAL_ALIAS_SEGMENTS to locate and validate alias
collision behavior and add assertions that collisions either do not occur or
resolve deterministically.
In `@packages/jira/src/layout.test.ts`:
- Around line 8-26: Add a test that, for each alias subtree declared in
layoutManifest().aliasSegments, verifies alias-collision behavior by
constructing two resources that would resolve to the same alias key under that
subtree and asserting the collision is detected/handled; update
packages/jira/src/layout.test.ts to iterate manifest.aliasSegments and for each
alias segment create two minimal resource entries (matching the existing
resource shape used by layoutManifest()/manifest.resources) with identical alias
values, then assert the system reports a collision (e.g., throws, returns an
error, or marks duplicates) for that alias segment while still validating
CANONICAL_ALIAS_SEGMENTS membership.
In `@packages/linear/src/layout.test.ts`:
- Around line 8-27: Add a unit test to layout.test.ts that covers alias-subtree
collisions: call layoutManifest (or construct a manifest variant) with two
resources that deliberately produce the same alias key for a given aliasSegment
(e.g., same "by-title" value), then assert the deterministic collision
resolution (for example, that manifest.aliasSegments include the segment, that
only one entry exists for the colliding alias key or that entries are
ordered/picked deterministically), and keep using the existing symbols:
layoutManifest, manifest.resources, aliasSegments and CANONICAL_ALIAS_SEGMENTS;
ensure the new test verifies the collision behavior for each alias subtree and
fails if resolution is non-deterministic.
In `@packages/notion/src/digest.ts`:
- Around line 55-59: The predicate function hasCanonicalPath incorrectly
excludes the exact "/notion" root; update the check in hasCanonicalPath to
accept both "notion" and "/notion" as well as child paths by treating
startsWith('/notion/') and startsWith('notion/') as valid — i.e., add an
explicit equality check for "/notion" (or normalize leading slash before testing
event.canonicalPath) so event.canonicalPath === '/notion' is accepted alongside
the existing conditions.
- Around line 62-67: The compareEvents function currently compares eventTime
strings lexicographically which can misorder ISO timestamps with different
offsets; update compareEvents to parse the eventTime values into numeric
timestamps (e.g., Date.parse or new Date(...).getTime()) and compare those
numeric values first (handling invalid/undefined times by treating as -Infinity
or similar), then fall back to the existing comparisons on left.id/right.id and
left.canonicalPath/right.canonicalPath; refer to compareEvents and eventTime to
locate and replace the string localeCompare logic with numeric timestamp
comparisons.
In `@packages/notion/src/layout.test.ts`:
- Around line 8-27: Update the test for layoutManifest to also assert
alias-subtree collision behavior: after calling layoutManifest() iterate
manifest.resources and for each alias in resource.aliasSegments (and for
alias-derived paths in resource.aliasSubtrees or however alias paths are
exposed) assert that any potentially colliding names produce distinct suffixed
aliases (e.g., check that when two resources would map to the same alias segment
the produced alias strings are different and include the expected collision
suffix), and add assertions that these collision-suffixed aliases still start
with 'notion/' and conform to CANONICAL_ALIAS_SEGMENTS rules; use the existing
symbols layoutManifest, manifest.resources, resource.aliasSegments,
CANONICAL_ALIAS_SEGMENTS and resource.writebackResources to locate where to add
the checks.
---
Nitpick comments:
In `@packages/notion/src/digest.test.ts`:
- Around line 6-58: Add regression tests to digest.test.ts to cover
timestamp-offset ordering and canonicalPath roots that start with a leading
slash: add a case where DigestContext.changeEvents returns events with
equivalent timestamps expressed with different timezone offsets and/or identical
timestamps but different ids to ensure digest() still deterministically sorts by
timestamp then id (use the same pattern as the existing test that calls
digest(ctx) twice and asserts equality), and add a case where events contain
canonicalPath values beginning with '/notion/...' to ensure digest() treats
those paths as eligible and normalizes them (output canonicalPath should match
the existing expected format without the leading slash); reference the digest
function and the DigestContext.changeEvents implementation to locate where to
add these tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b90b5d3d-027f-4eae-ac98-da244243036c
📒 Files selected for processing (30)
packages/confluence/src/digest.test.tspackages/confluence/src/digest.tspackages/confluence/src/index.tspackages/confluence/src/layout.test.tspackages/confluence/src/layout.tspackages/github/src/digest.test.tspackages/github/src/digest.tspackages/github/src/index.tspackages/github/src/layout.test.tspackages/github/src/layout.tspackages/jira/src/digest.test.tspackages/jira/src/digest.tspackages/jira/src/index.tspackages/jira/src/layout.test.tspackages/jira/src/layout.tspackages/linear/src/digest.test.tspackages/linear/src/digest.tspackages/linear/src/index.tspackages/linear/src/layout.test.tspackages/linear/src/layout.tspackages/notion/src/digest.test.tspackages/notion/src/digest.tspackages/notion/src/index.tspackages/notion/src/layout.test.tspackages/notion/src/layout.tspackages/slack/src/digest.test.tspackages/slack/src/digest.tspackages/slack/src/index.tspackages/slack/src/layout.test.tspackages/slack/src/layout.ts
Comment on lines
+8
to
+26
| test('layoutManifest exposes Confluence resources with canonical aliases and writeback schema pointers', () => { | ||
| const manifest = layoutManifest(); | ||
|
|
||
| assert.equal(manifest.provider, 'confluence'); | ||
| assert.deepEqual(manifest.aliasSegments, ['by-id', 'by-title', 'by-state']); | ||
|
|
||
| for (const resource of manifest.resources) { | ||
| assert.ok(resource.path.startsWith('confluence/')); | ||
| assert.doesNotMatch(resource.path, /^\//u); | ||
| for (const alias of resource.aliasSegments) { | ||
| assert.ok(CANONICAL_ALIAS_SEGMENTS.has(alias), `unexpected alias segment ${alias}`); | ||
| } | ||
| for (const writeback of resource.writebackResources) { | ||
| assert.ok(writeback.path.startsWith('confluence/')); | ||
| assert.doesNotMatch(writeback.path, /^\//u); | ||
| assert.match(writeback.schemaId, /^confluence\/[a-z-]+$/u); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
Add collision coverage for alias subtrees declared by the manifest.
This suite validates allowed alias segment names but does not verify collision behavior (e.g., two inputs normalizing to the same alias key) for the declared alias subtrees. Please add at least one collision test per subtree to meet the repository test contract.
As per coding guidelines packages/**/src/**/*.test.ts: Each alias subtree needs a collision test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/confluence/src/layout.test.ts` around lines 8 - 26, Add collision
tests for each alias subtree declared by layoutManifest: call layoutManifest(),
iterate manifest.aliasSegments and for each segment add a unit test (e.g.,
"alias collision for <segment>") that uses the same normalization function used
by the implementation (import the normalize/alias-key helper the code uses,
e.g., normalizeAlias or aliasKeyFrom) to produce two different inputs that
normalize to the same alias key, then assert the manifest/resource behavior on
collision (for manifest.resources[*].aliasSegments and the corresponding alias
subtree — e.g., check the produced alias key is identical and that
lookups/registry expose the expected single resolved entry or the defined
collision resolution). Ensure tests reference layoutManifest,
manifest.resources, manifest.aliasSegments and CANONICAL_ALIAS_SEGMENTS so each
declared subtree has one collision test.
Comment on lines
+8
to
+27
| test('layoutManifest exposes GitHub resources with canonical aliases and writeback schema pointers', () => { | ||
| const manifest = layoutManifest(); | ||
|
|
||
| assert.equal(manifest.provider, 'github'); | ||
| assert.deepEqual(manifest.aliasSegments, ['by-id', 'by-title']); | ||
| assert.ok(manifest.resources.length > 0); | ||
|
|
||
| for (const resource of manifest.resources) { | ||
| assert.ok(resource.path.startsWith('github/')); | ||
| assert.doesNotMatch(resource.path, /^\//u); | ||
| for (const alias of resource.aliasSegments) { | ||
| assert.ok(CANONICAL_ALIAS_SEGMENTS.has(alias), `unexpected alias segment ${alias}`); | ||
| } | ||
| for (const writeback of resource.writebackResources) { | ||
| assert.ok(writeback.path.startsWith('github/')); | ||
| assert.doesNotMatch(writeback.path, /^\//u); | ||
| assert.match(writeback.schemaId, /^github\/[a-z-]+$/u); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
Add alias-collision coverage for declared alias subtrees.
This test validates allowed alias names, but it does not verify collision behavior for the alias subtrees exposed by the manifest (for example by-title/by-name). Please add collision tests so colliding aliases are proven to resolve deterministically.
As per coding guidelines, "packages/**/src/**/*.test.ts: Each alias subtree needs a collision test."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/github/src/layout.test.ts` around lines 8 - 27, Add a new test in
the same file that uses layoutManifest() to exercise alias-subtree collisions:
iterate manifest.resources and for each resource build the per-alias subtree key
(using manifest.aliasSegments and resource.aliasSegments) and assert there are
no ambiguous/duplicate exposed targets without a deterministic tie-breaker;
specifically verify colliding alias names (e.g., the documented case like
by-title vs by-name) are resolved consistently by checking that when two aliases
map to the same subtree the manifest always prefers the same alias according to
manifest.aliasSegments order. Use the existing symbols layoutManifest,
manifest.aliasSegments, resource.aliasSegments and CANONICAL_ALIAS_SEGMENTS to
locate and validate alias collision behavior and add assertions that collisions
either do not occur or resolve deterministically.
Comment on lines
+8
to
+26
| test('layoutManifest exposes Jira resources with canonical aliases and writeback schema pointers', () => { | ||
| const manifest = layoutManifest(); | ||
|
|
||
| assert.equal(manifest.provider, 'jira'); | ||
| assert.deepEqual(manifest.aliasSegments, ['by-id', 'by-title', 'by-state']); | ||
|
|
||
| for (const resource of manifest.resources) { | ||
| assert.ok(resource.path.startsWith('jira/')); | ||
| assert.doesNotMatch(resource.path, /^\//u); | ||
| for (const alias of resource.aliasSegments) { | ||
| assert.ok(CANONICAL_ALIAS_SEGMENTS.has(alias), `unexpected alias segment ${alias}`); | ||
| } | ||
| for (const writeback of resource.writebackResources) { | ||
| assert.ok(writeback.path.startsWith('jira/')); | ||
| assert.doesNotMatch(writeback.path, /^\//u); | ||
| assert.match(writeback.schemaId, /^jira\/[a-z-]+$/u); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add alias-collision coverage for each declared alias subtree.
Current assertions validate format/allowlist only; they do not verify collision behavior in alias subtrees.
As per coding guidelines, "packages/**/src/**/*.test.ts: Each alias subtree needs a collision test."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/jira/src/layout.test.ts` around lines 8 - 26, Add a test that, for
each alias subtree declared in layoutManifest().aliasSegments, verifies
alias-collision behavior by constructing two resources that would resolve to the
same alias key under that subtree and asserting the collision is
detected/handled; update packages/jira/src/layout.test.ts to iterate
manifest.aliasSegments and for each alias segment create two minimal resource
entries (matching the existing resource shape used by
layoutManifest()/manifest.resources) with identical alias values, then assert
the system reports a collision (e.g., throws, returns an error, or marks
duplicates) for that alias segment while still validating
CANONICAL_ALIAS_SEGMENTS membership.
Comment on lines
+8
to
+27
| test('layoutManifest exposes Linear resources with canonical aliases and writeback schema pointers', () => { | ||
| const manifest = layoutManifest(); | ||
|
|
||
| assert.equal(manifest.provider, 'linear'); | ||
| assert.deepEqual(manifest.aliasSegments, ['by-id', 'by-title', 'by-state']); | ||
| assert.ok(manifest.resources.length > 0); | ||
|
|
||
| for (const resource of manifest.resources) { | ||
| assert.ok(resource.path.startsWith('linear/')); | ||
| assert.doesNotMatch(resource.path, /^\//u); | ||
| for (const alias of resource.aliasSegments) { | ||
| assert.ok(CANONICAL_ALIAS_SEGMENTS.has(alias), `unexpected alias segment ${alias}`); | ||
| } | ||
| for (const writeback of resource.writebackResources) { | ||
| assert.ok(writeback.path.startsWith('linear/')); | ||
| assert.doesNotMatch(writeback.path, /^\//u); | ||
| assert.match(writeback.schemaId, /^linear\/[a-z-]+$/u); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
Add alias collision coverage for alias subtrees.
This test validates alias names and formatting, but it does not verify collision behavior for alias subtrees. Please add a collision case to assert deterministic handling when two entries map to the same alias key.
Suggested test addition
test('layoutManifest exposes Linear resources with canonical aliases and writeback schema pointers', () => {
const manifest = layoutManifest();
@@
}
});
+
+test('alias subtrees include collision coverage', () => {
+ const manifest = layoutManifest();
+ // Example policy assertion: duplicated alias keys must be collision-safe.
+ // Add fixture-based assertions against the alias/path-mapper used by these subtrees.
+ // (compose -> parse -> equality + collision suffix behavior)
+ assert.ok(manifest.resources.length > 0);
+});As per coding guidelines, packages/**/src/**/*.test.ts: Each alias subtree needs a collision test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/linear/src/layout.test.ts` around lines 8 - 27, Add a unit test to
layout.test.ts that covers alias-subtree collisions: call layoutManifest (or
construct a manifest variant) with two resources that deliberately produce the
same alias key for a given aliasSegment (e.g., same "by-title" value), then
assert the deterministic collision resolution (for example, that
manifest.aliasSegments include the segment, that only one entry exists for the
colliding alias key or that entries are ordered/picked deterministically), and
keep using the existing symbols: layoutManifest, manifest.resources,
aliasSegments and CANONICAL_ALIAS_SEGMENTS; ensure the new test verifies the
collision behavior for each alias subtree and fails if resolution is
non-deterministic.
Comment on lines
+8
to
+27
| test('layoutManifest exposes Notion resources with canonical aliases and writeback schema pointers', () => { | ||
| const manifest = layoutManifest(); | ||
|
|
||
| assert.equal(manifest.provider, 'notion'); | ||
| assert.deepEqual(manifest.aliasSegments, ['by-id', 'by-title', 'by-name']); | ||
| assert.ok(manifest.resources.length > 0); | ||
|
|
||
| for (const resource of manifest.resources) { | ||
| assert.ok(resource.path.startsWith('notion/')); | ||
| assert.doesNotMatch(resource.path, /^\//u); | ||
| for (const alias of resource.aliasSegments) { | ||
| assert.ok(CANONICAL_ALIAS_SEGMENTS.has(alias), `unexpected alias segment ${alias}`); | ||
| } | ||
| for (const writeback of resource.writebackResources) { | ||
| assert.ok(writeback.path.startsWith('notion/')); | ||
| assert.doesNotMatch(writeback.path, /^\//u); | ||
| assert.match(writeback.schemaId, /^notion\/[a-z-]+$/u); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Add collision assertions for alias subtrees.
This test validates allowed alias segment names, but it does not verify collision behavior for alias subtrees (e.g., duplicate titles/names mapping to distinct collision-suffixed aliases).
As per coding guidelines, packages/**/src/**/*.test.ts: Each alias subtree needs a collision test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/notion/src/layout.test.ts` around lines 8 - 27, Update the test for
layoutManifest to also assert alias-subtree collision behavior: after calling
layoutManifest() iterate manifest.resources and for each alias in
resource.aliasSegments (and for alias-derived paths in resource.aliasSubtrees or
however alias paths are exposed) assert that any potentially colliding names
produce distinct suffixed aliases (e.g., check that when two resources would map
to the same alias segment the produced alias strings are different and include
the expected collision suffix), and add assertions that these collision-suffixed
aliases still start with 'notion/' and conform to CANONICAL_ALIAS_SEGMENTS
rules; use the existing symbols layoutManifest, manifest.resources,
resource.aliasSegments, CANONICAL_ALIAS_SEGMENTS and resource.writebackResources
to locate where to add the checks.
CodeRabbit + Devin review feedback on #93. Bug fixes: 1. packages/github/src/digest.ts pastTense - Replace substring regex with \b word-boundary regex. The previous pattern matched "open" inside "reopened" (a real GitHub webhook action) and produced "was opened" instead of "was updated" — a semantically wrong digest line. Same fix applied to the close and delete branches for consistency. (Devin BUG-0001) - Added regression test asserting `action: "reopened"` produces "was updated". 2. packages/notion/src/digest.ts pastTense - Same word-boundary fix. The substring regex matched "archive" inside "unarchived" and produced "was archived" — the semantic inverse, which would report a restored page as archived. (Devin BUG-0002) - Added regression test asserting `action: "unarchived"` produces "was updated". 3. packages/github/src/layout.ts and packages/linear/src/layout.ts - Top-level aliasSegments was a subset of the resource-level aliasSegments. Consumers that inspect only manifest.aliasSegments to discover available alias types would miss alias keys declared by individual resources (github/repos uses by-name; linear/teams uses by-name). Top-level is now the union of every resource's alias segments. (Devin BUG-0003, BUG-0004) - Added regression tests in both layout.test.ts files asserting the top-level/resource union invariant so a future drift gets caught. 4. packages/{github,linear,notion}/src/digest.ts compareEvents - Sort by parsed timestamps (ms) rather than lexicographic ISO strings. String compare misorders timestamps describing the same instant with different textual offsets (`Z` vs `+00:00`). Added eventTimeMs helper that parses with Date.parse and falls back to NEGATIVE_INFINITY on parse failure. (CodeRabbit notion/digest.ts:67; applied consistently to github and linear because they had the same bug.) 5. packages/notion/src/digest.ts hasCanonicalPath - Add explicit equality check for "/notion" to match the existing "notion" check, so the absolute-root path isn't excluded as an inconsistent edge case. (CodeRabbit notion/digest.ts:59) - Added regression test for `canonicalPath === "/notion"`. 6. Determinism check for M1 no-op digests (packages/{confluence,jira,slack}/src/digest.test.ts) - Call `digest(ctx)` twice and assert byte-identical results, matching the determinism acceptance criterion already covered by the linear/github/notion digest tests. (CodeRabbit confluence/digest.test.ts:22; applied to jira and slack for consistency.) Skipped (with reason): - CodeRabbit `packages/{confluence,github,jira,linear,notion}/src/ layout.test.ts` alias-collision tests. These reviews ask layout.test to verify alias-collision behavior, but layout.ts only declares the manifest shape — it doesn't implement alias normalization or collision resolution. Collision behavior is owned by the path-mapper and alias-resolver code in each provider (see path-mapper.ts) and has its own dedicated tests. Adding collision tests to layout.test would test the wrong layer and create false coupling between the manifest contract and the runtime resolver. Verification: - npm test for each provider: confluence 64/0, github 241/0, jira 60/0, linear 112/0, notion 116/0, slack 85/0 - npm run typecheck for each provider: green Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
Addressed all 12 review comments in Bug fixes (7)
Skipped with reason (5)
These reviews ask the Verification
🤖 Generated with Claude Code |
This was referenced May 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements work items 2 and 3 (adapter-side) from AgentWorkforce/relayfile docs/workspace-primitives-spec.md, pairing with the relayfile implementation PR that consumes these contracts.
Pairs with AgentWorkforce/relayfile PR for work items 1, 3 (mount-side), 4, 5, 6.
What's in this PR
For each of
linear,notion,github,slack,jira,confluence:src/digest.ts+src/digest.test.ts—digest()handler with the spec's contract(ctx: DigestContext) => Promise<DigestSection | null>.DigestContext/DigestSection/DigestHandlerare defined locally per provider for M1 (hoisting them into@relayfile/adapter-core/digest-contract.tsis a follow-up; doing it now would block this PR on a core-side export change that isn't required for the relayfile digest generator to consume the per-provider results).src/layout.ts+src/layout.test.ts—layoutManifest()returning the resource list, alias indexes, and writeback subdirectories the mount daemon needs to render<mount>/<provider>/.layout.md.src/index.ts— re-exports both symbols from the public barrel.Implementations
nullon empty windows (per WI 2 contract).null, matching the spec's "others can no-op tonullfor M1" guidance.Determinism
Per WI 2 acceptance criterion 4: each
digest.test.tsasserts that two runs over the same fixture produce byte-identicalDigestSectionoutputs.Pairs-with PR
writeback listCLI,.schema.jsondiscovery, dead-letter sidecars. Required: the relayfile daemon is what calls thesedigest()/layoutManifest()handlers.Test plan
npm test --workspace=packages/linear,--workspace=packages/notion,--workspace=packages/github,--workspace=packages/slack,--workspace=packages/jira,--workspace=packages/confluencerelayfile digest rebuild --window yesterdayon a fixture mount and verify each connected provider contributes a non-empty sectionFollow-up (not in this PR)
DigestContext/DigestSection/DigestHandlerandLayoutManifestProviderintopackages/core/src/{digest-contract,layout-contract}.tsonce provider authors converge on a stable shape. Per-provider locals work today and let M1 ship without an adapter-core API change.Provenance
Generated by the workspace-primitives implementation workflow (run
382654597c96e23b0b1dfa39, executed from the relayfile worktree). Per-slice fix-loop reports confirm typecheck + provider tests green (linear 101/0, github 237/0, notion 106/0, slack 85/0, jira 42/0, confluence 63/0).🤖 Generated with Claude Code