Skip to content

EXPLORING: just want to see how this feels, effect rewrite#128

Open
bmdavis419 wants to merge 2 commits intomainfrom
davis/effect-rewrite
Open

EXPLORING: just want to see how this feels, effect rewrite#128
bmdavis419 wants to merge 2 commits intomainfrom
davis/effect-rewrite

Conversation

@bmdavis419
Copy link
Collaborator

@bmdavis419 bmdavis419 commented Jan 26, 2026


Open with Devin

Greptile Overview

Greptile Summary

This PR begins an Effect rewrite exploration of the btca CLI and Server. It's marked "EXPLORING" in the title, indicating this is experimental work to evaluate how Effect integration feels.

Key Changes:

  • Added extensive planning documentation (PROMPT.md, STATUS.md, 4 markdown files in effect-rewrite-scratch/)
  • Added effect dependency to both CLI and server packages
  • Created Effect wrapper utilities (runner.ts, cli-exit.ts, server-manager.ts)
  • Converted all CLI commands to use Effect.scoped pattern with withServer for automatic cleanup
  • Wrapped server route handlers in Effect.gen but still using Hono (not migrated to @effect/platform HttpServer per plan)

Planning Files Found:
The PR contains multiple comprehensive planning documents (over 3000 lines total). These are likely valuable for tracking the rewrite effort but significantly increase the PR size. Key files:

  • PROMPT.md - Task instructions for agents
  • STATUS.md - Progress tracking showing Phase 1 in progress
  • effect-rewrite-scratch/rewrite-plan.md - 869-line architecture plan
  • effect-rewrite-scratch/effect-patterns-reference.md - 1016-line reference guide
  • effect-loop.sh - Automated rewrite loop script

Implementation Status:
According to STATUS.md, this is Phase 1 (Core Server Services). The plan shows significant work remains including:

  • Phase 2: HTTP Server Migration (replace Hono with Effect HttpServer)
  • Phase 3: Agent & Stream conversion
  • Phase 4: CLI completion
  • Phase 5: Testing & cleanup

Confidence Score: 2/5

  • Not safe to merge - CLI commands have critical issue where void keyword prevents awaiting Effects
  • Score reflects previously-flagged critical bug where all CLI commands use void runCli() preventing proper async execution, plus incomplete Effect conversion still mixing old and new patterns
  • All CLI command files (apps/cli/src/commands/*.ts) need the void keyword removed to properly await Effect execution. apps/cli/src/effect/runner.ts needs return statement after process.exit

Important Files Changed

Filename Overview
PROMPT.md New planning document for Effect rewrite with task instructions
STATUS.md New status tracking file showing phase 1 progress and remaining tasks
effect-rewrite-scratch/rewrite-plan.md Comprehensive 869-line plan document with architecture, patterns, and code examples
apps/cli/src/effect/runner.ts New CLI runner with missing return after process.exit (previously flagged)
apps/cli/src/commands/add.ts Converted to Effect.scoped with void keyword preventing await (previously flagged issue)
apps/cli/src/commands/ask.ts Converted to Effect.scoped with void keyword preventing await (previously flagged issue)
apps/cli/src/commands/chat.ts Converted to Effect.scoped with void keyword preventing await (previously flagged issue)
apps/cli/src/commands/clear.ts Converted to Effect.scoped with void keyword preventing await (previously flagged issue)
apps/cli/src/commands/config.ts Converted to Effect.scoped with void keyword preventing await (previously flagged issue)
apps/cli/src/commands/connect.ts Converted to Effect.scoped with void keyword preventing await (previously flagged issue)
apps/cli/src/commands/remove.ts Converted to Effect.scoped with void keyword preventing await (previously flagged issue)
apps/cli/src/commands/serve.ts Converted to Effect.scoped with void keyword preventing await (previously flagged issue)
apps/server/src/index.ts All route handlers wrapped in Effect.gen, still using Hono (not HttpServer per plan)

Context used:

  • Context from dashboard - AGENTS.md (source)

@bmdavis419 bmdavis419 mentioned this pull request Jan 26, 2026
Copy link
Collaborator Author

bmdavis419 commented Jan 26, 2026

@bmdavis419 bmdavis419 changed the title started effect stuff EXPLORING: just want to see how this feels, effect rewrite Jan 26, 2026
@bmdavis419 bmdavis419 marked this pull request as ready for review January 26, 2026 20:17
@bmdavis419 bmdavis419 changed the base branch from davis/remote-local-rebuild to graphite-base/128 January 26, 2026 20:25
@bmdavis419 bmdavis419 force-pushed the davis/effect-rewrite branch from 737cc38 to 9969bb4 Compare January 26, 2026 20:54
@bmdavis419 bmdavis419 changed the base branch from graphite-base/128 to main January 26, 2026 20:54
@bmdavis419 bmdavis419 changed the base branch from main to graphite-base/128 January 26, 2026 21:10
@bmdavis419 bmdavis419 force-pushed the davis/effect-rewrite branch from 9969bb4 to 781b53c Compare January 26, 2026 21:10
@bmdavis419 bmdavis419 changed the base branch from graphite-base/128 to davis/remote-local-rebuild January 26, 2026 21:10
@bmdavis419 bmdavis419 force-pushed the davis/effect-rewrite branch from 781b53c to 0085820 Compare January 26, 2026 22:45
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8 files reviewed, 8 comments

Edit Code Review Agent Settings | Greptile


if (inReasoning) {
process.stdout.write('\n</thinking>\n');
void runCli(program, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runCli returns a Promise but void keyword prevents awaiting it, causing the action to exit immediately

Suggested change
void runCli(program, {
await runCli(program, {
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/cli/src/commands/ask.ts
Line: 206:206

Comment:
`runCli` returns a Promise but `void` keyword prevents awaiting it, causing the action to exit immediately

```suggestion
		await runCli(program, {
```

How can I resolve this? If you propose a fix, please make it concise.

console.error(formatError(error));
process.exit(1);
}
void runCli(program, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runCli returns a Promise but void keyword prevents awaiting it

Suggested change
void runCli(program, {
await runCli(program, {
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/cli/src/commands/chat.ts
Line: 74:74

Comment:
`runCli` returns a Promise but `void` keyword prevents awaiting it

```suggestion
		await runCli(program, {
```

How can I resolve this? If you propose a fix, please make it concise.

console.error(formatError(error));
process.exit(1);
}
void runCli(program, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runCli returns a Promise but void keyword prevents awaiting it

Suggested change
void runCli(program, {
await runCli(program, {
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/cli/src/commands/clear.ts
Line: 41:41

Comment:
`runCli` returns a Promise but `void` keyword prevents awaiting it

```suggestion
		await runCli(program, {
```

How can I resolve this? If you propose a fix, please make it concise.

console.error(formatError(error));
process.exit(1);
}
void runCli(program, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runCli returns a Promise but void keyword prevents awaiting it

Suggested change
void runCli(program, {
await runCli(program, {
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/cli/src/commands/config.ts
Line: 144:144

Comment:
`runCli` returns a Promise but `void` keyword prevents awaiting it

```suggestion
		await runCli(program, {
```

How can I resolve this? If you propose a fix, please make it concise.

if (error instanceof Error && error.message === 'Invalid selection') {
console.error('\nError: Invalid selection. Please try again.');
process.exit(1);
void runCli(program, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runCli returns a Promise but void keyword prevents awaiting it

Suggested change
void runCli(program, {
await runCli(program, {
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/cli/src/commands/connect.ts
Line: 284:284

Comment:
`runCli` returns a Promise but `void` keyword prevents awaiting it

```suggestion
		await runCli(program, {
```

How can I resolve this? If you propose a fix, please make it concise.

})
);

void runCli(program, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runCli returns a Promise but void keyword prevents awaiting it

Suggested change
void runCli(program, {
await runCli(program, {
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/cli/src/commands/remove.ts
Line: 127:127

Comment:
`runCli` returns a Promise but `void` keyword prevents awaiting it

```suggestion
		await runCli(program, {
```

How can I resolve this? If you propose a fix, please make it concise.

}
});

void runCli(program, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runCli returns a Promise but void keyword prevents awaiting it

Suggested change
void runCli(program, {
await runCli(program, {
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/cli/src/commands/serve.ts
Line: 52:52

Comment:
`runCli` returns a Promise but `void` keyword prevents awaiting it

```suggestion
		await runCli(program, {
```

How can I resolve this? If you propose a fix, please make it concise.

})
);

void runCli(program, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runCli returns a Promise but void keyword prevents awaiting it

Suggested change
void runCli(program, {
await runCli(program, {
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/cli/src/commands/add.ts
Line: 446:446

Comment:
`runCli` returns a Promise but `void` keyword prevents awaiting it

```suggestion
			await runCli(program, {
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View issue and 5 additional flags in Devin Review.

Open in Devin Review

Comment on lines +24 to +29
export const provideServerServices = (services: ServerServices) =>
Effect.provideService(ConfigService, services.config).pipe(
Effect.provideService(ResourcesService, services.resources),
Effect.provideService(CollectionsService, services.collections),
Effect.provideService(AgentService, services.agent)
);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Incorrect Effect.provideService usage causes service provisioning to fail

The provideServerServices function incorrectly chains Effect.provideService calls. Effect.provideService(Tag, value) returns a function (effect: Effect) => Effect, not an Effect itself. Calling .pipe() on this function result is incorrect.

Click to expand

How the bug occurs

The code at apps/server/src/effect/services.ts:24-29 does:

export const provideServerServices = (services: ServerServices) =>
  Effect.provideService(ConfigService, services.config).pipe(
    Effect.provideService(ResourcesService, services.resources),
    Effect.provideService(CollectionsService, services.collections),
    Effect.provideService(AgentService, services.agent)
  );

Effect.provideService(ConfigService, services.config) returns a function, not an Effect. Calling .pipe() on a function is semantically wrong and will cause the services to not be correctly provided.

Expected behavior

The function should return a composable operator that can be applied to an effect:

export const provideServerServices = (services: ServerServices) =>
  <A, E, R>(effect: Effect.Effect<A, E, R>) =>
    effect.pipe(
      Effect.provideService(ConfigService, services.config),
      Effect.provideService(ResourcesService, services.resources),
      Effect.provideService(CollectionsService, services.collections),
      Effect.provideService(AgentService, services.agent)
    );

Impact

All server routes that use runWithServerServices (which calls effect.pipe(provideServerServices(services))) will fail to properly provide services, causing runtime errors when effects try to access ConfigService, ResourcesService, CollectionsService, or AgentService.

Recommendation: Change provideServerServices to return a proper Effect operator function that takes an effect and pipes the service provisions through it.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@bmdavis419 bmdavis419 force-pushed the davis/effect-rewrite branch from 0085820 to 5765e4a Compare January 27, 2026 21:28
@bmdavis419 bmdavis419 force-pushed the davis/remote-local-rebuild branch from de1a1d3 to abfc6de Compare January 27, 2026 21:46
@bmdavis419 bmdavis419 force-pushed the davis/effect-rewrite branch 3 times, most recently from ea2d064 to 342ae71 Compare January 27, 2026 21:51
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +16 to +24
if (error instanceof CliExit) {
if (!error.printed && error.message && options?.onError) {
options.onError(error);
}
process.exit(error.code);
}

options?.onError?.(error);
process.exit(1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing return after process.exit(error.code) causes the code to continue executing and call options?.onError?.(error) and process.exit(1) again

Suggested change
if (error instanceof CliExit) {
if (!error.printed && error.message && options?.onError) {
options.onError(error);
}
process.exit(error.code);
}
options?.onError?.(error);
process.exit(1);
if (error instanceof CliExit) {
if (!error.printed && error.message && options?.onError) {
options.onError(error);
}
process.exit(error.code);
}
options?.onError?.(error);
process.exit(1);
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/cli/src/effect/runner.ts
Line: 16:24

Comment:
missing return after `process.exit(error.code)` causes the code to continue executing and call `options?.onError?.(error)` and `process.exit(1)` again

```suggestion
		if (error instanceof CliExit) {
			if (!error.printed && error.message && options?.onError) {
				options.onError(error);
			}
			process.exit(error.code);
		}

		options?.onError?.(error);
		process.exit(1);
```

How can I resolve this? If you propose a fix, please make it concise.

@bmdavis419 bmdavis419 changed the base branch from davis/remote-local-rebuild to graphite-base/128 January 27, 2026 21:56
@bmdavis419 bmdavis419 force-pushed the davis/effect-rewrite branch from 342ae71 to 080d4d6 Compare January 27, 2026 21:57
@graphite-app graphite-app bot changed the base branch from graphite-base/128 to main January 27, 2026 21:58
@bmdavis419 bmdavis419 force-pushed the davis/effect-rewrite branch from 080d4d6 to fe3b4c6 Compare January 27, 2026 21:58
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.

1 participant

Comments