Herald commented on pull request qpismont/steep#9 2026-10-13 13:00:14 +02:00
Postgres persistance + job queue + api with sse

[bug] L’attente d’un permis n’écoute pas shutdown. Si tous les slots sont occupés au moment de l’arrêt, le bot attend qu’un slot se libère puis lance quand même le job en attente, et peut continuer ainsi à vider la file. Cela contredit le comportement annoncé où seuls les jobs déjà en cours terminent ; rendez aussi cette attente interruptible.

Herald commented on pull request qpismont/steep#9 2026-10-13 13:00:14 +02:00
Postgres persistance + job queue + api with sse

[bug] Au démarrage, chaque instance remet à Pending tous les jobs Processing, y compris ceux qu’une autre instance est encore en train de traiter. Un redémarrage ou déploiement pendant une review peut donc la faire exécuter en double. La récupération doit distinguer les jobs réellement abandonnés de ceux détenus par des workers actifs.

Herald commented on pull request qpismont/steep#9 2026-10-13 13:00:14 +02:00
Postgres persistance + job queue + api with sse

[bug] La lecture des jobs en attente n’en réserve aucun : deux instances peuvent récupérer le même job avant que l’une d’elles n’appelle mark_processing, puis lancer deux reviews et poster des résultats en double. Il faut réclamer les jobs de façon atomique, par exemple avec une mise à jour conditionnelle ou un verrouillage SKIP LOCKED.

Herald commented on pull request qpismont/steep#9 2026-10-13 12:48:26 +02:00
Postgres persistance + job queue + api with sse

Review Feedback

Herald commented on pull request qpismont/steep#9 2026-10-13 12:48:26 +02:00
Postgres persistance + job queue + api with sse

[bug] Si mark_reviewed échoue, l’erreur est seulement journalisée, puis le job est supprimé. Une panne temporaire de la base peut donc faire perdre définitivement l’événement sans enregistrer son résultat. Ne supprimez le job qu’après l’enregistrement réussi, ou conservez-le pour reprise.

Herald commented on pull request qpismont/steep#9 2026-10-13 12:48:26 +02:00
Postgres persistance + job queue + api with sse

[bug] Si la mise à jour du statut échoue, le job est quand même traité. Il reste alors Pending et sera récupéré au prochain sondage, ce qui peut lancer plusieurs reviews concurrentes pour le même événement. Ne lancez pas le traitement si cette transition échoue, ou garantissez une prise en charge atomique.

Herald commented on pull request qpismont/steep#9 2026-10-13 12:48:26 +02:00
Postgres persistance + job queue + api with sse

[performance] Cette requête récupère tous les jobs en attente, puis les met en mémoire dans le buffer. Si la file grossit ou si le service redémarre avec un grand retard à rattraper, chaque worker peut charger un volume non borné de payloads JSON. Limitez la taille des lots et ne chargez que les jobs nécessaires à la capacité disponible.

Herald commented on pull request qpismont/steep#9 2026-10-13 11:46:18 +02:00
Postgres persistance + job queue + api with sse

Review Feedback

Herald commented on pull request qpismont/steep#9 2026-10-13 11:46:18 +02:00
Postgres persistance + job queue + api with sse

[performance] Cette requête récupère tous les jobs en attente sans limite, puis les accumule dans le VecDeque à la ligne suivante. Une file PostgreSQL qui reçoit plus de jobs qu'elle n'en traite peut donc faire croître la mémoire du processus sans borne pratique. Il faudrait paginer ou limiter le nombre de jobs réservés par poll.

Herald commented on pull request qpismont/steep#9 2026-10-13 11:46:18 +02:00
Postgres persistance + job queue + api with sse

[bug] La lecture des jobs Pending et leur passage à Processing ne sont pas atomiques. Avec plusieurs instances du serveur (ou plusieurs consommateurs), elles peuvent toutes récupérer le même job avant l'appel à mark_processing, puis exécuter plusieurs fois la review et publier des commentaires dupliqués. Il faut effectuer une réservation conditionnelle en base (UPDATE ... WHERE status = 'pending' ... RETURNING) plutôt que seulement filtrer puis mettre à jour.

Herald commented on pull request qpismont/steep#9 2026-10-13 11:46:18 +02:00
Postgres persistance + job queue + api with sse

[bug] Au démarrage, tous les jobs Processing sont remis à Pending sans notion de propriétaire ou de lease. Si deux instances tournent simultanément, le démarrage de l'une peut réenfiler les jobs activement traités par l'autre, en plus de la course déjà présente dans le poller. La récupération après crash doit être limitée aux jobs dont le lease a expiré, et le traitement doit être atomiquement réservé.

Herald commented on pull request qpismont/steep#9 2026-10-13 11:46:18 +02:00
Postgres persistance + job queue + api with sse

[bug] Même quand exec échoue, le code enregistre une Review avec ok = false, puis supprime le job. Un webhook de retry sera ensuite considéré comme déjà traité par is_reviewed et ignoré : une panne temporaire de Gitea, du sandbox ou d'OpenRouter devient donc un échec définitif sans possibilité de retry. Les jobs échoués devraient rester en file (avec backoff/retry) ou être distingués des reviews effectivement publiées.

Herald commented on pull request qpismont/steep#9 2026-10-13 11:46:18 +02:00
Postgres persistance + job queue + api with sse

[bug] BOT_MAX_CONCURRENT=0 est accepté par le parsing de configuration, mais acquire_owned() attendra alors indéfiniment un permis. Comme cette attente n'est pas incluse dans un select! avec l'annulation, l'arrêt peut rester bloqué jusqu'au délai forcé de 60 secondes. Il faut rejeter zéro au chargement de configuration ou rendre l'attente annulable.

Herald commented on pull request qpismont/steep#9 2026-10-13 11:46:18 +02:00
Postgres persistance + job queue + api with sse

[bug] En cas d'échec de mark_processing, le code journalise l'erreur mais lance quand même le traitement. Le job reste alors Pending et peut être repris par le poller pendant que la review est déjà en cours, ce qui crée des traitements et commentaires en double. Il faut abandonner ce job (ou le remettre explicitement dans un état contrôlé) lorsque la réservation échoue.

Herald commented on pull request qpismont/steep#8 2026-09-29 19:43:53 +02:00
add default image docker fallback

Review Feedback

Herald commented on pull request qpismont/steep#8 2026-09-29 19:43:53 +02:00
add default image docker fallback

[bug] L'image de repli debian:stable-slim ne crée pas le répertoire /workspace, alors que workspace_folder() retourne cette valeur par défaut et que le conteneur est créé avec ce répertoire comme working_dir (lignes 124-125). Le démarrage échouera avec une erreur de répertoire de travail inexistant, avant même la copie du dépôt. Il faut créer le répertoire dans l'image de repli ou démarrer temporairement avec un répertoire existant (par exemple /) afin de créer /workspace avant de l'utiliser.

Herald commented on pull request qpismont/steep#8 2026-09-22 22:59:33 +02:00
add default image docker fallback

Review Feedback

Herald commented on pull request qpismont/steep#8 2026-09-22 22:59:33 +02:00
add default image docker fallback

[maintainability] La documentation indique que le fallback s'exécute dans /workspace, alors que l'implémentation utilise /workspaces/workspace par défaut. Cette divergence rend les commandes et instructions d'utilisation incorrectes pour les consommateurs du fallback ; alignez la documentation et le code.

Herald commented on pull request qpismont/steep#8 2026-09-22 22:59:33 +02:00
add default image docker fallback

[performance] Le tag d'une image construite n'est enregistré dans Container::owned_image qu'après la création, le démarrage et l'upload du workspace. Si le réseau, la création/le démarrage du container ou l'upload échoue avant cette ligne, l'image construite reste dans le daemon sans possibilité de nettoyage, ce qui laisse des images potentiellement volumineuses à chaque sandbox échouée. Nettoyez l'image dans chaque chemin d'erreur après le build, ou encapsulez sa propriété dans un garde-fou jusqu'à la création de Container.

Herald commented on pull request qpismont/steep#8 2026-09-22 22:59:33 +02:00
add default image docker fallback

[bug] L'image par défaut debian:stable-slim ne crée pas le répertoire /workspaces/workspace, qui est pourtant le chemin retourné par workspace_folder() lorsque cette valeur est absente. up() tente ensuite d'y téléverser le workspace et configure ce chemin comme répertoire de travail ; le fallback échouera donc probablement dès le démarrage ou l'upload. Il faut créer ce répertoire avant l'upload, ou choisir un chemin garanti d'exister comme /tmp/workspace.