Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,22 @@ LOG_FILE=~/.agentboard/agentboard.log # default; set empty to disable file log

Console output is pretty-printed in development, JSON in production (`NODE_ENV=production`). File output is always JSON. Set `LOG_FILE=` (empty) to disable file logging.

### Logging Quality Contract

- Event names must be `snake_case`
- Catch blocks in server runtime paths must either:
- emit a structured log event, or
- be explicitly documented as intentional with `logging-audit:intentional` and a brief rationale
- `logger.warn` and `logger.error` used in catch paths must include useful error context fields (`error_message`, `error_name`, `error_stack`, `code`, etc.)

Run the logging auditor:

```bash
bun run audit:logging
```

The command fails if high-severity findings are present.

## Troubleshooting

### "open terminal failed: not a terminal" errors
Expand Down
72 changes: 72 additions & 0 deletions docs/logging-audit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# Logging Quality Audit

## Scope (Frozen)

- Runtime server observability only: `src/server/**`
- Excluded: client/UI logs and all test files
- Priority remediation targets for this pass:
- API error paths in `src/server/index.ts`
- DB migration error paths in `src/server/db.ts`
- Intentional silent catches in `src/server/SessionManager.ts`

## Quality Criteria

- Event names are `snake_case`
- Catch blocks are either logged or explicitly marked intentional (`logging-audit:intentional`)
- `logger.warn` / `logger.error` in catch paths include error context (`error_message`, `error_name`, `error_stack`, `code`, etc.)

## Auditor

- Command: `bun run audit:logging`
- Implementation:
- `scripts/logging-audit.ts`
- `src/server/loggingAudit.ts`

## Baseline Findings (Before Remediation)

`bun run audit:logging` reported:

- `high=10`
- `medium=97`
- `low=0`

High-severity findings (all fixed in this task):

1. `src/server/db.ts:414` `catch_without_logging`
2. `src/server/db.ts:581` `catch_without_logging`
3. `src/server/index.ts:830` `catch_without_logging`
4. `src/server/index.ts:882` `catch_without_logging`
5. `src/server/index.ts:916` `catch_without_logging`
6. `src/server/index.ts:978` `catch_without_logging`
7. `src/server/index.ts:1051` `catch_without_logging`
8. `src/server/index.ts:1073` `catch_without_logging`
9. `src/server/index.ts:1097` `catch_without_logging`
10. `src/server/index.ts:1130` `catch_without_logging`

## Remediation Completed

- `src/server/index.ts`
- Added structured warning logs for client-log ingestion failures.
- Added structured warning logs for session-preview read failures with session/log path context.
- Added structured warning logs for directory stat/read failures.
- Added structured warning logs for settings parsing failures and upload/clipboard failures.
- Added structured warning logs for websocket payload parse failures and session create/kill/rename/resume error paths.
- Replaced non-snake event `ssh_proxy_onExit` with `ssh_proxy_on_exit`.
- `src/server/db.ts`
- Added migration rollback/failure logging with migration IDs and error details before rethrow.
- Added debug logs for non-fatal data directory permission setup failures.
- `src/server/SessionManager.ts`
- Added debug logs for non-fatal tmux fallback/error paths.
- Marked high-frequency pane-capture silent catch as intentional with `logging-audit:intentional`.
- Supporting runtime modules
- Marked deliberate best-effort catch paths with `logging-audit:intentional` (log discovery/matcher, worker plumbing, and terminal proxies) so silent behavior is explicitly documented.

## Current Status (After Remediation)

`bun run audit:logging` now reports:

- `high=0`
- `medium=0`
- `low=0`

No outstanding findings in scope.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"lint": "oxlint .",
"typecheck": "tsc --noEmit",
"test": "bun scripts/test-runner.ts",
"audit:logging": "bun scripts/logging-audit.ts --fail-on=high",
"test:coverage": "bun scripts/test-runner.ts --coverage --coverage-reporter=lcov --skip-isolated && bun run coverage:all",
"coverage:all": "bun scripts/coverage-all.ts",
"test:e2e": "playwright test",
Expand Down
78 changes: 78 additions & 0 deletions scripts/logging-audit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import {
formatLoggingAuditReport,
runLoggingAudit,
type LoggingAuditSeverity,
} from '../src/server/loggingAudit'

interface CliOptions {
failOn: LoggingAuditSeverity | 'none'
json: boolean
}

const severityRank: Record<LoggingAuditSeverity, number> = {
high: 0,
medium: 1,
low: 2,
}

function parseArgs(argv: string[]): CliOptions {
let failOn: CliOptions['failOn'] = 'high'
let json = false

for (const arg of argv) {
if (arg === '--json') {
json = true
continue
}
if (arg.startsWith('--fail-on=')) {
const value = arg.slice('--fail-on='.length)
if (value === 'none' || value === 'high' || value === 'medium' || value === 'low') {
failOn = value
} else {
throw new Error(`Invalid --fail-on value: ${value}`)
}
}
}

return { failOn, json }
}

function shouldFail(
highestSeverity: LoggingAuditSeverity | null,
failOn: CliOptions['failOn']
): boolean {
if (failOn === 'none') {
return false
}
if (highestSeverity === null) {
return false
}
return severityRank[highestSeverity] <= severityRank[failOn]
}

function findHighestSeverity(
counts: Record<LoggingAuditSeverity, number>
): LoggingAuditSeverity | null {
if (counts.high > 0) return 'high'
if (counts.medium > 0) return 'medium'
if (counts.low > 0) return 'low'
return null
}

async function main() {
const options = parseArgs(process.argv.slice(2))
const report = await runLoggingAudit()

if (options.json) {
console.log(JSON.stringify(report, null, 2))
} else {
console.log(formatLoggingAuditReport(report))
}

const highestSeverity = findHighestSeverity(report.counts)
if (shouldFail(highestSeverity, options.failOn)) {
process.exitCode = 1
}
}

await main()
52 changes: 47 additions & 5 deletions src/server/SessionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,11 @@ export class SessionManager {
ensureSession(): void {
try {
this.runTmux(['has-session', '-t', this.sessionName])
} catch {
} catch (error) {
logger.debug('tmux_session_missing_create', {
sessionName: this.sessionName,
...toErrorLogFields(error),
})
this.runTmux(['new-session', '-d', '-s', this.sessionName])
}
// Set mouse mode for scroll wheel support (SGR mouse sequences)
Expand All @@ -98,7 +102,12 @@ export class SessionManager {
'mouse',
enabled ? 'on' : 'off',
])
} catch {
} catch (error) {
logger.debug('tmux_mouse_mode_apply_deferred', {
sessionName: this.sessionName,
enabled,
...toErrorLogFields(error),
})
// Session doesn't exist yet, will be applied on next ensureSession
}
}
Expand Down Expand Up @@ -305,7 +314,11 @@ export class SessionManager {
.split('\n')
.map((line) => line.trim())
.filter(Boolean)
} catch {
} catch (error) {
logger.debug('tmux_list_sessions_failed', {
sessionName: this.sessionName,
...toErrorLogFields(error),
})
return []
}
}
Expand Down Expand Up @@ -358,6 +371,10 @@ export class SessionManager {
if (!isTmuxFormatError(error)) {
throw error
}
logger.debug('tmux_window_list_format_fallback', {
sessionName,
...toErrorLogFields(error),
})
}

return this.runTmux([...args, WINDOW_LIST_FORMAT_FALLBACK])
Expand Down Expand Up @@ -409,7 +426,11 @@ export class SessionManager {
try {
const output = this.runTmux(['show-options', '-gv', 'base-index'])
return Number.parseInt(output.trim(), 10) || 0
} catch {
} catch (error) {
logger.debug('tmux_base_index_defaulted', {
sessionName: this.sessionName,
...toErrorLogFields(error),
})
return 0
}
}
Expand All @@ -427,7 +448,11 @@ export class SessionManager {
.split('\n')
.map((line) => Number.parseInt(line.trim(), 10))
.filter((n) => !Number.isNaN(n))
} catch {
} catch (error) {
logger.debug('tmux_window_indices_unavailable', {
sessionName: this.sessionName,
...toErrorLogFields(error),
})
return []
}
}
Expand Down Expand Up @@ -572,9 +597,26 @@ function capturePaneWithDimensions(tmuxWindow: string): PaneCapture | null {
const content = lines.slice(-30).join('\n')
return { content, width, height }
} catch {
// logging-audit:intentional Pane capture runs at poll cadence; transient
// failures are expected when panes close and logging each one would flood logs.
return null
}
}

// Re-export for external use
export { detectsPermissionPrompt } from './statusInference'

function toErrorLogFields(error: unknown): {
error_message: string
error_name?: string
error_stack?: string
} {
if (error instanceof Error) {
return {
error_message: error.message,
error_name: error.name,
error_stack: error.stack,
}
}
return { error_message: String(error) }
}
Loading